chore(notes): update ccli-songselect-import notepad and task-1 evidence
This commit is contained in:
Thorsten Bus
parent
fdabaadb89
commit
5b9a877607
|
|
@ -1,8 +1,3 @@
|
|||
ls tests/fixtures/ccli/*.txt | wc -l
|
||||
22
|
||||
|
||||
grep -rl "Strophe\|Refrain" tests/fixtures/ccli/ | wc -l
|
||||
4
|
||||
|
||||
grep -rl "(Repeat)" tests/fixtures/ccli/ | wc -l
|
||||
2
|
||||
22
|
||||
4
|
||||
2
|
||||
|
|
|
|||
|
|
@ -1,9 +1,10 @@
|
|||
ddev exec php artisan test --filter=CcliFixtureSanityTest
|
||||
|
||||
PASS Tests\Feature\CcliFixtureSanityTest
|
||||
✓ ccli fixture corpus has at least 20 txt files
|
||||
✓ each ccli fixture is valid utf8 with section labels and title
|
||||
✓ ccli fixture corpus covers german labels
|
||||
✓ ccli fixture corpus covers repeat markers
|
||||
PASS Tests\Feature\CcliFixtureSanityTest
|
||||
✓ ccli fixture corpus has at least 20 txt files 0.23s
|
||||
✓ each ccli fixture is valid utf8 with section labels and title
|
||||
✓ ccli fixture corpus covers german labels
|
||||
✓ ccli fixture corpus covers repeat markers
|
||||
|
||||
Tests: 4 passed (160 assertions)
|
||||
Duration: 0.35s
|
||||
|
||||
Tests: 4 passed (160 assertions)
|
||||
|
|
|
|||
|
|
@ -33,3 +33,53 @@ ### T7: Global Label Slide Replacement Caveat
|
|||
### 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.
|
||||
|
|
|
|||
|
|
@ -108,3 +108,25 @@ ### 2026-05-11 CcliPasteController (T10)
|
|||
- `tests/fixtures/ccli/` (lowercase) is the canonical fixture directory; existing tests already declare a top-level `ccliFixturePath()` helper, so new test files need a uniquely-named helper to avoid `Cannot redeclare function` errors in Pest.
|
||||
- Web route `songs.import-from-ccli-paste` needs the `auth` middleware (web-style redirect to login), while the API routes use sanctum (401 JSON response); the difference matters for unauthenticated test assertions (`assertRedirect(route('login'))` vs `assertUnauthorized()`).
|
||||
- `CcliImportService::import()` throws `RuntimeException` for missing CCLI id and `InvalidArgumentException` (via parser) for parse failures; controller catches both to return 422 with a German message.
|
||||
|
||||
## 2026-05-11 Plan Compliance Audit
|
||||
|
||||
- CCLI SongSelect Import audit approved: 20/20 must-have checks present, 4/4 forbidden-pattern checks clean.
|
||||
- Targeted PHP filter run passed: 140 tests, 807 assertions; npm build and Pint check also passed.
|
||||
- Registered routes verified: api.ccli.preview, api.ccli.import, bookmarklets.ccli, songs.import-from-ccli-paste.
|
||||
|
||||
## QA Re-verification (2026-05-11): Sanctum fix confirmed
|
||||
|
||||
After applying `pp-planer.ddev.site` to the default stateful domains in `config/sanctum.php` and clearing config cache:
|
||||
|
||||
- `sanctum.stateful` resolves to `[pp-planer.test, pp-planer.ddev.site, localhost, localhost:8000, 127.0.0.1, 127.0.0.1:8000, ::1]` ✓
|
||||
- `/api/ccli/preview` returns 200 JSON when called from an authenticated DDEV browser session (was 401 before)
|
||||
- End-to-end Vue dialog flow on `/songs`: paste lyrics → click "Vorschau" → preview metadata (CCLI-Nr, sections) renders. No "Netzwerkfehler" message.
|
||||
- E2E test `paste fixture and preview shows metadata` now PASSES (was silently skipped before the fix).
|
||||
- Suite result: 11 passed, 3 skipped (vs 10 passed, 4 skipped pre-fix). Net +1 test exercising the API endpoint.
|
||||
|
||||
Remaining skipped tests (3):
|
||||
- `duplicate import shows error with edit link` — guarded skip when initial create import returns non-201. Likely a separate data-state or status-code issue (worth investigating but unrelated to the Sanctum fix).
|
||||
- 2× CCLI translation pairing tests — were skipped before the fix too; not regressions.
|
||||
|
||||
Bookmarklet endpoint still serves the same JS payload, header `Content-Type: text/javascript`, body starts with `javascript:`. Note: the embedded `APP_URL` is still `http://pp-planer.test` (driven by `config('app.url')`), so a DDEV user clicking the bookmarklet on SongSelect still lands on the wrong host. That's an APP_URL / bookmarklet-config concern, not a Sanctum concern.
|
||||
|
|
|
|||
Loading…
Reference in a new issue