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

Hide specific features from nav is dashboard tenant #51103

Merged
merged 1 commit into from
Jan 16, 2025
Merged

Conversation

avatus
Copy link
Contributor

@avatus avatus commented Jan 16, 2025

This PR makes an update to our logic to the "backdoor" that would keep things hidden after making updates to show most of the features to promote discoverability. Currently, we only continued to hide them if their license specified it, but this incorrectly ignored dashboard, usage based, and team tenants as well.

// are usually "always visible"
export function shouldHideFromNavigation(cfg: {
isTeam: boolean;
isUsageBasedBilling: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Most subscriptions nowadays should have isUsageBasedBilling=true IIRC. The main exceptions are OSS and 14-day trials.

This means that shouldHideFromNavigation will return true for most customers, is this what we want?

// However, there are some cases where hiding the feature is explicitly requested. Use this as a backdoor to hide the features that
// are usually "always visible"
export function shouldHideFromNavigation(cfg: {
isTeam: boolean;
Copy link
Contributor

@mcbattirola mcbattirola Jan 16, 2025

Choose a reason for hiding this comment

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

I think isTeam is hardcoded to false now, we use instead isStripeManaged.

We refactored things not to need to check isStripeManaged most of the time. We should use the entitlements instead. So I think we can remove isTeam from this function.

For reference, isStripeManaged is used for Stripe-specific things, like showing a UI to add a credit card, downloading invoices, etc.

This PR makes an update to our logic to the "backdoor" that would keep
things hidden after making updates to show most of the features to
promote discoverability. Currently, we only continued to hide them if
their license specified it, but this incorrectly ignored dashboard,
usage based, and team tenants as well.
@avatus avatus requested a review from mcbattirola January 16, 2025 14:50
@avatus
Copy link
Contributor Author

avatus commented Jan 16, 2025

Ok, updated. thanks @mcbattirola

@avatus avatus added this pull request to the merge queue Jan 16, 2025
Merged via the queue into master with commit 60ff7d4 Jan 16, 2025
41 checks passed
@avatus avatus deleted the avatus/fix_perms branch January 16, 2025 16:11
@public-teleport-github-review-bot

@avatus See the table below for backport results.

Branch Result
branch/v16 Failed
branch/v17 Create PR

avatus added a commit that referenced this pull request Jan 16, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jan 16, 2025
mvbrock pushed a commit that referenced this pull request Jan 18, 2025
This PR makes an update to our logic to the "backdoor" that would keep
things hidden after making updates to show most of the features to
promote discoverability. Currently, we only continued to hide them if
their license specified it, but this incorrectly ignored dashboard,
usage based, and team tenants as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants