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.

Sign up for GitHub

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

Jump to bottom

Added members to admin team page #6915

Merged
merged 6 commits into from
Mar 15, 2023
Merged

Added members to admin team page #6915

merged 6 commits into from
Mar 15, 2023

Conversation

hotzenklotz
Copy link
Member

@hotzenklotz hotzenklotz commented Mar 13, 2023 •
edited
Loading

A quick PR to add the list of all team members to each team on the respective admin page. In a way it is the same information as in the user list just transposed.

image

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • Create a bunch of teams and assign them to users.
  • Check that the correct users are display for an expanded team
  • Consider edge cases: Admins, Teams with no members etc

Issues:

(Please delete unneeded items, merge only when none are left open)

@hotzenklotz hotzenklotz self-assigned this Mar 13, 2023
philippotto
philippotto approved these changes Mar 13, 2023
View reviewed changes
Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

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

Works well 👍 The next step would probably be to allow adding a user to a team from that page, too. But obviously not a blocker. See my one suggestion about DRYing things a bit, though.

frontend/javascripts/admin/team/team_list_view.tsx Outdated
Comment on lines 122 to 135
const tags = user.isAdmin
? [["Admin - Access to all Teams", "red"]]
: user.teams
.filter((team) => team.id === highlightedTeam.id)
.map((team) => {
const roleName = team.isTeamManager ? "Team Manager" : "Member";
return [`${roleName}`, stringToColor(roleName)];
});

return tags.map(([text, color]) => (
<Tag key={`${text}_${user.id}`} color={color} style={{ marginBottom: 4 }}>
{text}
</Tag>
));
Copy link
Member

Choose a reason for hiding this comment

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

could you DRY this with the corresponding code in the user_list_view?

Copy link
Member Author

@hotzenklotz hotzenklotz Mar 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That was my first thought too. But there are slight differences. On this page, the Dataset Manager role is ignored/not display as it has no meaning for Teams. Also the the team name is also no part of the Tag, e.g. Member vs Default - Member

Copy link
Member

Choose a reason for hiding this comment

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

I see. How about moving the similar functions into the same module (could be either the team list view or user list view) so that they are next to each other? I think, it's better to keep them together, so that the chance of divergence is reduced.

Copy link
Member Author

Choose a reason for hiding this comment

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

I move both render methods into one file. Please see commit c0546d3

Copy link
Member

Choose a reason for hiding this comment

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

great 👍 feel free to merge.

@hotzenklotz
Merge branch 'master' into team_members
0a336c2
@hotzenklotz
Merge branch 'master' of github.com:scalableminds/webknossos into tea…
7b91d56 
…m_members

* 'master' of github.com:scalableminds/webknossos:
  Release 23.03.1 (#6917)
  Redesign Welcome UI (#6904)
  Fix race condition which could disable saving (#6914)
@hotzenklotz
applied PR feedback
c0546d3
@hotzenklotz
Merge branch 'team_members' of github.com:scalableminds/webknossos in…
24b192d 
…to team_members

* 'team_members' of github.com:scalableminds/webknossos:
@hotzenklotz hotzenklotz merged commit 516a6ac into master Mar 15, 2023
@hotzenklotz hotzenklotz deleted the team_members branch March 15, 2023 08:27
hotzenklotz added a commit that referenced this pull request Mar 15, 2023
@hotzenklotz
Merge branch 'master' of github.com:scalableminds/webknossos into fi…
62cf6f8 
…x_login_styles

* 'master' of github.com:scalableminds/webknossos:
  Added members to admin team page (#6915)
hotzenklotz added a commit that referenced this pull request Mar 15, 2023
@hotzenklotz
Merge branch 'master' of github.com:scalableminds/webknossos into ema…
7f1ec66 
…il-notification

* 'master' of github.com:scalableminds/webknossos:
  Fix styling for login pages for dark mode users (#6916)
  Added members to admin team page (#6915)
hotzenklotz added a commit that referenced this pull request Mar 20, 2023
@hotzenklotz
Merge branch 'master' of github.com:scalableminds/webknossos into dro…
0879f2e 
…pdown-menu

* 'master' of github.com:scalableminds/webknossos:
  Fix download button for annotations when tiff export is disabled (#6931)
  Update PULL_REQUEST_TEMPLATE.md
  Prepare multi modality support (#6748)
  Improvements for terms-of-services modal (#6930)
  Fix creating task types with preferred mode (#6928)
  Fix styling for login pages for dark mode users (#6916)
  Added members to admin team page (#6915)
  Release 23.03.1 (#6917)
  Redesign Welcome UI (#6904)
  Fix race condition which could disable saving (#6914)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philippotto philippotto philippotto approved these changes

Assignees

@hotzenklotz hotzenklotz

Labels
frontend
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Show users in admin team list
2 participants
@hotzenklotz @philippotto