summaryrefslogtreecommitdiffstats
path: root/docs/refactor-groups.md
diff options
context:
space:
mode:
Diffstat (limited to 'docs/refactor-groups.md')
-rw-r--r--docs/refactor-groups.md72
1 files changed, 69 insertions, 3 deletions
diff --git a/docs/refactor-groups.md b/docs/refactor-groups.md
index 3d7412b..f94a18b 100644
--- a/docs/refactor-groups.md
+++ b/docs/refactor-groups.md
@@ -1,14 +1,14 @@
# Groups Refactor — Per-Root Permissions & App Plugin Architecture
-> Status: **Phase 1 complete and reviewed** (2026-09-06). Phase 2 and 3 not started.
+> Status: **Phases 1 and 2 complete** (2026-09-06). Phase 3 not started.
>
> This is the most significant refactoring of the project. It changes how roots
> are permissioned, how group applications are configured, and how the Settings
> and Create Group pages are structured.
>
> §7b records what the review of Phase 1 found and how the plan below was wrong
-> where it was wrong. Read it before starting Phase 2 — two of its entries are
-> rules the later phases have to follow, not one-off fixes.
+> where it was wrong; §7c does the same for Phase 2. Read both before starting
+> Phase 3 — several entries are rules rather than one-off fixes.
---
@@ -752,3 +752,69 @@ configured directory.
- `test_ops.py::test_a_backslash_path_written_into_node_toml_stays_parseable`
fails on any non-Windows machine and always has — it builds a
`PurePosixPath` from a Windows path. Unrelated to this refactor, left alone.
+
+---
+
+## 7c. Phase 2 as built (2026-09-06)
+
+The plan held. Four things were done differently, and one of them is a rule.
+
+### The rule
+
+**A settings key added to the client must be added ten times.** `test_locales`
+holds the nine other catalogues to `en.js`, so a missing key is a failing test
+rather than a silent gap — but Phase 2 added 27 keys, and doing them one file
+at a time is how the Phase 1 gap happened. Write the table, generate the
+insert.
+
+**And a second one, which cost a bug in this phase:** `node --check foo.js`
+does **not** reliably report a module syntax error. It accepted a file with
+`${/* ... */''}` — htm template syntax, pasted into a plain object literal —
+and reported success. Copying to `.mjs` first forces the module parser, which
+reports it. `test_spa_syntax.py` now does that for every module; the suite had
+no syntax check at all before, which is how the file was committed.
+
+### Done differently
+
+- **No migration script.** The plan (§4.3) called for one to rename
+ `video_root` → `video_directories` in `roster.db`. Instead the roster falls
+ back to the old key when the new one is unset, and the first save through the
+ new path leaves it behind. A script that has to be run by hand on the machine
+ where it matters is a step that does not happen; a fallback is one that
+ cannot be skipped.
+- **`music`, not `audio`.** The app's registry key was `music` while its
+ storage said `audio_root` and its ops said `set_audio_root`. One identifier
+ per app now — the registry key — with the correspondence in exactly one
+ table (`Roster.LEGACY_DIR_KEYS`).
+- **One storage shape.** `set_app_directory` (single) writes a one-element
+ list, so there is no scalar form anywhere below the wire. `video_root` and
+ friends survive on the handshake ack only, *derived* from the list rather
+ than stored beside it — a second stored value drifts within one run, which
+ reads as "it works after a restart".
+- **The panes call the transport themselves.** The plan had every pane report
+ through one `onSave`, which would have made the page a dispatcher naming
+ every app's settings keys — the thing the phase exists to remove. The line
+ is: what every app has (directories) the page does, generically; what one
+ app alone has (a TMDB key, a link-preview switch) the pane does with the
+ transport it is handed. An app that only wants directories touches neither
+ file, which is `test_app_settings_plugin.py`'s subject.
+
+### Worth knowing
+
+- `settings-ui.js` exists because `group-settings` → `apps` → a pane →
+ `group-settings` is an import cycle, and ES modules answer that with a
+ temporal-dead-zone `ReferenceError` at first render — the component simply
+ does not appear, which is the fault already recorded in CLAUDE.md about hook
+ ordering. The shared widgets live outside both.
+- The folder picker asks the node for **nothing**. The tree is derived from
+ paths the client already holds, so it shows what the group's index contains
+ and no more — a folder the node never indexed does not exist as far as the
+ group is concerned. There is no folder-browsing protocol and this does not
+ add one.
+- `_ASSETS` in `webapp.py` had to grow by six. Modules reached through the
+ registry rather than imported by name are exactly the ones nothing else would
+ notice changing, and a stale one is served from a browser cache with no
+ version bump. `test_asset_versioning` caught it.
+- **What Phase 3 still owes:** the HelloWorld app (§4.1) — which is the actual
+ proof of the above, since every test here reads source rather than adding an
+ app and watching it work — plus the CLI polish and the Windows pass.