Repository navigation
Scope review authority by category reviewer groups - #137
Merged
Merged
Conversation
Add DoctrineCategory.reviewer_groups (blank M2M to Group): members of those groups who also hold a Fit Check review permission may review submissions for that category's doctrines. An empty list is unscoped - any reviewer may act - so an install that configures nothing keeps the original behaviour exactly. Reviewer authority is kept distinct from category visibility (Selected/Required groups): being able to review a category is separate from being admitted to see it. The scope is enforced across the whole review path: the queue base queryset and its doctrine dropdown, individual and bulk approve/reject actions, the decide guard (403 out of scope), submission detail access and its decision panel, the new-submission reviewer ping fan-out, and the reviewer digest's per-reviewer counts. Narrow the manager bypass: _is_manager now means doctrine managers and superusers only. A plain review permission no longer reveals every doctrine, fit, and category - a reviewer sees member-visible content plus the categories they are scoped to review (authority grants sight). This flows through the visible_to querysets and the category-chip helpers via the shared visible_category_ids fold. New helpers _has_review_perm, reviewable_category_ids, can_review_submission, and FitSubmissionQuerySet.reviewable_by centralise the rules; _can_review is removed in favour of the shared helpers. Add the Reviewer groups field to the category edit form/template and the admin, and cover the whole behaviour in test_review_scoping.py (unscoped-install invariants, scoped authority across every surface, multi-category and doctrine-less openings, and the reviewer visibility narrowing). Migration 0041.
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.
What
Adds per-category review scoping so an alliance can restrict who reviews which submissions (e.g. a "Super Capital Admin" group reviews titan-doctrine submissions while a blackops reviewer cannot see or decide them), while keeping group membership (visibility) distinct from review authority.
Model
DoctrineCategory.reviewer_groups— a blank M2M toGroup. Members of these groups who also hold a Fit Check review permission may review submissions for that category's doctrines. Empty list = unscoped (any reviewer). Migration0041.Semantics
A submission is reviewable by a plain reviewer if its doctrine is
None, its doctrine has no categories, ≥1 of its categories has emptyreviewer_groups(unscoped), or the user's groups intersect ≥1 category'sreviewer_groups. Doctrine managers and superusers bypass everything. Both review permissions (review_submissions,secure_group_management) are treated identically. With nothing configured, behaviour is byte-for-byte the same as before.Enforcement
Scope is applied across the whole review path:
decide(403 when out of scope), bulk approve, bulk deletenotify_reviewers_new_submission)send_review_digest) — NotificationSettings gating and per-user mute preservedVisibility narrowing
_is_managernow means doctrine managers and superusers only. A plain review permission is no longer a see-everything bypass: a reviewer sees member-visible content plus the categories they are scoped to review (authority grants sight). This flows through thevisible_toquerysets and the category-chip helpers via a sharedvisible_category_idsfold. An unscoped category still lets any reviewer decide its submissions even when they cannot open the doctrine page (authority ≠ visibility).New helpers (
_has_review_perm,reviewable_category_ids,can_review_submission,FitSubmissionQuerySet.reviewable_by) centralise the rules; the duplicated_can_reviewhelper is removed.UI
Category edit form/template and the admin gain the Reviewer groups field, with help text making the membership-vs-authority distinction explicit.
Tests
New
fitcheck/tests/test_review_scoping.pycovers: unscoped-install invariants (queue/decisions/notifications/digest unchanged), scoped authority across every surface, doctrine in two categories (one unscoped opens it to all), doctrine-less submissions, the reviewer visibility narrowing (in-scope sees, out-of-scope 403, manager sees all), and the pilot self-view. Full suite green (833 tests);checkandmakemigrations --checkclean.