public/pp-planer
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity
pp-planer/.sisyphus/notepads/ccli-songselect-import/issues.md

4.7 KiB
Raw Blame History

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.