feat(SCIM): Support deactivated user membership - #8370
Conversation
Adds `UserOrganisation.is_active`, defaulting to `True`. Deactivated members can still log in, but cannot access the organisation, do not see it in the organisation picker, and do not count towards the seat limit. Their roles, permissions and group memberships are retained, so reactivation restores access. Reactivation deliberately does not enforce the seat limit: it is driven by an external identity provider over SCIM, where failing the call would leave the provider and Flagsmith out of sync. Ref: #8368
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe change adds an Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to This change adds deactivated membership handling and exposes its status without any supplied merge-blocking concern; no actionable merge-blocking risk remains after normal checks and review. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Docker builds report
|
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19767 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
🗂️ Previous results✅ oss · depot-ubuntu-latest-16 — run #19767 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #19767 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
Skipped testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss ✅ oss · depot-ubuntu-latest-arm-16 — run #19767 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #19765 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-16 — run #19765 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19765 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19763 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #19763 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19763 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-16 — run #19763 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-16 — run #19761 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 74b6d7d8-d0ac-45cc-a7a3-64a569bf5999
📒 Files selected for processing (17)
api/api_keys/user.pyapi/audit/views.pyapi/organisations/invites/views.pyapi/organisations/migrations/0061_add_user_organisation_is_active.pyapi/organisations/models.pyapi/organisations/serializers.pyapi/organisations/task_helpers.pyapi/organisations/views.pyapi/permissions/permission_service.pyapi/sales_dashboard/views.pyapi/tests/unit/organisations/test_unit_organisations_deactivated_membership.pyapi/tests/unit/users/test_unit_users_models.pyapi/users/abc.pyapi/users/models.pydocs/docs/administration-and-security/access-control/scim.mddocs/docs/deployment-self-hosting/observability/_events-catalogue.mdopenapi.yaml
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
Visual Regression19 screenshots compared. See report for details. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8370 +/- ##
========================================
Coverage 98.79% 98.80%
========================================
Files 1616 1618 +2
Lines 65283 65409 +126
========================================
+ Hits 64499 64625 +126
Misses 784 784 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
`cancel_users` picked the earliest-joined admin without regard to `is_active`. A deactivated admin could therefore be retained while every active membership was deleted, leaving the organisation with no user able to access it and no way to undo it. Prefer the earliest active admin, fall back to the earliest active member, and no-op when no seat is in use \u2014 which also removes the latent `AttributeError` when the organisation has no admin at all. Ref: #8368
The early return in `set_organisation_membership_active` lost its test when the seat-limit check was removed, leaving the branch uncovered. Ref: #8368
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to #8368
In this PR, we add
UserOrganisation.is_active, defaulting toTrue, so a membership can be suspended without being destroyed.A deactivated member:
403on the organisation and no longer sees it in the organisation picker;Membership is checked in a lot of places, all of which previously traversed the
Organisation.usersM2M and so would have kept letting deactivated members in.The traversals are now expressed as
permission_service.get_active_membership_filter, which keeps theuserandis_activelookups in a singlefilter()call.UserOrganisation.is_activeis exposed read-only onUserOrganisationSerializerso the frontend can surface deactivated members later. Nothing in the api writes it yet outsideset_organisation_membership_active.How did you test this code?
Added unit tests.