feat: add waffle flag states rest api and util#358
Conversation
|
Thanks for the pull request, @MaferMazu! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
|
I still need to add the tests, but that's the base. |
- get_waffle_flag_states used a manual Flag query for the global tier instead of enable_authz_course_authoring, and returned flat booleans for org/course overrides instead of the actual affected orgs/courses, split by whether the override forces the flag on or off. - Adds test coverage for get_waffle_flag_states and WaffleFlagStatesAPIView, and an ADR documenting why this endpoint (issue #358) supersedes PR #361's approach of enforcing the flag cascade inside release-blocking endpoints. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- get_waffle_flag_states used a manual Flag query for the global tier instead of enable_authz_course_authoring, and returned flat booleans for org/course overrides instead of the actual affected orgs/courses, split by whether the override forces the flag on or off. - Adds test coverage for get_waffle_flag_states and WaffleFlagStatesAPIView, and an ADR documenting why this endpoint (issue #358) supersedes PR #361's approach of enforcing the flag cascade inside release-blocking endpoints. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Here how the changes could look like: 567e9a0. I also added an ADR explaining why this is how we're moving forward. |
- get_waffle_flag_states used a manual Flag query for the global tier instead of enable_authz_course_authoring, and returned flat booleans for org/course overrides instead of the actual affected orgs/courses, split by whether the override forces the flag on or off. - Adds test coverage for get_waffle_flag_states and WaffleFlagStatesAPIView, and an ADR documenting why this endpoint (issue openedx#358) supersedes PR openedx#361's approach of enforcing the flag cascade inside release-blocking endpoints. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@mariajgrimaldi, thanks a lot for your commit. I cherry-picked it. The only thing is, I reverted the change to use |
dcoa
left a comment
There was a problem hiding this comment.
I check and looks good to me, thanks Mafer.
|
@MaferMazu it should work with this PR: https://github.com/openedx/openedx-platform/pull/38855/changes#diff-201a5011ca69071dd6ad00c38d5dd9741add9ee1d780325c11f1631887e37d41R46, the idea is to decrease the dependency on other models as much as we can |
BryanttV
left a comment
There was a problem hiding this comment.
I tested it locally with the latest changes in openedx-platform and it works as expected. Thanks!
because that needs an argument
c1d1ed4 to
b9fa3a2
Compare
|
Thanks for the feedback. I updated the adr as mentioned and also reverted to using Note: this change depends on https://github.com/openedx/openedx-platform/pull/38855/changes#diff-201a5011ca69071dd6ad00c38d5dd9741add9ee1d780325c11f1631887e37d41R46 |
mariajgrimaldi
left a comment
There was a problem hiding this comment.
LGTM! Thanks a lot for this
|
@mariajgrimaldi I found out that using the direct The final version would be like this: 1cb3457 Context of the errors I get using Removing the try except to understand why the object is None: |
|
@mariajgrimaldi @BryanttV I dropped the revert and maintain the use of Flags to ensure a proper answer. Details: #358 (comment) |
|
Thanks, @MaferMazu. I agree with your comment. I tested again, and everything worked fine. |
Description
This PR adds a util and a REST API to check whether the waffle flag related to authz (
authz.enable_course_authoring) is enabled.Those functions return a dictionary mapping scopes to their activation status:
(forces the flag on) and 'off' (forces the flag off) lists.
This PR is related to:
And try to solve:
📌 We still need to modify the REST API endpoints to provide the correct information to the front end, but this is an effort to centralize the waffle flag information.
How to test
More about how to activate the waffle flags: openedx/openedx-platform#37985
More context on how it should work
Operator notes: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/6331662350/RBAC+AuthZ+for+Course+Authoring+-+Operator+Release+Notes
Merge checklist:
Check off if complete or not applicable: