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

Show team usage tab only for team owners #15350

Merged
merged 1 commit into from
Dec 15, 2022

Conversation

gtsiolis
Copy link
Contributor

@gtsiolis gtsiolis commented Dec 14, 2022

Description

Not sure if the code changes here will work, but this will attempt to fix #11588.

Any hints or help is appreciated. 🏀

See relevant discussion (internal). Cc @loujaybee

Related Issue(s)

Fixes #11588

How to test

  1. Join a team as a non-owner and notice that the team Usage tab is no longer visible for you.

Release Notes

Show team usage tab only to team owners

Documentation

Werft options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-gt-show-usage-tab-only-for-team-owners.2 because the annotations in the pull request description changed
(with .werft/ from main)

Comment on lines +234 to +237
if (
currentUserInTeam?.role === "owner" &&
(showUsageView || (teamBillingMode && teamBillingMode.mode === "usage-based"))
) {
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

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:

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!

Copy link
Member

@svenefftinge svenefftinge left a comment

Choose a reason for hiding this comment

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

LGTM (see comment inline)

@roboquat roboquat merged commit 9bfcd07 into main Dec 15, 2022
@roboquat roboquat deleted the gt/show-usage-tab-only-for-team-owners branch December 15, 2022 07:55
@gtsiolis
Copy link
Contributor Author

gtsiolis commented Dec 15, 2022

Thanks @svenefftinge!

Opened #15361 to keep track of the discussion around API access, feel free to update or close if needed. 🏓 Cc @easyCZ

@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/XS team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider hiding usage tab for team members who are not owners
4 participants