-
-
Notifications
You must be signed in to change notification settings - Fork 658
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
fix: prevent deleting the last variable variant on the ui #2964
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
...EnvironmentVariantsCard/EnvironmentVariantsTable/VariantsActionsCell/VariantsActionsCell.tsx
Show resolved
Hide resolved
return { patch: [], error: formatUnknownError(error) }; | ||
} | ||
const updatedNewVariants = updateWeight(newVariants, 1000); | ||
return { patch: createPatch(variants, updatedNewVariants) }; |
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 allows the error to bubble back to the caller, which gives us a nice pretty error message. This should be prevented by the other changes for this error path but other cases can also cause failures, in which case an error message would be pretty cool
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.
Awesome 😎
@@ -130,6 +131,23 @@ export const EnvironmentVariantsTable = ({ | |||
[] | |||
); | |||
|
|||
const isProtectedVariant = (variant: IFeatureVariant): boolean => { | |||
const isVariable = variant.weightType === 'variable'; |
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.
I think we have some weight type constants around somewhere if you'd like to use those instead of a literal string.
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 point, shot.
He wasn't exposed, so he got a new home in constants for reusing
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.
G!
This blocks the UI from allowing you to attempt to delete the last fixed variant in environment variants, this was previously possible, but the UI catch an error and then never make a request. For bonus points this removes the problematic try catch, which allows the error message to bubble correctly to the handler