Skip to content
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

REST API - Features Create & Update endpoints #1478

Merged
merged 21 commits into from Oct 10, 2023
Merged

REST API - Features Create & Update endpoints #1478

merged 21 commits into from Oct 10, 2023

Conversation

bttf
Copy link
Collaborator

@bttf bttf commented Jul 18, 2023

This PR adds two endpoints for Feature Flags

  • POST /features (create)
  • POST /features/:id (update)

Changes

  • New schemas:
    • PostFeaturePayload.yaml
    • UpdateFeaturePayload.yaml
    • Under postFeature/ subdirectory (necessary due to required fields differing vs. read operation)
      • FeatureEnvironment.yaml
      • FeatureExperimentRule.yaml
      • FeatureForceRule.yaml
      • FeatureRolloutRule.yaml
      • FeatureRule.yaml
  • Adds following methods for creating and updating environment settings for a feature
    • fromApiEnvSettingsRulesToFeatureEnvSettingsRules
    • createInterfaceEnvSettingsFromApiEnvSettings
    • updateInterfaceEnvSettingsFromApiEnvSettings

Screenshots

Create
Screenshot 2023-07-17 at 9 17 32 PM

Update
Screenshot 2023-07-17 at 11 27 41 PM

@github-actions
Copy link

github-actions bot commented Jul 18, 2023

Deploy preview for docs ready!

✅ Preview
https://docs-ac2viunxi-growthbook.vercel.app

Built with commit 468b4d3.
This pull request is being automatically deployed with vercel-action

@github-actions
Copy link

github-actions bot commented Jul 18, 2023

Your preview environment pr-1478-bttf has been deployed.

Preview environment endpoints are available at:

@bttf bttf changed the base branch from main to adnan/validate-features July 28, 2023 23:27
@bttf bttf added the blocked label Jul 31, 2023
@bttf
Copy link
Collaborator Author

bttf commented Jul 31, 2023

PR is blocked by #1139 & #1525

Base automatically changed from adnan/validate-features to main August 18, 2023 16:39
@igorlovich
Copy link

igorlovich commented Aug 28, 2023

PR is blocked by #1139 & #1525

Both of these have been merged, should the blocked label be removed ?

@igorlovich
Copy link

@bttf what is the priority of this item in your backlog ?

@bttf bttf removed the blocked label Oct 2, 2023
@bttf
Copy link
Collaborator Author

bttf commented Oct 2, 2023

@igorlovich Sorry for the delay. There have been extensive changes to our Features <> Experiments relationships. (And I've also been on vacation.) This is getting attention now.

@bttf
Copy link
Collaborator Author

bttf commented Oct 5, 2023

@jdorn Following our conversation re: license key checks, the most recent commit makes the following changes so that we always enforce license key checking for both internal API requests and REST API requests.

  1. In processJWT (our auth middleware responsible for loading req.organization for the internal API), I've added a licenseInit call that supplies req.organization.licenseKey, which will be set only if the org has the license key stored on its mongoDB row. Otherwise, licenseInit should fallback to the env var, and simply bypass if none is found
  2. With licenseInit, the subsequent verifyLicenseMiddleware should now always verify either env-var OR mongoDB sourced license keys
  3. For the REST API, I've added a similar licenseInit call to the middleware responsible for loading the organization based on the API Key
  4. I've also added subsequent verifyLicenseMiddleware to the REST API middleware chain
  5. With verifyLicenseMiddleware in the REST API chain, we can now check for res.locals.licenseError in our REST API endpoints. I've added both checks to postFeature and updateFeature accordingly

@bttf
Copy link
Collaborator Author

bttf commented Oct 6, 2023

Tested creating an experiment ref rule
Screenshot 2023-10-06 at 11 31 50 AM

incomingEnvs: ApiFeatureEnvSettings
): FeatureInterface["environmentSettings"] => {
const existing = feature.environmentSettings;
return Object.keys(incomingEnvs).reduce((acc, k) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the feature currently has environmentSettings for both dev and production, but the update call only specifies changes for production? If I'm following the logic right, I think this will delete the existing dev settings, which we don't want to do. Is that right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a feature has envSettings for both dev and production, and the update only includes production, it will not override dev. We use existing to hold existing envSettings and use that as the initial value for acc in the reduce fn, so we only overwrite what is supplied by the update. Additionally, if the update only includes the enabled field for an envSetting (and doesn't specify the rules), we do not erase the rules but include the existing set of rules with the new enabled value ... Let me know if that makes sense

return Object.keys(incomingEnvs).reduce((acc, k) => {
return {
...acc,
[k]: {
enabled: incomingEnvs[k].enabled ?? existing[k].enabled,
rules: incomingEnvs[k].rules
? fromApiEnvSettingsRulesToFeatureEnvSettingsRules(
feature,
incomingEnvs[k].rules
)
: existing[k].rules,
},
};
}, existing);

@bttf bttf merged commit 34f2b51 into main Oct 10, 2023
5 checks passed
@bttf bttf deleted the adnan/features-crud branch October 10, 2023 18:04
@bttf
Copy link
Collaborator Author

bttf commented Oct 10, 2023

@igorlovich This is now live.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants