-
Notifications
You must be signed in to change notification settings - Fork 927
chore: remove global organization id state #14135
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
Conversation
🤖 Meticulous spotted visual differences in 458 of 944 screens tested: view and approve differences detected. Last updated for commit f7879a1. This comment will update as new commits are pushed. |
dashboard.experiments.includes("multi-organization") && | ||
selectFeatureVisibility(dashboard.entitlements).multiple_organizations; | ||
|
||
return hasMultipleOrganizations || organizationsEnabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the deployment has multiple organizations. but the organization's feature is not enabled, should this return true? The use case I'm seeing is a deployment that had access to enterprise and does not have it anymore after the trial.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's the goal. We don't want swaths of the app to become unaccessible because they're in the final stages of renewing a contract with us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good, and I think we have good coverage around the affected components. I'm approving it, and I will run a quick check afterward.
To run this in the new workspace version after upgrading to NodeJS 20+, we have to rebase this branch with |
Closes #14057
organizationId
from the dashboard provider