diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-06 19:03:22 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-06 19:03:22 +0200 |
| commit | ab44526a291fa673aa2850d105f6412a70a5341f (patch) | |
| tree | 5940f18acfc15fc732eb65d90de346920461b8c8 /docs/refactor-groups.md | |
| parent | 85a2ec47b7ad334208a3dbb091fadccc7631785c (diff) | |
| download | meshbay-ab44526a291fa673aa2850d105f6412a70a5341f.tar.gz | |
feat(client): Phase 2 — per-app settings panes, folder tree, multi-directory
Each app's settings were inlined in `group-settings.js` — TMDB, MusicBrainz,
and one folder picker per app, each with its own draft state and save handler
saying the same thing about a different key. They are one file per app now,
reached through the `apps.js` registry, and the page that renders them names no
application at all: adding one is a registry entry and a settings file.
The line between the two is what makes that true. What every app has — folders
— the page does generically, through one `saveDirectories` bound to the app.
What one app alone has, its pane does itself with the transport it is handed.
An app that only needs directories touches neither `group-settings.js` nor
`group-page.js`, which is `test_app_settings_plugin.py`'s subject.
`settings-ui.js` exists because a pane importing the page that renders it is a
cycle, and ES modules answer that with a temporal-dead-zone ReferenceError at
first render — a component that silently does not appear, the fault already
recorded in CLAUDE.md about hook ordering.
The flat depth-indented `<select>` of every folder in the library becomes a
modal tree. It asks the node for nothing: the tree is derived from paths the
client already holds, so it shows exactly what the group's index contains and
adds no folder-browsing protocol. For Chat's attachment folder — the one
directory that is written to rather than read — read-only roots are greyed
out, so the node's refusal arrives before the operator picks rather than when
somebody sends a file.
Videos and Music take a list of folders. A library on two drives could not be
described before; the only recourse was pointing the app at a parent containing
both, which pulls in everything else under it. The scalar shapes survive on the
wire alone, for a node speaking MNP 1.0, and the client reads them as a
one-element list.
Two things the tests caught that I would not have:
`test_asset_versioning` — six new modules were missing from `_ASSETS`. Reached
through the registry rather than imported by name, they are exactly the files
nothing else would notice changing, and a stale one is served from cache with
no version bump.
And `node --check foo.js` does **not** reliably report a module syntax error:
it accepted `${/* ... */''}` — htm template syntax pasted into a plain object
literal — and reported success. A `.mjs` copy forces the module parser and
reports it. The suite had no syntax check at all, which is how that reached a
file; `test_spa_syntax.py` does it for every module now, and pins that the
loose path is not what it uses.
Suite: 12 failures, all pre-existing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011pvMdvLBG92jyhvD5pD6us
Diffstat (limited to 'docs/refactor-groups.md')
| -rw-r--r-- | docs/refactor-groups.md | 72 |
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. |