# CCLI SongSelect Import — Issues

## [2026-05-10] Known Issues / Gotchas

### T1: Fixture corpus requires structurally accurate CCLI format
- Plan originally asked for "real CCLI text pastes" but since we can't access SongSelect, agent creates synthetic fixtures.
- Synthetic fixtures MUST match exact CCLI format: title line, blank, section label, lyrics, blank, footer with © and CCLI #.
- The parser tests depend on these fixtures — structural accuracy is critical.

### T7: ProImportService method name
- Plan was corrected: the public method is `ProImportService::import(UploadedFile $file)`, not `upsertSong()`.
- When mirroring the pattern, READ app/Services/ProImportService.php before implementing.

### No Admin Role
- No `admin` role or Policy exists in the codebase.
- CCLI Settings section is visible to ALL authenticated users.
- Document this decision, don't create a policy gate.

### AGENDA_KEYS whitelist
- SettingsController has a `const AGENDA_KEYS` array.
- T4 MUST add `'default_translation_language'` to this array OR update the validation to include it.
- Failure to update AGENDA_KEYS = PATCH /settings will silently ignore the new key.

### Translation Pairing Label Direction
- CCLI paste can have English labels; local songs may have German labels (Strophe, Refrain).
- CcliLabels::normalizeLabelName() normalizes BOTH directions to canonical English before pairing.
- Do NOT assume same language on both sides.

### T7: Global Label Slide Replacement Caveat
- The requested CCLI import pattern deletes `songSlides()` on the resolved global `Label` before recreating slides.
- Because labels are shared globally, a later import using the same canonical label name replaces that label's slide text globally; this intentionally matches the task spec and existing `ProImportService` pattern, but remains a design caveat for future per-song slide ownership work.

### T8: Translation Pairing Leaves Missing Sections Non-Fatal
- `CcliTranslationPairingService` intentionally does not throw when a local arrangement label is absent from the CCLI paste.
- Missing labels are returned in `unmatched_labels`, and their mapping entries keep empty slide placeholders so `distributed_text` still aligns with the local arrangement shape.

## Code Quality Review (2026-05-11)

### Found
- **app/Services/CcliPasteParser.php:17-19** — Empty `if` block in constructor.
  Constructor accepts optional closures (`$sectionDetector`, `$metadataDetector`) but the body
  contains `if ($sectionDetector !== null || $metadataDetector !== null) { }` with no statements.
  Dead code — same category as forbidden empty `catch` blocks. Remove the entire `if` statement.

### Passed
- Pint: 9 files clean (after fixing `-rw-------` perms on DTO + Service files)
- Tests: 503 passed, 0 failed
- Build: vite production build clean
- No `as any`, `@ts-ignore`, `console.log` in Vue
- No `@` error suppression in PHP
- German Du-form throughout (no `Sie`/`Ihre`)
- All `final` + `readonly` correct
- data-testid coverage complete on interactive Vue elements

### Side note — file permissions
Several new PHP files were created with `-rw-------` (owner-only). DDEV web container could not
read them, breaking Pint. Fixed by `chmod 644`. Likely an editor/umask issue worth investigating
in the dev environment to avoid recurrence.

### Resolution (2026-05-11 re-review)
- Empty `if` block in `CcliPasteParser.php` removed; constructor is now `) {}`.
- Pint PASS on the file.
- Tests: 503 passed.
- Build: clean.
- Vue: no forbidden patterns.
- Final verdict: APPROVE.

## QA Finding (2026-05-11): API auth broken via DDEV URL

**Root cause**: `APP_URL=http://pp-planer.test` in `.env`, but DDEV serves on `https://pp-planer.ddev.site`.
`config/sanctum.php` derives `SANCTUM_STATEFUL_DOMAINS` from `APP_URL` host → list contains `pp-planer.test` only.
Cookie-based auth on `/api/*` routes (`auth:sanctum` middleware) fails when accessed via `pp-planer.ddev.site`.

**Symptom**: 
- `/api/ccli/preview` with `Accept: application/json` → `401 Unauthenticated`
- `/api/ccli/preview` with default `Accept: */*` (Vue dialog's fetch) → 200 HTML (auth redirect), then `res.json()` throws "Unexpected token '<'" → caught → dialog shows "Netzwerkfehler. Bitte versuche es erneut."
- E2E tests (`ccli-paste-import.spec.ts` tests 6+7) skip silently via `test.skip()` guards when API fails, masking this in CI.

**Fix**: Add to `.env` and `.env.example`:
```
SANCTUM_STATEFUL_DOMAINS=pp-planer.ddev.site,pp-planer.test,localhost,127.0.0.1
```
Or change `APP_URL` to `https://pp-planer.ddev.site` if DDEV is the canonical dev URL.

**Bookmarklet has matching issue**: `BookmarkletController` uses `Config::get('app.url')` → bookmarklet opens `http://pp-planer.test/songs/import-from-ccli-paste?prefill=...` instead of the ddev URL.