146 lines
14 KiB
Markdown
146 lines
14 KiB
Markdown
# Learnings — macros-and-labels-import
|
|
|
|
## [2026-05-03] Session ses_210cd1557ffeGs4SEGrt7hnvyS — Plan Created
|
|
|
|
### Parser Library
|
|
- Source at `/Users/thorsten/AI/propresenter/src/` (NOT `/Users/thorsten/AI/propresenter-work/php/` per stale AGENTS.md)
|
|
- VCS repo: `https://git.stadtmission-butzbach.de/public/propresenter-php.git` (dev-master)
|
|
- New classes (NOT yet in vendor/): `MacrosFileReader`, `LabelsFileReader`, `Macro`, `MacroLibrary`, `MacroCollection`, `Label`, `LabelLibrary`
|
|
- `MacrosFileReader::read(string $filePath): MacroLibrary` — raw protobuf binary, no extension
|
|
- `LabelsFileReader::read(string $filePath): LabelLibrary` — same
|
|
- `Label::getName()` returns protobuf `text` field — name is the identity (no UUID for labels)
|
|
- `Macro::getColor()` returns `?array{r,g,b,a}` floats 0..1 — need `MacroColorConverter` to get hex
|
|
- `Label::getColorHex()` already returns `#RRGGBB` — mirror its formula for macros
|
|
- **PHP 8.4 required** by parser. App currently requires `^8.2` — BLOCKER for T0.1
|
|
|
|
### DB Schema Key Facts
|
|
- `slides.type` enum is `[information, moderation, sermon]` ONLY — no `agenda_item`
|
|
- `agenda_item` part_type = slide where `service_agenda_item_id IS NOT NULL` at runtime
|
|
- `song_groups.color` is NOT NULLABLE (migration says so) — new `labels.color` IS nullable
|
|
- `service_songs.song_id` is `cascadeOnDelete` — wiping `songs` auto-cascades to `service_songs`
|
|
|
|
### Export Flow
|
|
- `ProExportService::buildGroups()` lines 38-69 — macro injection point
|
|
- `ProExportService::buildMacroData()` lines 71-86 — reads 4 legacy settings keys
|
|
- Currently injects macro ONLY when group name is "COPYRIGHT" (case-insensitive)
|
|
- `ProImportService::import(UploadedFile $file): array` — method signature (NOT `importFromFile`)
|
|
|
|
### Settings Pattern
|
|
- `Setting::get($key, $default)` / `Setting::set($key, $value)` — simple key/value
|
|
- `settings` table: `key UNIQUE, value TEXT`
|
|
|
|
### Critical Decisions
|
|
- song_groups → labels: global table, "drop all data" migration (no backwards compat)
|
|
- Hybrid macro scope: global defaults in Settings; per-(service, part_type) override via "Anpassen"
|
|
- Override = snapshot of globals at creation time; future global changes don't propagate
|
|
- Stacking: all matching assignments fire, ordered by `macro_assignments.order ASC`
|
|
- Hidden macros/labels: skip at export, warning badge in editor
|
|
- Label colors: read-only in UI; Labels file import is sole authority; .pro auto-discovery only sets color on CREATE
|
|
- FK rules: `restrictOnDelete` on macro/label refs (use `hidden_at`); `cascadeOnDelete` on service-scoped rows
|
|
|
|
### 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.
|