Uh oh!
There was an error while loading. Please reload this page.
feat(teams): team-based multi-tenancy - #317
Closed
github-actions[bot] wants to merge 1 commit into
Closed
Conversation
* feat(teams): add Buzz Team and Buzz Team Membership Multi-tenancy foundation for v2: two doctypes, their controllers, and a patch so existing installs come out of migrate owning a team rather than none. Ownership is the membership row — there is deliberately no owner field on Buzz Team. An earlier draft had one and it drifted immediately: changing it moved nothing and left the field disagreeing with the rows it claimed to describe. after_insert takes the owner from flags instead, since the patch creates teams for users other than the session user. sync_frappe_roles recomputes Event Manager / Frontdesk Manager across all of a user's enabled memberships rather than adding and removing per row, so someone holding Manager in two teams keeps the role when one membership is disabled. Buzz User is not in that map: utils.add_buzz_user_role already grants it, and two writers for one role would fight. Only on_update is wired, not after_insert — Document._action is "save" for both, so the latter was a second recompute on every insert. Membership uniqueness is enforced in validate rather than a unique index, so the invitation upsert in a later step gets a readable message instead of an integrity error. enabled is a Check named exactly that: frappe's link search filters on the fieldname and delete_doc offers "disable instead of delete" for free. Slugs dedupe on generation because two users called Sam both want sams-team and the column is unique; an explicitly set duplicate still fails. Naming uses BTEAM.#### rather than format:, which frappe now warns against. Ownership cannot be transferred yet — the Owner row is locked outright. Relaxing that to "one enabled Owner must remain" is the change to make when there is a UI that needs it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(teams): add a team field to the team-direct doctypes Buzz Event, Event Venue, Event Host, Event Template and Buzz Campaign get a reqd, indexed team Link. Nothing else does: the 16 doctypes that carry an event link can reach their team through it, so a denormalized column on each of them would be a perf optimization for load nobody has measured. Add one per doctype if a query ever proves slow. reqd would break the 49 places tests construct these documents, so set_team_from_sole_membership fills an empty team from the user's only enabled membership, and setup_test_records seeds a Test Team owned by Administrator. Zero or several memberships leave the field empty and let reqd raise rather than picking a team on the user's behalf. Ordering holds because run_before_save_methods runs validate before _validate runs the mandatory check. Server-side inserts never consult frappe.defaults, so the handler is the mechanism; user defaults would only have covered the Desk client. assign_default_team backfills existing rows from the site's first owned team, creating one if the site has none. It carries its own frozen doctype tuple: a patch that follows a moving constant changes behaviour retroactively. Nothing is enforced yet. A site on this commit behaves as it did before, except that a user in two or more teams must pick one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(teams): give fresh sites and the e2e user a team install_app defaults to set_as_patched=True (frappe/installer.py:271, :330), so patches are recorded as applied and never run on a new site. create_default_teams therefore never fires there, leaving the site with no team at all — and the first Event Host insert dies on the reqd team field. E2E hit this first because its site is built from scratch every run. after_install now seeds Administrator a team, and setup_test_records reuses it instead of making a second one, which would have given Administrator two memberships and broken the resolver for every fixture. The e2e user is created after install with System Manager only, so it has no membership of its own. auth.setup.ts gives it one; every other setup project already depends on that project, and this keeps a local playwright run working too, which a CI step would not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(teams): drop the install-time team seed after_install seeded Administrator a team so a fresh site was never teamless. auth.setup.ts now covers the e2e site and setup_test_records covers the python suite, so the only thing left for it to do on a real install was put an Administrator's Team record on every new site that no organiser would use. A new install starts with no team, and the first user creates one from the Team field. That is the model: a user with no team picks one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(teams): enforce row-level team isolation Adds buzz/permissions.py: query conditions and deny-only has_permission hooks for every team-owned doctype. Doctypes with a team column are scoped on it; the rest reach their team through their event link, since step 02 deliberately left them without a denormalized column. has_team_access holds the whole role matrix — any enabled member reads, Owner/Admin/Manager write, Owner/Admin delete — and the four hook pairs are thin wrappers over it plus their carve-out. Rows with no team stay readable so a half-migrated site does not hard-break. Non-member carve-outs keep the attendee-facing dashboard working: owner matches for Event Booking, Event Ticket, Sponsorship Enquiry and Event Talk; published events widen Buzz Event and Sponsorship Tier for logged-in non-members. Guest reads are never narrowed. Event Payment and Ticket Cancellation Request get no carve-out — attendees reach those through APIs that already ignore permissions, so adding one would widen access past today's behaviour. Talk Proposal composes the team scope inside its existing two functions rather than re-registering, or speakers would lose sight of their own proposals. Its is_proposal_manager carve-out is gone: with teams, Event Manager is held by every team manager on the site, so a blanket role check would defeat isolation on the doctype most exposed to it. CheckinService gains the membership check in its constructor, the one place both check-in endpoints route through, and the e2e front-desk fixture now gets a membership — the role alone is no longer enough. hooks.py spells the 23 registrations out as literal data and imports nothing; test_every_tenant_doctype_is_wired_to_both_hooks derives the tenant inventory from the schema and fails if a doctype is missing from either dict. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(teams): return qb criterions from permission query hooks The permission_query_conditions builders spliced namespaced SQL by hand: backticked table names, embedded subqueries and manual frappe.db.escape calls. db_query renders a non-string hook return through pypika instead (frappe/model/db_query.py:1180-1184), inlining values with the driver's own escaping, so the fragments can be composable expressions. Every builder now returns a Criterion, or None when the user is unrestricted, replacing the "" sentinel. Talk Proposal's speaker conditions move with them, since it composes the derived builder's output and would otherwise concatenate a Criterion into an f-string. Adds two tests for Buzz Team and Buzz Team Membership list scoping. The suite never read either doctype, so the builders behind them had no coverage before this. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(teams): create team memberships from accepted invitations Invitations reuse frappe's User Invitation. The team and the team role ride on two custom fields — extra invite params are dropped on insert without them, and the accept flow re-reads the invitation from the database, so the hook would otherwise see nothing. on_invitation_accepted is the only place every accepted invitation routes through, whoever created it, so the inviter's authority is checked there rather than at invite time: invited_by must hold Owner or Admin on the team, and no invitation may grant ownership. Membership is upserted, so a lapsed member is re-enabled instead of duplicated and an enabled one is left alone. sync_frappe_roles now saves the User with ignore_permissions. The accept endpoint is allow_guest, so the invitee is Guest while the hook runs and could not write to User — every accept would have raised PermissionError, as would a team admin adding a member from the dashboard. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(teams): give every team its own settings Tenant-specific configuration moves out of the site-wide Buzz Settings single into a per-team Buzz Team Settings doctype, one row per team, named after it. Buzz Settings keeps the genuinely global config (login banner, event-proposal funnel) and becomes the seed source for the rest. A team's row is created with its owner membership in Buzz Team.after_insert, copying the current globals; create_team_settings_for_existing_teams does the same for teams that predate the doctype. Copy-on-create, no runtime fallback chain: changing a global later never mutates existing teams. Rewired readers: ticket transfer / add-on / cancellation windows, ticket and booking confirmation email templates, support_email, sponsor-deck defaults and the Zoom webinar template. Event-level fields keep precedence where they already had it. ticket.html no longer fetches Buzz Settings itself — support_email arrives in args, as booking_confirmation.html already did. Two things the spec assumed turned out otherwise. Int columns are NOT NULL DEFAULT 0, so an unset cutoff can never be None and a Python-side default is unreachable: the 7-day fallback is the doctype default and DEFAULT_CUTOFF_DAYS is gone. And the write rule for settings (members read, Owner/Admin write) already existed as membership_has_permission, now renamed team_admin_has_permission and shared by both doctypes. default_webinar_template stays a custom field, mirrored onto Buzz Team Settings from ZOOM_INTEGRATION_CUSTOM_FIELDS, so sites without zoom_integration get neither a broken picker nor a failing seed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(teams): stub sendmail when inviting in team membership tests The invitation mails itself out on insert, so the tests failed on any runner without a default outgoing Email Account. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(teams): migrate existing sites into one shared team Before teams, every enabled Event Manager worked on every event. The old create_default_teams patch gave each of them a personal team, and assign_default_team then backfilled every row to one arbitrary team, so every other manager lost the events they had created. create_default_teams now builds a single team: owner is the manager behind the most Buzz Events (ties, and a site with no events, go to the oldest account), everyone else joins as Admin. Administrator is excluded — it bypasses team permissions and owns no one's work. assign_default_team early-returns when no row is team-less, so a site that needs no backfill no longer mints a "Default Team", and only the doctypes with orphans are updated. Both patches are bumped to #2 so already-migrated dev and CI sites pick up the new semantics. Patch tests move to buzz/test_default_team_patches.py. The patches read the whole site, so setUp retires every enabled Event Manager first — the dev database carries real ones, and rollback is per class, so the previous test's managers are still around too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(teams): stub sendmail for every invitation, not just invite_by_email df61297 put the frappe.sendmail stub inside the invite helper, but test_accept_is_refused_when_the_invitation_names_no_team builds its User Invitation directly — it has no team, so invite_by_email cannot produce it. after_insert mails the invitee with now=True, which needs a default outgoing Email Account, so the test errored on any site without one. Local sites have an account configured; CI does not. The stub moves to setUp, where every insert in the class routes through it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(teams): refuse to delete an owner membership validate_owner_is_locked guards the save path only. The doctype grants System Manager delete and team_admin_has_permission allows delete for any team Owner or Admin, so the row that cannot be disabled or demoted could still be removed outright, leaving the team with no owner. on_trash closes the same invariant from the delete side. Nothing is foreclosed by it: a Buzz Team with memberships already raises LinkExistsError on delete, and the app has no team-deletion feature. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(teams): add team members from the Buzz Team form A team had no way to take on an existing user. Only an accepted User Invitation created a membership, and that path is for people who have no account yet. Adds an "Add Team Members" button: pick users as pills, give each one a role, then create every membership in one call and mail the people who were actually added. Both whitelisted methods are gated by can_manage_members, the rule team_admin_has_permission already applied to membership writes; onload publishes the same flag so the button stays hidden for a Manager or Viewer. Owner is not offered — it is granted at team creation and locked. Search is the app's own rather than frappe.db.get_link_options: the standard query for User hides website users whenever filters are passed, and never matches a full name. MultiSelectPills means to hide already-picked rows but discards its own filter() result, so that is done here too. upsert_membership now reports whether the user is newly on the team, so a no-op re-add sends no mail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 2e3eef2)
harshtandiya
marked this pull request as draft
August 18, 2026 13:48
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 freeto 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.
Automated backport of #312 to
main.