feat(settings): tenant self-service settings, validate TENANT scope_id (#382) [stack 10/11] - #394
Merged
Merged
Conversation
…dation (#382) - SettingDefinition gains tenant_overridable and an optional check hook (ValueError -> 422, LookupError -> 404) run on every TENANT-scope write. - GET /api/settings/tenant/current (+ GET/PUT/DELETE .../{key}) act on request.state.tenant_id only, overridable keys only, guarded by the new settings.tenant.edit permission mapped onto tenant:owner/tenant:admin; 403 with no active tenant, 422 for non-overridable keys. - Platform routes validate a TENANT scope_id via tenant_exists: 404 on GET/PUT, 422 for a create body / store form; DELETE stays unvalidated. - SYSTEM/TENANT writes publish a per-(tenant, key) notice on the settings.values invalidation channel after commit. - tenants: retire the unused tenants.settings.manage permission; add the /tenants/settings page for owners/admins. Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV
Deploying simple-module-python with
|
| Latest commit: |
ae88c53
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6f22a911.simple-module-python.pages.dev |
| Branch Preview URL: | https://tenancy-10-settings.simple-module-python.pages.dev |
…(review of #382) PUT /admin/settings/{id} wrote a TENANT row's new value without the tenant/key check the store form and the JSON routes run. Extract tenant_update_error (row's own scope/tenant/key; skipped for description-only or unchanged values) so views.py stays under the cap. Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV
…where (ship review) SettingDefinition.clear_via names the route that owns clearing a key (an upload route that reaps the stored file). The self-service, platform tenant, by-id and admin-form deletes answer 422 pointing there while the tenant exists; rows of deleted tenants stay clearable by an operator. Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV
…e file cap (ship review) Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV
…plicate create (qa BUG-001, BUG-002) Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV
…cope (ship review r2) The managed-key guard was a per-route Depends that only covered TENANT rows, so SYSTEM/USER rows of a clear_via key could be deleted generically and orphan the stored file, and a direct service call bypassed it. It now lives in SettingService.delete/delete_scoped (single row read): any scope raises ManagedKeyError, mapped to 422 in the API and to a translated toast in the Inertia store screen. Owners pass as_owner=True; a deleted tenant's leftover TENANT row stays deletable. Listing queries split into _listing.py for the 300-line cap. Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV
… i18n keys (ship review r2) Move the testable-packages helper next to the other module-settings prop code and commit the regenerated i18n catalog for the new delete keys. Claude-Session: https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV
antosubash
added this pull request to stack #397
October 1, 2026 16:14
antosubash
marked this pull request as ready for review
October 1, 2026 16:17
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #382 (which replaces #368). Stack 10/11 of the tenancy-adoption series (base: #393).
What
SettingDefinitiongetstenant_overridableand an optionalcheckhook. RaisingValueErrorgives 422 andLookupErrorgives 404. The hook runs before every TENANT-scope write.Settingkeeps its explicit(scope, scope_id, key)key and has no mixin.GET /api/settings/tenant/currentplusGET/PUT/DELETE /api/settings/tenant/current/{key}.request.state.tenant_idonly, never on a tenant id from the URL.settings.tenant.editpermission guards them. It is mapped ontotenant:ownerandtenant:admin; members get 403.scope_idwithtenant_exists. Unknown ids get 404 on GET and PUT and 422 on POST. DELETE stays unvalidated so leftover rows can still be cleaned up.tenants.settings.manageis retired in favour of the singlesettings.tenant.edit. The old permission was owner-only and nothing checked it.settings.valuesafter commit, keyed"<tenant>|<key>"./tenants/settings("Organisation settings"). For each key it shows the platform value, the organisation's override and the effective value.Tests
test_tenant_settings.py, 3 for the page and 2 JS tests.make test-py: 3444 passed.make test-js: 465 passed.make lintandmake doctorare clean.Follow-ups
/admin/settingsformPUTpath doesn't runcheckfor TENANT rows; the JSON API does.https://claude.ai/code/session_01F8RiTBUJQnZmSq56qReZeV