Catch the old frontend up with the team owner role and team-conferred access - #2918
Merged
Merged
Conversation
Teams gained a fourth role above admin, so the four are Owner, Admin, Manager and Member. They are the generic grant roles under team-facing names, which the team UI had no vocabulary for: it read a member's role out of the grant messages, which have no entry for owner, and guarded that lookup with `if (!role)` -- and owner is 0, the one role that is falsy. Every team has an owner since the migration promoted the oldest admin on each, so every team listed its owner as a Member. Team roles now have their own names and predicates, mirroring the server's TeamRole, and AsTeamMember can say whether a member owns their team, runs its content, or is its last owner. The grant vocabulary is left alone, since Read/Write/Admin is still what project grants mean. The server's two owner invariants are enforced here rather than discovered by a rejected request: only an owner may hand out ownership, so the role is shown disabled to everyone else, and a team is never left without an owner, so its last one cannot be demoted, removed, or leave. Both say why. Deleting a team is likewise owner-only now, as the server has always treated it, rather than offered to any admin.
A challenge can now be owned by a team, which hands it to that team's managers whatever role they hold on the parent project. canManageChallenge only ever asked about the parent project, so a team manager with nothing on that project saw no Manage controls on a challenge the server would have let them edit -- on the challenge detail page, on challenge result items, in the task pane, and in the admin pane's challenge list. It now also asks whether the user manages the team named by the challenge's ownerTeamId. Like projectRoles, that reads granted roles rather than accepted memberships, so someone invited to a team but yet to accept looks like a member here. That is the lenient direction, which keeps stale data from producing spurious permission errors, and the server -- which does require an accepted membership -- refuses anything real. Project managers needed no change: the owner role is only ever granted on a team, never on a project, so the project side still deals in the same three roles it always has.
A team's members reach a project at the role they hold in the team now, so the per-team dropdown set a value the server no longer reads. Team rows say what actually governs them instead.
The server folds the project grants of every team a user belongs to into their grant list, so projectRoles was reading a team's grant as if the person held it themselves. That matched the old behaviour, where a team's grant did hand its role to every member. It no longer does: access follows the role held in the team, so a plain member of a team attached as Admin was being offered controls the server now refuses. Only grants made to the person count as their own. What a team confers is worked out from their role in it -- owners and admins as admin, managers as write, plain members nothing -- by the same rule the server applies. canManageChallenge gains the attached-team route alongside the owning one, so a team let at a single challenge reaches it here too.
CollinBeczak
changed the title
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.
The old frontend against the same backend changes. Independent of the v4 branch;
still needs maproulette-backend#
project-owner-team.The owner role
Teams gained a fourth role above admin. The UI had no vocabulary for it: it read
a member's role out of the grant messages, which have no entry for owner, and
guarded that lookup with
if (!role)— and owner is0, the one role that isfalsy. Every team has an owner since the migration promoted the oldest admin on
each, so every team listed its owner as a Member.
Team roles now have their own names and predicates, mirroring the server's
TeamRole. The grant vocabulary is left alone, since Read/Write/Admin is stillwhat project grants mean.
The server's two invariants are enforced here rather than discovered by a rejected
request: only an owner may hand out ownership, and a team is never left without
one, so its last owner cannot be demoted, removed, or leave. Both say why.
Deleting a team is owner-only, as the server has always treated it.
Team-owned challenges
A challenge owned by a team belongs to that team's managers whatever role they
hold on the parent project.
canManageChallengeonly ever asked about the parent,so they saw no Manage controls on work the server would have let them edit.
Team-conferred access
projectRolesread a team's grant as if the person held it themselves — whichmatched the old flat behaviour, but no longer does. A plain member of a team
attached as Admin was being offered controls the server now refuses.
Testing
589 unit tests, including cases for each team role against an attached team and a
plain member being refused.