fix(api): add ff to resolver #16865
fix(api): add ff to resolver #16865
Conversation
WalkthroughThe changes in this pull request involve the integration of a new Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
libs/feature-flags/src/lib/features.ts (1)
48-49: Add JSDoc comments to document the feature flags.The new feature flags follow the established naming conventions and are correctly placed in the service portal modules section. Consider adding JSDoc comments to document their purpose and usage:
+ /** Controls the visibility of vaccinations page in the service portal's health section */ servicePortalHealthVaccinationsPageEnabled = 'isServicePortalHealthVaccinationsPageEnabled', + /** Controls the visibility of organ donation page in the service portal's health section */ servicePortalHealthOrganDonationPageEnabled = 'isServicePortalHealthOrganDonationPageEnabled',
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
libs/api/domains/health-directorate/src/lib/health-directorate.module.ts(1 hunks)libs/api/domains/health-directorate/src/lib/health-directorate.resolver.ts(4 hunks)libs/feature-flags/src/lib/features.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
libs/api/domains/health-directorate/src/lib/health-directorate.module.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/api/domains/health-directorate/src/lib/health-directorate.resolver.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/feature-flags/src/lib/features.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (5)
libs/api/domains/health-directorate/src/lib/health-directorate.module.ts (1)
5-8: LGTM! Clean integration of feature flags.
The addition of FeatureFlagModule is well-structured and follows NestJS best practices. This change appropriately supports the feature flag implementation in the resolver.
Let's verify the feature flag implementation across the codebase:
✅ Verification successful
Feature flag integration is properly implemented and verified
The verification confirms that:
FeatureFlagModuleis correctly imported and used in the module- The resolver properly implements feature flags using
@FeatureFlagdecorator with the correct feature flagservicePortalHealthOrganDonationPageEnabled - The feature flag is properly defined in the features enum
- The implementation follows the standard pattern with
@UseGuards(FeatureFlagGuard)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the feature flag implementation in related files
# Expected: Find corresponding feature flag usage in resolver and proper configuration
# Check resolver implementation
ast-grep --pattern 'class HealthDirectorateResolver {
$$$
@UseGuards(FeatureFlagGuard)
$$$
}'
# Check feature flag configuration
rg -l "servicePortalHealthOrganDonationPageEnabled|servicePortalHealthVaccinationsPageEnabled" --type ts
Length of output: 322
Script:
#!/bin/bash
# Let's check the actual implementation in the resolver and feature flags
rg -A 5 "@UseGuards\(FeatureFlagGuard\)" libs/api/domains/health-directorate/src/lib/health-directorate.resolver.ts
# Check feature flags configuration
cat libs/feature-flags/src/lib/features.ts
Length of output: 6055
Script:
#!/bin/bash
# Let's check the resolver implementation one more time with more context
rg -B 5 -A 10 "HealthDirectorateResolver" libs/api/domains/health-directorate/src/lib/health-directorate.resolver.ts
Length of output: 694
libs/api/domains/health-directorate/src/lib/health-directorate.resolver.ts (3)
25-29: LGTM! Imports are well-structured
The feature flag imports are correctly organized and follow tree-shaking best practices.
30-30: LGTM! Guard ordering is correct
The FeatureFlagGuard is correctly placed after authentication and scope guards, ensuring proper authorization flow.
42-42: Consider adding feature flag to resolveOrganList method
While the main organ donation endpoints are protected by feature flags, the resolveOrganList resolver field is not. Since it's part of the organ donation functionality, consider adding the same feature flag for consistency.
Let's verify the feature flag usage:
Also applies to: 66-66, 81-81
libs/feature-flags/src/lib/features.ts (1)
48-49: Verify feature flag usage in the resolver.
Let's verify that these feature flags are properly implemented in the resolver and that no unguarded routes exist.
✅ Verification successful
Let me check the resolver implementation to verify the feature flag usage.
Feature flags are properly implemented in the resolver
The verification shows that both feature flags are correctly used:
@FeatureFlag(Features.servicePortalHealthOrganDonationPageEnabled)guards bothgetDonorStatusandupdateDonorStatusmethods@FeatureFlag(Features.servicePortalHealthVaccinationsPageEnabled)guards thegetVaccinationsmethod
All health-related resolver methods are properly protected with their respective feature flags.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify feature flag implementation in resolver and routes
# Check for feature flag usage in resolver
echo "Checking feature flag usage in resolver..."
ast-grep --pattern 'class $_ {
$$$
@FeatureFlag($flag)
$$$
get$_($$_) {
$$$
}
$$$
}'
# Check for any unguarded health-related routes
echo "Checking for unguarded health-related routes..."
rg -l "servicePortalHealth(Vaccinations|OrganDonation)" --type ts
Length of output: 484
Script:
#!/bin/bash
# Check the implementation in the resolver
echo "Checking resolver implementation..."
rg -A 10 "servicePortalHealth(Vaccinations|OrganDonation)" libs/api/domains/health-directorate/src/lib/health-directorate.resolver.ts
# Check for any routes or components using these feature flags
echo "Checking for routes and components..."
rg -A 5 "isServicePortalHealth(Vaccinations|OrganDonation)PageEnabled"
Length of output: 2305
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16865 +/- ##
=======================================
Coverage 36.44% 36.45%
=======================================
Files 6851 6851
Lines 143567 143577 +10
Branches 40980 40976 -4
=======================================
+ Hits 52328 52340 +12
+ Misses 91239 91237 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 68 Total Test Services: 0 Failed, 66 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* fix(my-pages): health vaccination tags & locale (#16567) * fix: tags * feat: add locale to service WIP * feat: add locale from service * feat: add locale to query call * fix(api): add ff to resolver (#16865) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(api): add correct scope (#16874) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Api - Health Directorate
What
Add feature flag to resolver
Checklist:
Summary by CodeRabbit
New Features
FeatureFlagModuleto enhance feature management.servicePortalHealthVaccinationsPageEnabledandservicePortalHealthOrganDonationPageEnabled.HealthDirectorateResolverwith feature flags for specific methods, allowing granular access control.Bug Fixes
FeatureFlagGuardto the resolver's guards.