-
Notifications
You must be signed in to change notification settings - Fork 61
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
fix(ids-api): Filter delegated national registry scopes by delegation types. #17354
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces support for a new delegation type, Changes
Sequence DiagramsequenceDiagram
participant User
participant DelegationScopeService
participant NationalRegistry
User->>DelegationScopeService: findAllScopesTo(user, nationalId, delegationTypes)
DelegationScopeService->>DelegationScopeService: findAllNationalRegistryScopes(delegationTypes)
DelegationScopeService->>NationalRegistry: Query scopes based on delegation types
NationalRegistry-->>DelegationScopeService: Return applicable scopes
DelegationScopeService-->>User: Return scopes
Possibly related PRs
Suggested labels
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)apps/services/auth/public-api/src/app/modules/delegations/actorDelegations.controller.spec.ts (2)Pattern
Pattern
🔇 Additional comments (1)apps/services/auth/public-api/src/app/modules/delegations/actorDelegations.controller.spec.ts (1)
Providing an array for the ✅ Verification successfulLet me verify the usage of array in delegationType by checking the model and repository implementation. Let me try a different approach to verify the array usage in delegationType. Array usage in delegationType is correctly implemented and works as intended Based on the codebase analysis, the array usage for
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify that an array-based 'where' clause is treated as an 'IN' operation in all references to these delegation types.
rg -A 3 "delegationType:\s*\[.*"
Length of output: 11696 Script: #!/bin/bash
# Check model definition and repository implementation
ast-grep --pattern 'class $_ extends $_ {
$$$
delegationType
$$$
}'
# Check for destroy method implementation
ast-grep --pattern 'destroy($$$) {
$$$
}'
Length of output: 121 Script: #!/bin/bash
# Search for destroy operations with delegationType in test files
rg -B 5 -A 2 "destroy.*delegationType"
# Search for repository or service implementations handling delegationType
rg -B 3 -A 3 "delegationType.*\[.*\]" --type ts --glob '!*.spec.ts' --glob '!*test*'
Length of output: 19337 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
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/services/auth/ids-api/src/app/delegations/test/delegations-scopes.spec.ts (1)
22-22
: Add a clarifying comment for this new scope constant.
A short comment beforelegalGuardianMinorScopes
explaining its purpose and intended usage could help future contributors.libs/auth-api-lib/src/lib/delegations/delegation-scope.service.ts (1)
254-254
: Validate expected delegation types.
Ensure there's robust error handling ifdelegationTypes
is empty or includes unknown values.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/services/auth/ids-api/src/app/delegations/test/delegations-scopes.spec.ts
(3 hunks)libs/auth-api-lib/src/lib/delegations/delegation-scope.service.ts
(5 hunks)libs/services/auth/testing/src/fixtures/fixture-factory.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
libs/services/auth/testing/src/fixtures/fixture-factory.ts (2)
Pattern libs/services/**/*
: "Confirm that the code adheres to the following:
- Shared services and modules that can be utilized across NestJS apps.
- Proper implementation of NestJS providers, including custom providers.
- Adherence to NestJS module encapsulation and isolation principles."
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."
apps/services/auth/ids-api/src/app/delegations/test/delegations-scopes.spec.ts (2)
Pattern apps/services/**/*
: "Confirm that the code adheres to the following:
- NestJS architecture, including modules, services, and controllers.
- Dependency injection patterns and service encapsulation.
- Integration and unit testing coverage and practices."
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/auth-api-lib/src/lib/delegations/delegation-scope.service.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 (8)
apps/services/auth/ids-api/src/app/delegations/test/delegations-scopes.spec.ts (3)
30-30
: Good job extending the apiScopes
array.
Thanks for integrating the legalGuardianMinorScopes
.
49-51
: Approach validates new delegation type effectively.
This logic ensures that any scope falling under legalGuardianMinorScopes
is recognized as LegalGuardianMinor
.
116-127
: Great addition for combined delegation coverage testing.
The new test case clarifies the expected scope set when both LegalGuardian
and LegalGuardianMinor
delegations are present.
libs/auth-api-lib/src/lib/delegations/delegation-scope.service.ts (4)
6-6
: Remove duplicate or unused import references.
If kennitala
was already imported elsewhere, ensure not to duplicate. Otherwise, keep it as is.
31-31
: Confirm that filterByCustomScopeRule
is used consistently.
Since it is newly imported, verify that it does not conflict with or duplicate existing logic.
241-243
: Signature update ensures flexible filtering.
Revising findAllNationalRegistryScopes
to accept delegation types improves extensibility.
422-422
: Ensure correct usage of the newly added parameter.
Passing delegationTypes
to findAllNationalRegistryScopes
aligns with the new signature. Keep an eye on potential null or undefined delegation types.
libs/services/auth/testing/src/fixtures/fixture-factory.ts (1)
319-319
: Logical extension for minor guardians.
Including AuthDelegationType.LegalGuardianMinor
under NationalRegistry
is consistent with your new scopes.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17354 +/- ##
=======================================
Coverage 35.68% 35.68%
=======================================
Files 6931 6931
Lines 148782 148785 +3
Branches 42509 42510 +1
=======================================
+ Hits 53091 53097 +6
+ Misses 95691 95688 -3 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
What
Filter delegated national registry scopes by delegation types in scope check.
Why
So we grant correct scopes to legal guardians of youths (16-18 year old).
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
LegalGuardianMinor
, in delegation logic.getProvider
method to handle theLegalGuardianMinor
delegation type.LegalGuardian
andLegalGuardianMinor
.Bug Fixes
Tests