feat: implement apply, update, and delete feature flag #10189
Conversation
| """ | ||
| The deleted feature flag | ||
| """ | ||
| featureFlag: FeatureFlagNew! |
There was a problem hiding this comment.
Once I remove the existing FeatureFlag, I'll change this from FeatureFlagNew to FeatureFlag
|
Hey @Dschoordsch, would you mind giving this a light PR review to double check it's on the right track? In the next PR, I'll use these mutations and types and refactor the code in the application. In that PR, I'll clean this up, pass the tests, and request a more thorough review. |
| """ | ||
| The name of the feature flag to apply | ||
| """ | ||
| flagName: String! |
There was a problem hiding this comment.
-1 Shouldn't this be also an enum like in DeleteFeatureFlagSucces?
There was a problem hiding this comment.
DeleteFeatureFlagSuccess returns the type featureFlag: FeatureFlagNew! (after the refactor it'll be FeatureFlag instead of FeatureFlagNew), which is the FeatureFlag object, not an enum
There was a problem hiding this comment.
Sorry, missed this type because I only checked this PR. All good then.
| """ | ||
| Delete an existing feature flag | ||
| """ | ||
| deleteFeatureFlag( |
There was a problem hiding this comment.
-1 I'm not sure what this mutation is supposed to do. Does it remove the feature flag from all subjects or does it delete the entry from the feature flag table? What happens to the static types in this case?
I think we probably don't need static types for feature flags but rather a field on Org, Team and User: hasFeatureFlag(flagName: String!): Boolean!. But make sure first that this actually would work.
There was a problem hiding this comment.
It removes the flag from the featureFlag table.
It's not strictly necessary as we can just update the expiry date of the flag so that it no longer applies.
I thought it'd be helpful to include so that we can clean up flag names in the db that we're no longer using, but I can remove this if it's confusing.
There was a problem hiding this comment.
I think that's fine then. It will also remove it from all owners. Maybe update the description to match it?
nickoferrall
requested review from
Dschoordsch
and removed request for
Dschoordsch
| """ | ||
| Delete an existing feature flag | ||
| """ | ||
| deleteFeatureFlag( |
There was a problem hiding this comment.
I think that's fine then. It will also remove it from all owners. Maybe update the description to match it?
|
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size. |
No description provided.