diff --git a/CLAUDE.md b/CLAUDE.md index c06c28e..554556b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -812,7 +812,7 @@ limiter = Limiter( ## 17. Alembic Migration Chain -**Current HEAD:** `phase40_support_chat_kb` (38 migrations total). +**Current HEAD:** `phase52_user_ui_theme` (51 migrations total). **Chain root:** `0003_add_user_active` — a guarded squashed baseline (MT-2) that recreates the full 25-table schema with INFORMATION_SCHEMA guards. The original baseline migrations (0001/0002/0003) were lost; this file restores the chain root so Alembic can build the revision map. `down_revision = None`. @@ -846,9 +846,31 @@ limiter = Limiter( → phase36_issue_work_orders → phase37_contract_recipients → phase38_facility_qr - → phase39_issue_handler_type → phase40_support_chat_kb ← HEAD + → phase39_issue_handler_type → phase40_support_chat_kb + → phase41_auditor_role → phase42_area_qr_token → phase43_schedule_plan_fields + → phase44_internal_handler → phase45_schedule_frequency_enum + → phase46_schedule_recurrence → phase47_schedule_end_date + → phase48_schedule_parent_inspection → phase49_followup_requested_by + → phase50_sched_acknowledged → phase51_external_inspector + → phase52_user_ui_theme ← HEAD ``` +`phase41` → `phase52` are the single-tenant feature-parity track — see +MULTI_TENANT_PLAN.md §12. Two notes on that tail: + +* **`phase51_external_inspector`** widens the `users.role` ENUM. It must be + deployed together with its code: MT tested `role == 'inspector'` literally in + ~80 places, and widening the ENUM alone drops external inspectors into the + *unscoped* branch, which is a cross-tenant data leak rather than a cosmetic + bug. Use `User.INSPECTOR_ROLES` / `user.is_inspector`, never a literal. +* **ST's `phase50_default_modern` is deliberately NOT ported.** It overwrites + every saved `ui_theme` preference, which in MT would run against every tenant + DB. Set `DEFAULT_UI_THEME=modern` per tenant instead. See + MULTI_TENANT_PLAN.md §12.3. + +Migration revision IDs must be **≤ 32 characters** to fit +`alembic_version.version_num VARCHAR(32)`. + ### phase40_support_chat_kb Creates three tables backing support chat persistence and the AI knowledge base: @@ -1734,7 +1756,57 @@ Row highlights: yellow = suspended, red = trial expired. --- -## 26. Coding Rules for AI Assistants +## 26. Web Portal Design (MT-16) + +Two designs share one set of page templates. + +* `base.html` is a **one-line dispatcher**: `{% extends jqc_layout %}`. Page + templates keep `{% extends "base.html" %}` and need no edits. +* `layouts/classic.html` is the original chrome, verbatim. + `layouts/modern.html` is the sidebar shell. +* `jqc_layout` comes from `inject_ui_theme()` in `app/__init__.py`, driven by + `users.ui_theme` with config `DEFAULT_UI_THEME` as the fallback. +* Per-page overrides live at `templates/modern/.html` and are indexed + once at boot. Look for `UI themes | modern overrides indexed: N` in the log — + `0` on a host that should have them means the directory did not deploy. + +**Do not move the template swap into the Jinja loader.** It lives in +`ThemedEnvironment.get_template()` so the template cache is keyed on the +*rewritten* name. A loader-level swap caches under the original name, so a +modern template can be served to a classic user — and in MT, where one worker +serves many tenants, across tenants. + +When adding a page: write it once as a normal template. Only add a +`modern/` override if the layout genuinely differs; styling alone is handled by +`static/css/theme_modern.css`, which is scoped to `body.jqc-modern`. + +--- + +## 27. Roles (MT-15) + +`users.role` ENUM: `admin`, `director`, `inspector`, `external_inspector`, +`project_manager`, `customer`, `auditor`. + +**Never test `role == 'inspector'`.** `external_inspector` (customer / +third-party inspectors) has identical capabilities and identical +`InspectorAssignment` scoping. Use: + +* `user.is_inspector` — true for both inspector roles; use for every capability + and scoping check +* `User.INSPECTOR_ROLES` — for `User.role.in_(...)` queries +* `user.is_external_inspector` — only where the two genuinely differ (display) +* `user.role_label` / `ROLE_LABELS` — for any role name shown in the UI + +A literal comparison sends external inspectors down the unscoped branch, where +`get_inspector_scope()` returns `None` and every downstream query drops its +facility filter. That is a cross-customer leak. + +External inspectors are **invited**, never given a password: `password_set=False` +plus an emailed 72-hour token, with `auth.resend_invite` for bounced invitations. + +--- + +## 28. Coding Rules for AI Assistants These rules apply to every change made to this codebase, without exception. diff --git a/MULTI_TENANT_PLAN.md b/MULTI_TENANT_PLAN.md index 4b68e8e..78c61aa 100644 --- a/MULTI_TENANT_PLAN.md +++ b/MULTI_TENANT_PLAN.md @@ -3,6 +3,8 @@ > **Audience:** AI assistants and developers extending JQC into a multi-tenant SaaS. > **Companion to:** `CLAUDE.md` (single-tenant architecture reference). > **Status:** MT-0 through MT-8 complete and deployed (MT-8 billing is flag-gated behind `BILLING_ENABLED`, default off). MT-9 (iOS multi-tenant) is fully pending — both the server-side discovery endpoints and the iOS client are unbuilt. +> +> **Feature parity with the single-tenant tree (MT-10 → MT-17): complete.** MT forked from ST before ST kept shipping, and that gap has now been closed phase by phase — see §12. The only deliberate divergence is ST's `phase50_default_modern`, which MT does not adopt (§12.3). Tenant migration head: `phase52_user_ui_theme`. --- @@ -328,4 +330,146 @@ python -m control.tenant_migrate bootstrap --tenant acme # fresh DB only # Superadmin panel sudo systemctl status jqc-panel sudo systemctl restart jqc-panel -``` \ No newline at end of file +``` +--- + +## 12. Feature parity with the single-tenant tree (MT-10 → MT-17) + +MT forked from the single-tenant codebase (`LT_Janitorial_Quality_Control`, "ST") +before ST continued shipping features. This section records how that gap was +closed. It is **complete** as of MT-17. + +### 12.1 Why the two trees look more different than they are + +A file-by-file comparison of the two trees overstates the gap. Several ST files +have no MT counterpart *by name* while the feature is fully present under MT's +own naming. These are **not** gaps and must not be "fixed": + +| ST | MT equivalent | +|---|---| +| `models/scheduled_inspection.py` | `models/inspection_schedule.py` | +| `routes/scheduled_inspections.py` | `routes/inspection_schedules.py` | +| `routes/public.py` | `routes/facility_qr.py` | +| `models/notification_recipient.py` | `models/project_recipient.py` | +| `ContractNotificationRecipient` / `get_event_types()` | `ProjectNotificationRecipient` / `get_events()` | +| `app/add_form_schema.py` | `scripts/add_form_schema.py` | +| `support/admin_conversation_detail.html` | `support/conversation_detail.html` | + +There is also one place where **MT is ahead of ST**: `utils/forms.py` +`strong_password()` (length + complexity + common-password blocklist) versus +ST's bare `Length(min=6)`. Porting ST's version would be a downgrade. + +### 12.2 Phase log + +Migrations `phase41` → `phase52` in `migrations/versions/` are the parity track: +auditor role, area QR tokens, schedule plan fields, internal handler, frequency +ENUM widening, recurrence, end date, parent inspection, follow-up attribution, +schedule acknowledgement, and then: + +**MT-15 — External Inspector role. ✅ DONE** (`phase51_external_inspector`) +Adds `external_inspector` to the `users.role` ENUM: an inspector employed by the +customer or a third party, with identical capabilities to `inspector` and scoped +the same way through `InspectorAssignment`. + +The ENUM widening and the code **must ship together**. MT had ~80 sites testing +`role == 'inspector'` with a literal comparison; widening the ENUM alone would +make every one of them evaluate False for the new role and fall through to the +*unscoped* branch — `get_inspector_scope()` returns `None`, downstream queries +drop their facility filter, and an inspector employed by one customer sees every +other customer's contracts. `User.INSPECTOR_ROLES` (exposed as the +`is_inspector` property) is now the single definition, and +`test_external_inspector_scope_is_not_unrestricted` fails loudly if anyone +reverts a membership test to a literal. + +Also in this phase: external inspectors are **invited**, never given a password +(`password_set=False` + emailed 72-hour token, reusing the customer invite mail), +with a new `auth.resend_invite` route so a bounced invitation cannot brick an +account permanently. Creating any other role with a blank password is now +rejected — it previously stored the hash of the empty string. + +**MT-16 — Modern web portal design. ✅ DONE** (`phase52_user_ui_theme`) +Ports ST's `phase48`. `base.html` became a one-line dispatcher +(`{% extends jqc_layout %}`); the old chrome moved verbatim to +`layouts/classic.html`; `layouts/modern.html` is the sidebar shell. All existing +page templates needed **zero edits** — Jinja resolves `{% block %}` overrides +through the whole inheritance chain. + +`ThemedEnvironment.get_template()` swaps `x.html` → `modern/x.html` for modern +users. **The swap is in `get_template()`, not the loader, on purpose:** Jinja's +template cache is keyed on the name `get_template()` receives, so a cached +modern template can never be served to a classic user — and in MT, where one +Gunicorn worker serves many tenants, a loader-level swap would leak across +tenants too. + +MT-specific adaptations that ST's files required: tenant branding (ST hardcodes +its own company name), `inspection_schedules` for ST's `scheduled_inspections`, +`facilities.qr_print_all` for ST's `facility_qr_print_all`, the billing banner, +`role_label` for MT-15's new role, and a rewritten tenant-neutral About page. +`_quota_warning.html` is deliberately **not** in the modern layout — it is a +per-form include, not chrome, and would render twice on four pages. + +**MT-17 — Enrollment intake form. ✅ DONE** (no migration) +Ports ST's `app/enrollment/` — a public, login-free intake form plus an +admin-only inbox, kept deliberately outside the schema (flat JSON, no model, no +migration, deletable package). + +**Tenant isolation was the change ST's version required.** ST keeps every +submission in one flat directory; in MT that directory is shared by every tenant +on the host, so `/enrollment/admin` would list other organisations' submissions. +Submissions are now filed under `/t/`, mirroring +`storage.tenant_key_prefix()`. When multi-tenancy is on and no tenant is bound, +`storage.enrollment_dir()` **raises `TenantUnresolved` rather than falling back +to the root** — a fallback would be a silent cross-tenant leak; an exception is +loud and safe. + +Branding was the second change: ST hardcodes its company name in four places and +a personal Gmail address as the customer-facing "corrections" contact. Both now +resolve from `TenantSettings`, with a test that greps the package so they cannot +silently return. + +### 12.3 Deliberate divergence: ST `phase50_default_modern` is NOT ported + +ST's `phase50` flips the `ui_theme` column default to `modern` **and** runs: + +```sql +UPDATE users SET ui_theme = 'modern' WHERE ui_theme = 'classic'; +``` + +That overwrites every saved preference. It was defensible for a single-tenant +deployment deciding for its own staff after its own A/B test. + +**It is not portable to MT.** The same statement runs against *every tenant +database*, flipping the entire UI for tenants who never saw the test and never +asked. MT therefore ships the `phase48` semantics only: default `classic`, **no +backfill of any kind**. + +The effective default for accounts that never chose is config +`DEFAULT_UI_THEME` (`app/__init__.py::resolve_ui_theme`), which reads the +environment and itself defaults to `classic`. A stored `users.ui_theme` always +wins. **To put a tenant on the modern design, set `DEFAULT_UI_THEME=modern` in +that tenant's process environment** — a config change, reversible, with no +preferences destroyed. `test_new_user_defaults_to_classic` pins this so a future +port of `phase50` has to be a deliberate act. + +### 12.4 Open decisions + +- **Seat quota.** `tenancy/quota.py::count_active_users()` counts all active + users regardless of role, so external inspectors consume a seat against + `max_users`. Intentional (they are real accounts), but tenants near their cap + will hit `@quota_soft_check('users')` when inviting third parties. Excluding + them is a billing-policy decision, not a bug fix. +- **`enrollment/schema.py::CORRECTIONS_EMAIL`** is now an empty last-resort + default. Decide whether to drop the constant and its two config fallbacks in + favour of requiring `TenantSettings.support_email`. + +### 12.5 Deferred, with reasons + +- **`_handler_split` dashboard cards.** ST's classic dashboard shows handler + breakdowns for *opened today* and *unassigned* as well as open issues. MT + supplies `handler_breakdown` (open issues) and both MT dashboards render it; + the other two would require converting `.count()` queries to `.all()` and + fetching full rows for a cosmetic card, which regresses large tenants. If + wanted, do it as a SQL `GROUP BY handler_type` rather than ST's Python-side + count over fetched rows. +- **PDF / audit hardening.** +- **MT-9 iOS client** (see §7).