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

Show team usage tab only for team owners #15350

Merged
merged 1 commit into from
Dec 15, 2022
Merged

Show team usage tab only for team owners #15350

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion components/dashboard/src/Menu.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,10 @@ export default function Menu() {
link: `/t/${team.slug}/members`,
},
];
if (showUsageView || (teamBillingMode && teamBillingMode.mode === "usage-based")) {
if (
currentUserInTeam?.role === "owner" &&
(showUsageView || (teamBillingMode && teamBillingMode.mode === "usage-based"))
) {
Comment on lines +234 to +237
Copy link
Member

Choose a reason for hiding this comment

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

This would hide it from the UI, but leaves the APIs accessible. To actually restrict access, we need to change the APIs.

Would it make sense to continue showing it but show "You need to be a team Owner to access this view"? This helps with discovery, and action on what is needed to use it.

Copy link
Contributor Author

@gtsiolis gtsiolis Dec 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This would hide it from the UI, but leaves the APIs accessible.

Oh, I didn't know that—thanks for the heads-up @easyCZ! 🏀

Would it make sense to continue showing it ...

We already show a message like this:

gitpod/components/dashboard/src/components/UsageView.tsx

Line 85 in 40e78a2

setErrorMessage("Access to usage details is restricted to team owners.");

However, I see no benefit or much value for users to show this access wall, as there's not much (action) they can do to access, and it's causing more confusion than help.

Cross-posting from the issue description in #11588:

@gtsiolis: Promoting a feature by making the usage tab visible and making the feature inaccessible not because of the user's subscription, but because of their team role can be confusing as it will probably be impossible for a user to change their role as this contradicts the initial goal of having roles in teams. This issue will become more obvious for larger teams which usually require specific role access so that they can control data access, and more.

Cross-postng from the relevant discussion (internal):

@gtsiolis: Having this tab visible but not its content was a not-so-cool decision we took that we need to fix. I’ve also received negative feedback about this feature behavior in private conversations with other team members but we can fix this by either making this tab visible for all members or hiding the tab and improve usage attribution. ❗

I'd suggest to 🅰️ also limit API access to usage to team owners or 🅱️ show usage to all team members. Happy to discuss more about this and roles in general, or go with an MVC that requires the least engineering effort for this iteration, but would be nice to have some product input or direction on this.

Copy link
Member

Choose a reason for hiding this comment

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

I understood that we are already returning an error on the API but still show the UI. So hiding the UI seems to be the right thing to do here. In anyway I'd say we should merge this PR and do additional follow up work separately.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @svenefftinge, wasn't aware there's already a guard for this. Makes sense to proceed with this!

teamSettingsList.push({
title: "Usage",
link: `/t/${team.slug}/usage`,
Expand Down