Story 3.3 done

2026-05-25 10:32:49 +02:00
parent 7b56d62563
commit 748c7d7f85
12 changed files with 451 additions and 105 deletions
@@ -729,3 +729,29 @@ All acceptance criteria (AC-1 through AC-9) are satisfied except AC-9 (README do
 ---

 *This story file was created using the BMad Method Ultimate Context Engine. The developer now has everything needed for flawless implementation.*
+
+---
+
+### Review Findings
+
+#### decision-needed
+
+- [x] [Review][Decision] AC-8 merge success message format differs from spec — Spec says `"Imported N presets (M new, K replaced)"` but implementation correctly outputs `"Imported X presets (Y new, Z skipped as duplicates)"`. **Resolution**: Update spec AC-8 to match implementation — merge mode skips duplicates, never replaces.
+
+#### patch
+
+- [x] [Review][Patch] PresetExport filename mismatch preview vs actual download [`src/ui/gm/PresetExportDialog.js`]
+- [x] [Review][Patch] World name fallback inconsistency between `exportAllPresets` and `generateExportFilename` [`src/core/PresetImportExportManager.js:84` vs `:109`]
+- [x] [Review][Patch] Hardcoded notification strings bypass localization keys defined in en.json [`src/ui/gm/PresetExportDialog.js:_onExport`, `src/ui/gm/PresetImportDialog.js:_onImport`]
+- [x] [Review][Patch] `_onExport` double-calls `adapter.scenes.current()` [`src/ui/gm/PresetExportDialog.js:73-74`]
+- [x] [Review][Patch] `success` flag not recomputed after merging extraction errors into result [`src/core/PresetImportExportManager.js:341-347`]
+- [x] [Review][Patch] Import preview marks merge duplicates as invalid (red X) instead of skipped (yellow) [`src/ui/gm/PresetImportDialog.js:_parseAndPreviewFile`]
+- [x] [Review][Patch] Regex `\s` in preset name character validation allows newlines/tabs [`src/core/PresetImportExportManager.js:238`]
+- [x] [Review][Patch] No concurrency guard on async `_parseAndPreviewFile` — rapid file selection races [`src/ui/gm/PresetImportDialog.js:_parseAndPreviewFile`]
+- [x] [Review][Patch] Import preview doesn't validate each preset structure with `isValidScenePreset()` [`src/ui/gm/PresetImportDialog.js:_parseAndPreviewFile`]
+- [x] [Review][Patch] Export dialog renders when no active scene (then fails on export) [`src/ui/gm/PresetExportDialog.js`]
+- [x] [Review][Patch] Import mode labels hardcoded as `'Merge'`/`'Replace'` instead of localized [`src/ui/gm/PresetImportDialog.js:_prepareContext`]
+
+#### defer
+
+- [x] [Review][Defer] Replace mode rollback can fail partially leaving corrupted state [`src/core/PresetImportExportManager.js:434-448`] — deferred, browser env lacks transactions; existing error reporting is reasonable
@@ -60,3 +60,7 @@
 - [ ] 5MB MAX_PORTRAIT_SIZE vs ~50KB Foundry flag limit — documented design limitation; flag limit is server-dependent and can't be changed in code
 - [ ] No magic-byte file content validation — spec mentions "MIME type AND file content" but only format/MIME check implemented; enhancement for future
 - [ ] No animated-vs-static GIF distinction — FR-26 requires static GIF only but MIME-type alone can't distinguish; requires binary GIF parsing
+
+## Deferred from: code review of 3-3-preset-import-and-export (2026-05-26)
+
+- [ ] Replace mode rollback can fail partially leaving corrupted state — Browser env lacks transaction support; existing error reporting is reasonable but state can be partially deleted/restored when both delete and rollback fail. [`src/core/PresetImportExportManager.js:434-448`]
@@ -0,0 +1,16 @@
+# Blind Hunter — Code Review: Story 3.3 (Preset Import & Export)
+
+You are the **Blind Hunter**. You receive ONLY the diff below. No project context, no spec file, no documentation.
+
+Your job: Find bugs, security vulnerabilities, logic errors, code smells, and anti-patterns. Be adversarial. Assume nothing.
+
+## Rules
+- Output findings as a Markdown list with severity labels: `CRITICAL`, `HIGH`, `MEDIUM`, `LOW`
+- Each finding: one-line title + evidence from the diff
+- If nothing is found, output: `No findings.`
+
+## Diff
+
+```
+[PASTE THE FULL DIFF HERE]
+```
@@ -0,0 +1,41 @@
+# Edge Case Hunter — Code Review: Story 3.3 (Preset Import & Export)
+
+You are the **Edge Case Hunter**. You receive the diff AND read access to the project.
+
+Your job: Walk every branching path and boundary condition in the diff. Report only unhandled edge cases — conditions where the code crashes, silently fails, behaves inconsistently, or leaves state corrupted.
+
+## Rules
+- Output findings as a Markdown list with severity labels: `CRITICAL`, `HIGH`, `MEDIUM`, `LOW`
+- Each finding: one-line title + reproduction path + evidence from the diff
+- Only report genuine unhandled edge cases, not theoretical impossibilities
+- For non-trivial findings, suggest how to reproduce
+
+## Diff
+Files changed for Story 3.3 (Preset Import & Export):
+
+```
+src/core/PresetImportExportManager.js       (NEW - 463 lines)
+src/ui/gm/PresetExportDialog.js             (NEW - 200 lines)
+src/ui/gm/PresetImportDialog.js             (NEW - 436 lines)
+templates/preset-export.hbs                 (NEW - 29 lines)
+templates/preset-import.hbs                 (NEW - 90 lines)
+styles/components/_preset-import-export.less (NEW - 403 lines)
+tests/unit/core/PresetImportExportManager.test.js (NEW - 476 lines)
+src/ui/gm/DirectorsBoard.js                 (MODIFIED - lines 7-8 imports, 92-94 refs, 435-438, 735-770, 793-808)
+templates/directors-board.hbs               (MODIFIED - added export/import buttons)
+lang/en.json                                 (MODIFIED - added presetExport/presetImport keys)
+styles/scrying-pool.less                    (MODIFIED - added _preset-import-export import)
+```
+
+Full diff file: `/tmp/opencode/story-3.3-diff.txt`
+
+## Project Access
+Root: `/home/morr/work/foundryvtt/video-view-manager/`
+
+Key reference files to read as needed:
+- `src/contracts/scene-preset.js` — `isValidScenePreset()`, `MAX_PRESETS_PER_WORLD`, `SCENE_PRESET_VERSION`
+- `src/core/ScenePresetManager.js` — `list()`, `save()`, `delete()`, `get()` methods
+- `src/foundry/FoundryAdapter.js` — adapter surface used by the new code
+- `src/utils/html.js` — `escapeHtml()` helper
+
+Walk every path in the diff and report any unhandled edge cases.
@@ -0,0 +1,47 @@
+# Acceptance Auditor — Code Review: Story 3.3 (Preset Import & Export)
+
+You are the **Acceptance Auditor**. Review this diff against the spec and context docs.
+
+Your job: Check for violations of acceptance criteria, deviations from spec intent, missing implementation of specified behavior, and contradictions between spec constraints and actual code.
+
+## Rules
+- Output findings as a Markdown list
+- Each finding: one-line title, which AC/constraint it violates, and evidence from the diff
+- Use AC numbers (AC-1 through AC-9) when referencing acceptance criteria
+
+## Spec File
+
+Path: `_bmad-output/implementation-artifacts/3-3-preset-import-and-export.md` (full 731-line story file, also in diff)
+
+Key Acceptance Criteria:
+- AC-1: Export All Presets as JSON — click "Export Presets" downloads JSON
+- AC-2: Export File Format — JSON with `{ _version: 1, presets: { [name]: ScenePreset } }`, filename `scrying-pool-presets-[world-name]-[timestamp].json`
+- AC-3: Import Dialog with Merge/Replace — file picker, merge/replace choice, preview
+- AC-4: Merge Behavior — add new, keep existing, success message
+- AC-5: Replace with Confirmation — warn, list removed count, confirm
+- AC-6: Invalid JSON Error Handling — error notification, no changes
+- AC-7: Schema Validation — error notification with field details, no changes
+- AC-8: Import Confirmation Notification — "Imported N presets (M new, K replaced)" format
+- AC-9: README Documentation — deferred after code review
+
+## Diff
+Files changed for Story 3.3:
+
+Full diff path: `/tmp/opencode/story-3.3-diff.txt`
+
+New files:
+- `src/core/PresetImportExportManager.js` — Core import/export logic
+- `src/ui/gm/PresetExportDialog.js` — Export dialog
+- `src/ui/gm/PresetImportDialog.js` — Import dialog with merge/replace
+- `templates/preset-export.hbs` — Export template
+- `templates/preset-import.hbs` — Import template with preview
+- `styles/components/_preset-import-export.less` — Dialog styles
+- `tests/unit/core/PresetImportExportManager.test.js` — 38 tests
+
+Modified files for 3.3:
+- `src/ui/gm/DirectorsBoard.js` — Import/Export button handlers
+- `templates/directors-board.hbs` — Export/Import buttons in footer
+- `lang/en.json` — Localization keys
+- `styles/scrying-pool.less` — Import for preset styles
+
+Read the source files referenced above and the spec file, then report any deviations.