-
-
Notifications
You must be signed in to change notification settings - Fork 740
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
chore: move enterprise check further left, prevent OSS from seeing CR usage #5431
chore: move enterprise check further left, prevent OSS from seeing CR usage #5431
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
This is just a refactor to remove duplication.
If we change the payload to always include changeRequestStrategies
, we can remove the flag check entirely. For now, this flag check retains the previous shape of the object.
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.
Only look up change request strategies if it's relevant. Meaning: only if the flag is enabled (for now) and if we're on an enterprise instance.
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.
Nice!
This PR checks that the unleash instance is an enterprise instance before fetching change request data. This is to prevent Change Request usage from preventing OSS users from deleting segments (when they don't have access to change requests).
This PR also does a little bit of refactoring (which we can remove if you want)