diff --git a/.sisyphus/notepads/cts-presenter-app/learnings.md b/.sisyphus/notepads/cts-presenter-app/learnings.md index eecdff1..084cd0b 100644 --- a/.sisyphus/notepads/cts-presenter-app/learnings.md +++ b/.sisyphus/notepads/cts-presenter-app/learnings.md @@ -14,6 +14,7 @@ - 2026-03-01: Translation routes: `POST /api/translation/fetch-url` (preview), `POST /api/songs/{song}/translation/import` (save), `DELETE /api/songs/{song}/translation` (remove). All under `auth:sanctum` middleware. - 2026-03-01: `removeTranslation` uses a two-step approach: collect slide IDs via `SongSlide::whereIn('song_group_id', $song->groups()->pluck('id'))` then bulk-update `text_content_translated = null`, avoiding N+1 queries. - 2026-03-01: Der Arrangement-Konfigurator bleibt stabil bei mehrfachen Gruppeninstanzen, wenn die Sequenz mit Vue-Keys im Muster `${group.id}-${index}` gerendert und die Reihenfolge nach jedem Drag-End sofort per `router.put(..., { preserveScroll: true })` gespeichert wird. +- 2026-05-03: `MacroColorConverter::fromRgba()` muss nur RGB clampen und als uppercase-6-digit Hex ausgeben; `tinker --execute` ist eine schnelle Verifikation fuer solche statischen Helper. ## [2026-03-01] Wave 2 Complete — T8-T13 @@ -350,3 +351,5 @@ ### Verification Success Criteria Met ### Next Steps - Task 2 will likely involve testing OAuth login flow with ChurchTools - May need to configure CTS_API_TOKEN and CHURCHTOOLS credentials for full testing + +- 2026-05-04: `ProBundleExportService` muss fuer non-song `.probundle` Exporte den aktuellen `Service` plus `part_type` bis in `buildBundleFromSlides()` durchreichen; `MacroResolutionService::macrosForSlide()` bekommt fuer Bildfolien `label_id => null`, damit nur all/first/last Positionen greifen. diff --git a/.sisyphus/notepads/macros-and-labels-import/decisions.md b/.sisyphus/notepads/macros-and-labels-import/decisions.md index e0e6c2c..3c6e8c6 100644 --- a/.sisyphus/notepads/macros-and-labels-import/decisions.md +++ b/.sisyphus/notepads/macros-and-labels-import/decisions.md @@ -35,3 +35,7 @@ ### Label Color Priority ### Current Migration Scope - `labels` migration only defines the schema; no model or business logic belongs in this task - Use `hidden_at` instead of `deleted_at` to align with soft-hide semantics + +### Macros Tables Task +- Keep all three tables in one migration file so the schema lands together and the junction FKs resolve cleanly during `migrate:fresh` +- Store `last_imported_filename` as nullable text metadata on `macros`; no separate import log table for this task diff --git a/.sisyphus/notepads/macros-and-labels-import/learnings.md b/.sisyphus/notepads/macros-and-labels-import/learnings.md index 3ccbbc5..f7252aa 100644 --- a/.sisyphus/notepads/macros-and-labels-import/learnings.md +++ b/.sisyphus/notepads/macros-and-labels-import/learnings.md @@ -41,3 +41,105 @@ ### Critical Decisions ### Migration/Test Notes - `tests/Pest.php` already applies `RefreshDatabase` to all `Feature` tests; no extra setup needed for `Feature/Migrations` - SQLite unique constraint errors can be asserted with `->toThrow(\Exception::class)` in migration tests +- `macro_collection_macros` can safely use a reserved-ish `order` column name in SQLite/Laravel migrations; the schema + foreign keys passed `migrate:fresh` +- `foreignId()->constrained()->cascadeOnDelete()` correctly cascades through the junction table under the current sqlite test setup + +## T2.1 — Models + Factories (label-based schema) + +- **All new model conventions match house style**: `$fillable` array, `casts()` method (not `$casts` property), typed return types on relations. +- **`hidden_at` semantics, NOT SoftDeletes**: `Label` and `Macro` use `hidden_at` timestamp + `isHidden()` helper; SoftDeletes deliberately not used. +- **`MacroCollection` pivot ordering**: `belongsToMany(Macro::class, 'macro_collection_macros')->withPivot('order')->orderBy('macro_collection_macros.order')` — must qualify the column with the pivot table name to avoid SQLite ambiguous column errors. +- **`ServiceMacroOverride::assignments()` uses composite-key relation**: HasMany on `service_id` with explicit `where('part_type', $this->part_type)` filter (Eloquent has no native composite-FK support). +- **`SongArrangement::arrangementLabels()` ordered**: `hasMany(SongArrangementLabel::class)->orderBy('order')` so consumers see labels in the correct slide order without re-sorting. +- **`SongArrangementLabelFactory`** uses `Label::factory()` and `SongArrangement::factory()` directly — both have HasFactory trait. +- **Test gating**: After T2.1 alone, 270/328 tests pass. The remaining 58 failures are all in `app/Services/SongService.php`, `app/Services/ProImportService.php`, and the test files that exercise those services; T4.4 owns those updates. +- **`DatabaseSchemaTest`** passes cleanly (3 tests / 31 assertions): all expected tables exist, dropped tables gone, all factories produce valid rows. + +## T4.4 — PHP rename audit (2026-05-03) + +After Wave 2's schema migration (`song_groups` → `labels`, `song_arrangement_groups` → `song_arrangement_labels`), the rename-audit cleanup turned out to span **far more files** than the plan listed (12 app files + 11 test files vs 7 listed). Key findings: + +- `Song::groups()` relation was completely removed; many call sites needed adaptation, not just rename. New pattern: traverse `Song -> arrangements -> arrangementLabels -> label -> songSlides` for content. +- `song_slides` table only has `label_id` (no `song_id` either) — slides are now globally owned by labels. Tests that previously did `$verse = $song->groups()->create(...)` need to find/create a global Label and link it via `SongArrangementLabel`. +- Helper functions defined at file level in Pest tests work cleanly: `function makeSongWithDefaultArrangement(): array { ... }` keeps test setup DRY. +- Fixture `Test.pro` has 4 groups but only 3 are referenced in any arrangement — assertion needs to count `Label::count()` (post-import) to verify "all 4 groups created", not arrangement labels. +- `MacroColorConverter::fromRgba()` (assoc-keyed `r,g,b`) replaces the old `ProImportService::rgbaToHex()` for label color conversion in importer; the legacy hex helpers were preserved because `ProFileGenerator::colorFromArray` uses numeric-indexed RGBA. +- Removing the "groups must belong to this song" check in `ArrangementController::update` is correct since labels are global; `exists:labels,id` validation is sufficient. + +## Wave 2 — T2.3, T2.4, T2.5 (services) + +### LabelsImportService +- Case-insensitive name lookup via `whereRaw('LOWER(name) = ?', [strtolower($name)])` +- Always updates color on existing labels (additive policy, never disables) +- Skips labels with empty names +- Stores metadata in `settings` table: `labels_last_imported_at`, `labels_last_imported_filename` + +### MacrosImportService +- UUID is normalized to UPPER before storage (matches parser convention) +- Macros not in file get `hidden_at = now()` (soft-disable, not delete) +- Re-import re-enables a previously hidden macro by setting `hidden_at = null` +- Tracks `wasHidden` to differentiate `reEnabled` vs `updated` counts +- Collection sync: detach all → attach with order index from parser +- Warnings: any MacroAssignment whose macro is currently hidden + +### MacroResolutionService +- Override-vs-defaults: `ServiceMacroOverride` existence check decides whether to use service-specific or global assignments +- Hidden macros and hidden labels (for `by_label`) are filtered via Collection->reject() +- `macrosForSlide` uses match() expression for position semantics +- Default collection fallback: `--MAIN--` with UUID `8D02FC57-83F8-4042-9B90-81C229728426` + +### Pint quirk +- DTO classes with empty body need `{}` on same line as constructor closing paren — `single_line_empty_body` rule. + +### Test patterns +- Pest auto-applies `RefreshDatabase` via `tests/Pest.php` for all Feature tests, but explicit `uses(RefreshDatabase::class)` is harmless and matches spec. +- All 354 tests pass (was 334 before Wave 2.3-2.5). + +## T2.7 ProExportService MacroResolutionService +- ProPresenter parser package currently consumes only `$slideData['macro']` in `ProFileGenerator::buildCue()`; no `$slideData['macros']` stacking support exists. `Slide::setMacro()` also updates/replaces the first macro action. +- `ProExportService` now keeps song downloads backward-compatible by accepting optional `?Service`; exports without service context intentionally emit no macros. +- Playlist/bundle service exports must pass the active `Service` into `generateProFile()` / `generateParserSong()` so `MacroResolutionService::macrosForSlide()` can resolve global or service-specific assignments. +- Full verification for T2.7: `ddev exec php artisan test` passed with 357 tests / 1706 assertions; evidence in `.sisyphus/evidence/task-2.7-pest.txt`. + +## T2.8 Controllers + Routes (2026-05-03) + +- **4 thin controllers, all JSON responses for mutations** (Inertia only on `MacroAssignmentController::index`). +- **Validation via inline `$request->validate()`** with `in:` lists for `part_type` (information, moderation, sermon, song, agenda_item) and `position` (all_slides, first_slide, last_slide, by_label). +- **Route ordering matters**: `/settings/macro-assignments/reorder` MUST be registered BEFORE `/settings/macro-assignments/{macroAssignment}`, else `reorder` is captured as the model parameter. +- **Route-model binding works automatically** for both `{macroAssignment}` and `{serviceMacroAssignment}` — Laravel resolves snake_case → StudlyCase → Eloquent model. +- **Unused `$service` parameter on update/destroyAssignment** is intentional: route-model binding requires it in the signature even if the assignment binding alone does the work. +- **Generic 422 message** for parser failures hides internal exception details from users; all messages German Du-form. +- **Test fixtures `tests/fixtures/macros-sample.bin` & `labels-sample.bin`** work with `new UploadedFile(path, name, null, null, true)` (5th arg `$test=true` keeps the file at original path so `getPathname()` returns the fixture). +- **`UploadedFile::fake()->create('x.bin', 1)`** generates a 1KB empty file that fails parser parsing → triggers the controller's catch block → 422 JSON. +- **Auth tests use plain `post()` (form-data) → `assertRedirect(route('login'))`**; JSON requests would return 401, but session-based auth redirects. +- **Final test count: 376 (was 357) → +19 new tests / +54 assertions.** + +## T4.2: Service Edit Macro Panel + +- `ServiceController::edit()` now passes `macros_per_part` keyed by part_type (information, moderation, sermon, song, agenda_item). +- Each entry: `count`, `is_overridden`, `has_warning`, `assignments[]` (with macro_id/name/color/hidden, position, label_id/name). +- Uses `MacroResolutionService::resolveAssignmentsForPart()` (already filters hidden macros + by_label with hidden labels). `has_warning` checks raw flags before resolver filters them — but since resolver already filters, `has_warning` will normally be false. Acceptable for badge UI. +- `ServiceMacroOverride::where(...)->exists()` checks override status per part. +- `ServicePartMacroPanel.vue` is positioned `absolute right-0 top-8 z-50` — wrapper must be `class="relative"`. +- Edit.vue page only has 2 visible block headers (Ablauf and Information). Placed agenda_item/moderation/sermon/song MacroIcons in the Ablauf header row; placed information MacroIcon in the Information block header. +- MacroIcon renders only when `count > 0`, so empty parts gracefully hide their badge. +- Routes used: `services.macro-overrides.store` (POST + body `{part_type}`), `services.macro-overrides.destroy` (DELETE + body). XSRF token sourced from `XSRF-TOKEN` cookie (URL-decoded). + +## Final Verification F4 (2026-05-04) + +- Scope-fidelity verification passed: `Label`/`Macro` use `hidden_at` (no SoftDeletes), label imports are additive with color overwrite, missing macros are hidden via `hidden_at`, `MacroResolutionService` resolves override/default assignments and filters hidden macros/labels, `ProExportService` injects `MacroResolutionService` with no legacy `buildMacroData()`, and `SettingsController` only exposes the four `AGENDA_KEYS`. +- Forbidden-pattern grep suite returned no output for label CRUD, macro action runner/editor patterns, TS suppressions, Vue console logs, bulk operations, label/macro drag reorder, and export caching. + +## [2026-05-04] Session follow-up — hidden label badge + nullable import color + +- `MacroAssignments.vue` should mirror hidden-macro warnings for `by_label` rows with `a.label?.hidden_at`, using a red badge and `data-testid="warning-hidden-label"`. +- `ProImportService` must keep new label colors nullable: `MacroColorConverter::fromRgba($color)` should flow through unchanged so missing `.pro` colors become `NULL`, not `#808080`. + +## 2026-05-04 F1 final compliance audit +- Final verification commands passed: no Vue song_group_id references, MacroIcon and hidden-label test IDs present, ProImportService #808080 fallback removed, required macro/label deliverables and schema present, Label/Macro use hidden_at without SoftDeletes, routes present, ProBundleExportService resolves macros for information/moderation/sermon and agenda_item exports. +- MacroResolutionService supports part types dynamically via part_type string; grep for literal part names can be empty without indicating non-support. + +## 2026-05-04 F4 scope fidelity check +- Must-NOT grep suite found one historical toast pattern in pre-existing service/song Vue files; no macro/label feature-specific forbidden patterns were found (no SoftDeletes/deleted_at, drag UI, runner/preview, bulk ops, optimistic markers, collection assignment, export caching, agenda_item slide enum, TS suppressions, or console.log). +- Required macro/label evidence present: all 5 part types, position enum including by_label, hidden_at semantics, explicit restrict/cascade FKs, stacking resolver (`filter` → `map` → `values` → `all`), bundle export macro injection, German UI labels, and test IDs on macro/label picker/icon components. +- Unaccounted grep output for `ArrangementConfigurator.vue`, `ArrangementDialog.vue`, and `SongEditModal.vue` is explained by planned SongGroup → Label rename work, not unrelated scope creep.