PER-9717 [back-end] Endpoint to retrieve environment configuration

[iulianvsp]

PER-9717 [back-end] Endpoint to retrieve environment configuration

[liam-lloyd]

You're generally on the right path here as far as how we write endpoints and what this one should do. I've added a couple comments in places where I think a better course exists.

You asked also whether there are any additional fields this endpoint should return; I've now added an OpenAPI specification describing the desired endpoint in detail (though that specification need not be taken as dogma; if you see a better way by all means present your case for it). If you prefer not to read the YAML directly, you can create a much nicer HTML version of the specification by running redocly build-docs packages/api/docs/present/api.yaml (we have just recently adopted this API specification format and aren't yet hosting a docs site).

Some other documentation that might give you useful further context:
The longer term plan for feature flags
Our best practices for API design

[iulianvsp]

You're generally on the right path here as far as how we write endpoints and what this one should do. I've added a couple comments in places where I think a better course exists.

You asked also whether there are any additional fields this endpoint should return; I've now added an OpenAPI specification describing the desired endpoint in detail (though that specification need not be taken as dogma; if you see a better way by all means present your case for it). If you prefer not to read the YAML directly, you can create a much nicer HTML version of the specification by running redocly build-docs packages/api/docs/present/api.yaml (we have just recently adopted this API specification format and aren't yet hosting a docs site).

Some other documentation that might give you useful further context: The longer term plan for feature flags Our best practices for API design

I updated the PR with the suggested changes, please have a look. I also saw in the best practices API design page that for lists of object we want to have pagination. Does this apply to the feature_flags as well?

Also should I create the other CRUD methods to this entity? Thanks

[liam-lloyd]

I also saw in the best practices API design page that for lists of object we want to have pagination. Does this apply to the feature_flags as well?

I think feature flags are most naturally an exception to this rule. From the user-facing client's perspective, it would be a hassle to have to make several calls through multiple pages of feature flags in order to determine which features should be active. From the admin panel's perspective, pagination might be more reasonable, but I think realistically we should never have so many feature flags at once that they would be unmanageable without pagination (unless we neglect to delete old ones, but then the lack of pagination may become a useful inducement to not neglect that).

Also should I create the other CRUD methods to this entity?

I think that would be a great thing for you to do once this endpoint is done; the create and update methods are higher priority than delete. The tickets for those endpoints are here:
https://permanent.atlassian.net/browse/PER-9718
https://permanent.atlassian.net/browse/PER-9866

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.67%. Comparing base (9e56697) to head (f2e0d9f).
Report is 11 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #127      +/-   ##
==========================================
- Coverage   88.12%   86.67%   -1.46%     
==========================================
  Files          75       78       +3     
  Lines        1347     1403      +56     
  Branches      207      213       +6     
==========================================
+ Hits         1187     1216      +29     
- Misses        146      171      +25     
- Partials       14       16       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[liam-lloyd]

Just one more comment, then I think this will be ready to merge!

[liam-lloyd]

I found one more thing while testing locally - this function will throw an exception (because of the way the Fusionauth client works) if it receives an authenticationToken that's an empty string. Since we don't want that case to fail, we should check for an empty authenticationToken and then only call this if it's not empty.