-
Notifications
You must be signed in to change notification settings - Fork 26
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
feat: feature flag consumption #4489
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for partners-bloom-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for bloom-exygy-dev ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Tested out all of the endpoints and they work great! Just the one question about if we can change the type on the associate endpoint. But otherwise good, so approving!
); | ||
} | ||
|
||
@Get(`:featureFlagId`) |
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.
We don't have an explicit reason for it yet, but at some point we might want a get endpoint that takes in the "name" of the feature flag since we won't know the id. But that doesn't need to be done with this PR.
@Type(() => IdDTO) | ||
@IsArray({ groups: [ValidationsGroupsEnum.default] }) | ||
@ApiProperty({ type: IdDTO, isArray: true }) | ||
associate: IdDTO[]; |
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.
By having this be an IdDTO the api shows that it wants an "ordinal"
{
"id": "string",
"associate": [
{
"id": "string",
"name": "string",
"ordinal": 0
}
],
"remove": [
{
"id": "string",
"name": "string",
"ordinal": 0
}
]
}
Would this work if the type is string[]
instead?
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 wanted the validations on the id field which would be missing with a string[]
being passed in. This is the patten I could find elsewhere
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.
Is the validation just that it is a UUID? Are you not able to do the validation on string because it's an array?
This PR addresses #(4459)
Description
Creates the module, controller and service files for the new Feature Flag table. Allows for standard CRUD methods, as well as a list endpoint and an endpoint to add/remove jurisdiction associations with the flag (
associateJurisdictions
). Permissions are limited to admin only. Flags associated with jurisdictions will appear when jurisdiction data is returned.How Can This Be Tested/Reviewed?
Start up the backend. Login as an admin, either through the API or partners portal. Test each endpoint:
Author Checklist:
yarn generate:client
and/or created a migration when requiredReview Process: