-
Notifications
You must be signed in to change notification settings - Fork 20
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: extend BFF feature flag to take into account Waffle flag #1239
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1239 +/- ##
==========================================
- Coverage 89.05% 88.99% -0.07%
==========================================
Files 401 402 +1
Lines 8708 8721 +13
Branches 2123 2125 +2
==========================================
+ Hits 7755 7761 +6
- Misses 912 918 +6
- Partials 41 42 +1 ☔ View full report in Codecov by Sentry. |
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.
Great job tackling the removal of all the React
imports. One minor optional nit on coverage. Approved 👍🏽
export default function useIsBFFEnabled() { | ||
const { data: enterpriseCustomer } = useEnterpriseCustomer(); | ||
const { data: enterpriseFeatures } = useEnterpriseFeatures(); | ||
return isBFFEnabled(enterpriseCustomer.uuid, enterpriseFeatures); |
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.
[nit/optional] I know tests already exist for the isBFFEnabled
function, but because we have a new hook wrapping it, maybe just refactoring those tests to test for this hooks wrapper instead should pick this line up for coverage.
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.
Ah, yeah. Didn't see this was missing coverage. Will account for it in tests before merging. Thanks!
CHANGELOG
enterprise.learner_bff_enabled
Waffle flag exposed via theenterprise-learner
REST API from the LMS in its serializedenterprise_features
object.enterprise.learner_bff_enabled
Waffle flag istrue
for the user, use the BFF. This addition enables an incremental percentage based rollout to other enterprise learners beyond the learners explicitly opted in via the enterprise customer's configuration.import React
throughout codebase.For all changes
Only if submitting a visual change