SluijsensandClaude Sonnet 5
0447993181
Frontend (Unit 2 completion): dual dev-server tooling (pnpm dev:slave, pnpm dev:all), per-instance browser tab titles, and a backend capability check (SystemController + useSystemCapabilities + ModuleGuard) so a Master-only page is hidden on a slave instance instead of assuming every backend has every module. Master/slave protocol fixes surfaced by actually running master and slave side by side locally: - Deactivating a CMS instance (Inactive) now releases the slave's master gate instead of leaving it stuck on its last pushed status. - The periodic integrity check now also re-pushes status to every reachable slave (previously URL-verification only) and runs once immediately on startup. - Added the originally-specified (but never implemented) slave-pull path: a slave now periodically polls its own status from the master (GET /api/v1/SlaveStatus) and fails open to Available if the master is unreachable for too long, complementing the existing push. - The slave's own Settings page can no longer "successfully" change local availability while the master controls it; it's now locked with an explanatory banner and the backend rejects the write with 409 instead of silently no-op'ing it. - CMS instance status badges now match the dashboard's color/icon styling instead of a plain grey badge. Also corrected the master-cms-module design docs to match this as-built behavior, and flagged (without a full rewrite) a larger, pre-existing divergence between its inception-stage application design and what construction actually built. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
64 lines
6.1 KiB
Markdown
64 lines
6.1 KiB
Markdown
# Tech Debt Backlog
|
|
|
|
Items discovered incidentally while working on other features. Not yet prioritized. Each item lists where it was found and what "done" looks like.
|
|
|
|
---
|
|
|
|
## TD-001: Dead config properties on `MasterModuleOptions`
|
|
|
|
**Found during**: `master-cms-module` — Build & Test / appsettings mapping (2026-07-01)
|
|
|
|
**Location**: `src/SlpModularCms.Modules.Master/Options/MasterModuleOptions.cs`
|
|
|
|
**Issue**: `CacheMinutes` and `ApiKey` are declared on the options POCO and were originally designed (per `aidlc-docs/features/master-cms-module/construction/master-backend/functional-design/domain-entities.md`) as slave-side settings for Unit 2 (slave-availability-extension) to consume. When Unit 2 was actually implemented, it used a different pattern instead — per-instance Data-Protection-encrypted API keys pushed from the Master, and a no-TTL in-memory cache (`REL-02` in the Availability NFR requirements) rather than a configurable `CacheMinutes`. As a result, neither property is read anywhere in the codebase.
|
|
|
|
**Done looks like**: Either remove both properties from `MasterModuleOptions` (and any corresponding appsettings documentation), or — if a future feature revives the pull/TTL model — wire them up for real. Confirm via `grep -rn "CacheMinutes\|Options.Value.ApiKey" src/` that nothing reads them before removing.
|
|
|
|
---
|
|
|
|
## TD-002: Pre-existing frontend lint errors (react-hooks plugin)
|
|
|
|
**Found during**: `master-cms-module` — Build & Test (2026-07-01), confirmed pre-existing via `git worktree` checkout of commit `c156107` (the commit immediately before this feature's frontend work) with a clean `npm install`
|
|
|
|
**Issues** (4 total — 4 errors, 0 warnings at that baseline):
|
|
1. `frontend/src/components/ui/badge.tsx:32` — `react-refresh/only-export-components`: the file exports both a component and a non-component value (likely `badgeVariants`), breaking Fast Refresh. Fix: move the shared constant/function to a separate file.
|
|
2. `frontend/src/components/users/InviteUserDialog.tsx` — two related findings in the same `useEffect`:
|
|
- `react-hooks/set-state-in-effect`: `setStep(1)` (and sibling `setState` calls) run synchronously inside a `useEffect` that resets dialog state on close.
|
|
- `react-hooks/immutability`: `reset()` (from `useForm`) is referenced in the effect body before its `const { reset } = useForm(...)` declaration further down the component — works today due to hoisting/closure timing but is flagged as fragile.
|
|
3. `frontend/src/pages/SettingsPage.tsx:39` — `react-hooks/set-state-in-effect`: `setSelectedMode`/`setReason` run synchronously inside a `useEffect` that syncs local form state from a query result (`availability`).
|
|
|
|
**Done looks like**: Refactor each flagged effect per the React docs' "you might not need an effect" guidance (react.dev/learn/you-might-not-need-an-effect) — e.g. compute derived state during render, or move the reset logic into the `onOpenChange`/event handler instead of an effect. For `badge.tsx`, split the non-component export into its own module. Re-run `npm run lint` in `frontend/` until clean.
|
|
|
|
---
|
|
|
|
## TD-003: New lint findings introduced by `master-cms-module` Unit 3 (same pattern as TD-002)
|
|
|
|
**Found during**: `master-cms-module` — Build & Test (2026-07-01). These were introduced by this feature's own Unit 3 (frontend-cms-page) code, but deferred here per explicit user decision (2026-07-01: "Laat ze staan... gooi die dan ook in deze feature") rather than fixed in-flight, to avoid blocking Unit 4.
|
|
|
|
**Issues** (2 total — 1 error, 1 warning):
|
|
1. `frontend/src/components/cms/AddCmsInstanceDialog.tsx:55` — `react-hooks/set-state-in-effect`: `setServerError(null)` (and sibling calls) run synchronously inside the dialog's close-reset `useEffect`. Same pattern as TD-002 item 2.
|
|
2. `frontend/src/components/cms/SetStatusDialog.tsx:72` — `react-hooks/incompatible-library` (warning): `watch()` from `react-hook-form`'s `useForm()` is used directly; React Compiler can't safely memoize it, so compilation is skipped for this component/hook.
|
|
|
|
**Done looks like**: Same remediation approach as TD-002 item 2 for the `AddCmsInstanceDialog.tsx` finding. For `SetStatusDialog.tsx`, either accept the compiler skipping memoization for this component (likely fine, low-traffic dialog) or replace `watch('status')` with a subscription-based pattern (`useWatch`) if it becomes a proven perf issue.
|
|
|
|
---
|
|
|
|
## TD-004: Review copy for the master-controlled availability lock on `SettingsPage`
|
|
|
|
**Found during**: `local-dev-master-slave-setup` — Build & Test follow-up fix (2026-07-04), user flagged the wording right after it was added but didn't have a replacement in mind yet.
|
|
|
|
**Location**:
|
|
- `frontend/src/pages/SettingsPage.tsx` (locked-state banner + disabled controls)
|
|
- `frontend/src/i18n/locales/nl/translation.json` and `en/translation.json`, keys `settings.availability.masterControlledTitle`, `settings.availability.masterControlled`, `settings.availability.masterControlledSaveError`
|
|
|
|
**Issue**: When a slave's availability is controlled by its Master CMS, the Settings page now shows a banner ("Beheerd door Master-CMS" / "Deze instantie is door de Master-CMS uitgeschakeld...") and disables the mode buttons, reason field, and save button, plus a specific toast on a 409 conflict ("Kan niet worden gewijzigd: de Master-CMS beheert deze status."). The behavior (locking + explaining why) is correct and intentional; only the exact phrasing needs a revisit — user wants different wording but hadn't decided on it at the time.
|
|
|
|
**Done looks like**: Update the three translation keys above (NL and EN) to the reviewed copy. No code/logic changes expected — this is copy-only. Re-run `pnpm test` for `SettingsPage.test.tsx` afterward (its assertions match on text via regex, e.g. `/master cms controls this status/i`, so wording changes will need matching test updates too).
|
|
|
|
---
|
|
|
|
## Notes
|
|
|
|
- None of these block functionality — `npm run build` and all test suites pass regardless.
|
|
- TD-002 and TD-003 share the same underlying pattern (`setState` in a mount/close-reset effect) — worth fixing as one pass across all four files rather than one-by-one, once picked up.
|