Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement support for multiple team owners and multiple teams per user #5008

Merged
merged 43 commits into from
Feb 19, 2025

Conversation

zoldar
Copy link
Contributor

@zoldar zoldar commented Jan 22, 2025

This PR addresses enabling support for following across the whole application:

  • more than one user as an owner of a single team
  • more than one non-guest membership per user

Summary of changes:

  • Teams are exposed in CRM now and all team related fields were moved there from User CRM with a link to it
  • Enterprise Plan CRM refers to team directly as well now though the team is still searchable via owner's name or email; the workarounds for limited Kaffy association selection are left in place because default way of using team name only when listing and looking up are not sufficient; A faulty feature selection prefill on enterprise plan create was fixed BTW
  • The Editor role is now checked for consistently across all AuthorizeSiteAccess calls instead of implicit translation of legacy Admin in the input roles to Editor
  • Team is now can now be explicitly provided during site creation service. In Sites API it's exposed as an optional argument, team_id (required though if user is an owner on more than one team already). If the argument is not passed, an implicit team is used or created if there isn't one already. In case of adding a website from the dashboard, a currently selected team is used. The controller action has an option to accept "team_id" but it's not exposed in the UI yet; This leads to a problematic corner case mentioned below
  • User deletion is prevented when user is the only owner on a setup team; the handling logic will be extended in a follow-up
  • API key provisioning is checked against Stats API availability only if the user is an owner of at most 1 team; otherwise, they are allowed to create the API regardless of feature availability on any of the owned teams; The feature availability is checked at the time the key is used anyway; We may consider removing the check at the provisioning time entirely - currently it's only preserved for "legacy" behavior
  • notification emails directed at site or team owner are now sent to all owners at once, if there's more than one
  • when handling Paddle webhook for subscription creation for a user without a team yet (a case of non-member user with pending site ownership transfer), we always default to creating a new "My Team" even if there are an owner on existing one to avoid ending up with 2 active subscriptions on one existing team
  • site invitation accept routine is now passed a team the user is currently switched to. This is only relevant for site ownership transfer; the transfer will be attempted with a current team (and will fail early if permissions are insufficient)
  • when checking team member usage quota, only one owner is subtracted from the usage calculation even there are multiple
  • when site transfer ownership is executed, all existing owners are added as guest editors to the transferred site (if they aren't members on the other team already)
  • direct site ownership transfer from CRM is now accepting optional "Team Identifier" for gracefully handling cases of users owning more than one team; the identifier is exposed in Teams CRM when viewing a particular team
  • there's a team switcher in the upper right menu now; the currently switched team is stored in session cookie; the switcher is only shown when user is team member on more than one team
  • there are now 3 assigns populated by AuthPlug and AuthContext - my_team (existing), current_team and multiple_teams?; multiple_teams? is a flag used to determine whether to show the team switcher in the upper menu; current_team is a team set via team switcher and stored under current_team_id in session cookie; if there's none set, it falls back to my_team; my_team is either the same as current_team is user's team role is owner or the first found team where the user is an owner otherwise - they may differ
  • Listing sites via /sites is now scoping the list with current team; That applies to only full memberships - sites where user is a guest and guest invitations are all visible regardless of which team the user is currently switched to
  • Listing sites via Sites API has an option to scope via team identifier with team_id parameter

There still is a number of corner cases to consider, most likely in a follow-up:

  • HelpScout integration implicitly falls back on the first team found owned by a user when they are an owner of more than one team; the integration will probably get extended with a way to list those teams and let support folks switch easily between them
  • Notes exposed in HelpScout integration are still on user only; it will probably make sense to have them on a team as well
  • when trying to create a site as a viewer member of another team (without any other team membership, including implicit one), the user is basically locked out of creating a team of their own until the are removed from the current one; We might want to provide an "exit hatch" where, on failed attempt, we suggest user an option to create a site under their own account
  • team Identifier is not yet explicitly exposed anywhere; given Sites API has now ability use it, the identifier should be exposed somehow - via Team Switcher in the UI and/or with additional endpoint in Sites API
  • when a user has their own implicit team and gets added as a team member to another team, when switched to their team, they still see CTA option and can't access team member management until they complete it. It feels a bit weird but maybe it's fine? Leaving it as is for now.
  • the /sites view still lists all sites the user has access to, regardless of which team they are switched to; the view will probably be scoped to the currently switched team, though the Sites API endpoint should probably still list everything, with an optional team ID parameter? Added
  • add ability to choose team to transfer ownership to in CRM

TODO

  • set is_autocreated to false either on role downgrade or team membership upsert where role is downgraded (or at least double-check the implications of leaving it on)
  • hide team switcher when there's only one team to choose from
  • list owned teams in user CRM edit form for easy navigation
  • add as view-only team CRM edit form: subscription plan, subscription status, grace period
  • properly enable lookup by team identifier in Enterprise Plan CRM edit picker
  • more manual testing, possibly extending automated test suite

Changes

Depends on #5004

Tests

  • Automated tests have been added

@zoldar zoldar changed the base branch from master to drop-one-team-per-user-constraint January 22, 2025 15:22
@zoldar zoldar force-pushed the multiple-teams-owners branch 9 times, most recently from 1a9aac5 to 4472c5b Compare January 29, 2025 11:03
@zoldar zoldar force-pushed the drop-one-team-per-user-constraint branch from a978a90 to 66c2123 Compare January 29, 2025 13:32
Base automatically changed from drop-one-team-per-user-constraint to master January 29, 2025 14:08
@zoldar zoldar force-pushed the multiple-teams-owners branch from 4472c5b to 181c547 Compare January 29, 2025 14:09
@zoldar zoldar added the preview label Jan 29, 2025
Copy link

Preview environment👷🏼‍♀️🏗️
PR-5008

@zoldar zoldar force-pushed the multiple-teams-owners branch 7 times, most recently from f7d9ee5 to 5fb8b28 Compare February 6, 2025 12:30
@zoldar zoldar force-pushed the multiple-teams-owners branch 2 times, most recently from 1598169 to 0354cc4 Compare February 10, 2025 20:48
@@ -90,17 +90,30 @@ defmodule Plausible.HelpScout do
plan = Billing.Plans.get_subscription_plan(team.subscription)
{team, team.subscription, plan}

{:error, :multiple_teams} ->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This case will be handled better on further iterations. For now we just fall back on first team the user owns.

{:error, :no_team} ->
{nil, nil, nil}
end

status_link =
if team do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If user does not own any team yet, we fall back on linking directly to the User CRM


case Sites.create(user, params) do
case Sites.create(user, params, team) do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sites(user, params, nil) will preserve the current behavior of adding the site to the existing implicit team or creating one.

@@ -73,6 +74,20 @@ defmodule PlausibleWeb.Api.ExternalSitesController do
"Your account has reached the limit of #{limit} sites. To unlock more sites, please upgrade your subscription."
})

{:error, _, :permission_denied, _} ->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The new error cases will only occur on explicit team selection or once user is an owner of more than one team at once.

@zoldar zoldar force-pushed the multiple-teams-owners branch from 22e91db to adfa9d2 Compare February 19, 2025 08:15
@zoldar zoldar added this pull request to the merge queue Feb 19, 2025
Merged via the queue into master with commit bf010a1 Feb 19, 2025
8 checks passed
zoldar added a commit that referenced this pull request Feb 19, 2025
@zoldar zoldar deleted the multiple-teams-owners branch February 19, 2025 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants