Skip to content

test(perm): shared helpers for legacy-only holders & grantless members - #2451

Open
vecchp wants to merge 2 commits into
feat/perm/teams-header-stripfrom
chore/perm-legacy-fixture-helper
Open

vecchp wants to merge 2 commits into
feat/perm/teams-header-stripfrom
chore/perm-legacy-fixture-helper

Conversation

@vecchp

@vecchp vecchp commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Consolidates the legacy-only fixture dance the grant cutover left copy-pasted across suites — create a leftover PermissionGroup row (nothing provisions grant-only templates any more), add a membership, drop the auto-mirrored Grant — into shared helpers, and adopts them everywhere.

Stacks on #2450 (feat/perm/teams-header-strip); lands after it.

What changes

New helpers (common/tests/utils.py)

  • make_permission_group(*, organization, template_name) — the org's PermissionGroup, created on demand (idempotent template + row).
  • revoke_grants(user) — drop every Grant for the user: the grantless-org-member state.
  • add_legacy_membership(user, *, group) — legacy membership without the Grant mirror (m2m-edge mirror dropped).
  • make_legacy_only_holder(*, organization, template_name, user) — the composite: org membership + leftover group + no Grants.

Adopted in

  • teams / reports / member-management test_grant_authorization suites (each lost its inline template/group/mirror block);
  • accounts/tests/test_roles.py — its local _add_legacy_membership and _legacy_org_admin_group copies are gone;
  • accounts/tests/test_role_manager_grants.py, accounts/tests/test_permission_group.pymake_permission_group;
  • shelters test_room_queries / test_bed_queries / test_reservation_queriesrevoke_grants(self.operator) replaces the inline Grant…delete().

Test plan

  • accounts common reports teams shelters1310 passed; ruff clean.
  • No production code touched (test utilities only).

Summary by Sourcery

Centralize legacy permission and grantless-member test fixtures and adopt them across the affected test suites.

New Features:

  • Add shared test helpers for creating permission groups, modeling legacy-only memberships, and removing user grants.

Enhancements:

  • Consolidate duplicated legacy permission fixture setup across account, team, report, member-management, and shelter tests.

Tests:

  • Update authorization, role, permission-group, and shelter query tests to use the shared helpers.

The grant cutover left the same fixture dance copy-pasted across suites:
create a leftover PermissionGroup row (nothing provisions grant-only templates
any more), add a membership, and drop the auto-mirrored Grant.

- common/tests/utils.py: `make_permission_group` (an org row created on
  demand), `revoke_grants` (the grantless-member state), `add_legacy_membership`
  (membership without the mirror), and the composite `make_legacy_only_holder`.
- adopt in the teams / reports / member-management grant-authorization suites;
- test_roles: drop the local `_add_legacy_membership` / `_legacy_org_admin_group`
  copies in favour of the shared helpers;
- test_role_manager_grants / test_permission_group: use `make_permission_group`;
- shelters room/bed/reservation queries: `revoke_grants(self.operator)` replaces
  the inline Grant delete.

Validation: accounts+common+reports+teams+shelters — 1310 passed; ruff clean.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @vecchp, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 4 days and 22 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

This test-only PR consolidates legacy permission-group and grantless-member fixture setup into reusable helpers, then applies them across authorization, role-backfill, permission-group, and shelter query suites without changing production code.

File-Level Changes

Change Details Files
Centralize construction of legacy-only permission fixtures and grantless users.
  • Add idempotent permission-group creation by organization and template.
  • Add helpers to remove all user grants and create group membership without the signal-generated Grant mirror.
  • Add a composite helper for organization members with leftover legacy groups and no grants.
apps/betterangels-backend/common/tests/utils.py
Replace duplicated legacy-only authorization fixtures with shared setup across authorization and role migration tests.
  • Adopt the composite helper in teams, reports, and member-management denial tests.
  • Replace local legacy membership and group-building helpers in role backfill tests.
  • Use shared group creation in permission-group and role-manager grant tests.
apps/betterangels-backend/accounts/tests/test_member_management_grant_authorization.py
apps/betterangels-backend/accounts/tests/test_roles.py
apps/betterangels-backend/accounts/tests/test_role_manager_grants.py
apps/betterangels-backend/accounts/tests/test_permission_group.py
apps/betterangels-backend/reports/tests/test_grant_authorization.py
apps/betterangels-backend/teams/tests/test_grant_authorization.py
Standardize grant removal in shelter query tests.
  • Replace inline Grant deletion with the shared revoke_grants helper in room, bed, and reservation query tests.
apps/betterangels-backend/shelters/tests/test_room_queries.py
apps/betterangels-backend/shelters/tests/test_bed_queries.py
apps/betterangels-backend/shelters/tests/test_reservation_queries.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

CI lint (uv run ruff format --check .) caught a wrap that fits on one
line (108 < 120 columns).
vecchp pushed a commit that referenced this pull request Sep 10, 2026
…y, nits

F1  reconcile_org_groups unmirrors stale derived rows before deleting them,
    so an org-type change fully revokes (mirrored Grants no longer survive
    config cleanup); teardown deletes keep leaving Grants standing.
F2  signal docstring states the sanctioned exception precisely (m2m-manager
    writers only); helper duplication already consolidated in #2451.
F3  same-row direct-grant revocation pinned by test + GrantAdmin fieldset
    description (unique constraint makes both grants one row).
F4  moot — accounts/extensions.py deleted in 590dc50.
F5  set(dict) -> .keys().
F6  stale delete_orphaned_group refs corrected (services.py, admin.py);
    tear-down described as the structural MTI cascade (migration 0007).
F7  LoadedRowRawIdWidget preserves the original widget's using kwarg.
F8  teams/tests/test_authz_registry.py canary added (4 fields).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants