feat(auth-admin): General mandate delegation type #16005
Conversation
WalkthroughThis pull request introduces a new delegation type, "GeneralMandate," into the database and updates various services to manage this type. A migration script adds the delegation type to the database schema, while the Changes
Possibly related PRs
Suggested reviewers
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- libs/auth-api-lib/migrations/20240913092844-GeneralMandate-delegation-type.js (1 hunks)
- libs/auth-api-lib/src/lib/clients/clients.service.ts (2 hunks)
- libs/auth-api-lib/src/lib/delegations/delegation-provider.service.ts (2 hunks)
- libs/auth-api-lib/src/lib/resources/admin/admin-scope.service.ts (2 hunks)
- libs/shared/types/src/lib/delegation.ts (1 hunks)
Additional context used
Path-based instructions (5)
libs/shared/types/src/lib/delegation.ts (2)
Pattern
libs/shared/**/*: "Confirm that the code adheres to the following:
- Cross-application utility functions, types, and constants.
- TypeScript best practices to ensure type safety and reusability.
- Documentation and examples for library consumers."
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/auth-api-lib/migrations/20240913092844-GeneralMandate-delegation-type.js (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/auth-api-lib/src/lib/delegations/delegation-provider.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."
libs/auth-api-lib/src/lib/resources/admin/admin-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."
libs/auth-api-lib/src/lib/clients/clients.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."
Biome
libs/auth-api-lib/migrations/20240913092844-GeneralMandate-delegation-type.js
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
Additional comments not posted (7)
libs/shared/types/src/lib/delegation.ts (1)
7-7: LGTM!The addition of the
GeneralMandateenum value aligns with the PR objective of introducing a new delegation type. The enum value name is clear and descriptive.As this enum is part of the shared types library, it adheres to the guidelines for:
- Cross-application reusability
- Type safety
- Documentation (although an explanatory comment could be beneficial for future maintainers)
Overall, this is a good addition to the
AuthDelegationTypeenum.libs/auth-api-lib/migrations/20240913092844-GeneralMandate-delegation-type.js (1)
4-16: LGTM!The migration script aligns with the PR objective and follows best practices:
- It introduces the new "GeneralMandate" delegation type and associates it with clients and API scopes that currently use the "Custom" type.
- It uses SQL transactions to ensure data consistency.
- It provides a rollback script to undo the changes.
The script is also reusable across different NextJS apps, adhering to the additional instructions for libs/** files.
Also applies to: 18-30
libs/auth-api-lib/src/lib/delegations/delegation-provider.service.ts (1)
25-29: LGTM! The filtering condition is correctly implemented.The added
whereclause in the association with thedelegationTypeModeleffectively filters out entries where theidmatchesAuthDelegationType.GeneralMandate. This ensures that the "General Mandate" delegation type is excluded from the results returned by thefindAllmethod.The use of the
Op.neoperator is appropriate for achieving the desired exclusion. The comment also provides clarity on the purpose behind this change, which is to prevent the "General Mandate" delegation type from being displayed in the UI.libs/auth-api-lib/src/lib/resources/admin/admin-scope.service.ts (2)
462-488: LGTM!The code correctly upserts each
delegationTypeinto theApiScopeDelegationTypemodel and establishes the one-to-one relationship between 'Custom' and 'GeneralMandate' types as per the PR objectives. It also adheres to the reusability and TypeScript usage guidelines for thelibsdirectory.
551-577: LGTM!The code correctly destroys each
delegationTypefrom theApiScopeDelegationTypemodel and maintains the one-to-one relationship between 'Custom' and 'GeneralMandate' types as per the PR objectives. It also adheres to the reusability and TypeScript usage guidelines for thelibsdirectory.libs/auth-api-lib/src/lib/clients/clients.service.ts (2)
621-646: LGTM!The changes to
addClientDelegationTypesenhance the functionality by ensuring that both the specified delegation type and an additional instance ofAuthDelegationType.GeneralMandateare managed simultaneously when the delegation type isAuthDelegationType.Custom. This improves the integrity of the delegation type handling.The code adheres to the reusability and modularity guidelines by utilizing the
AuthDelegationTypeenum andClientDelegationTypemodel. The changes are logically correct and the code is readable.
703-728: LGTM!The changes to
removeClientDelegationTypesensure that when a custom delegation type is removed, its associated general mandate is also appropriately handled by deleting an instance ofAuthDelegationType.GeneralMandate. This maintains consistency in the data model.The code adheres to the reusability and modularity guidelines by utilizing the
AuthDelegationTypeenum andClientDelegationTypemodel. The changes are logically correct and the code is readable.
libs/auth-api-lib/migrations/20240913092844-GeneralMandate-delegation-type.js
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16005 +/- ##
==========================================
+ Coverage 36.73% 36.75% +0.01%
==========================================
Files 6735 6736 +1
Lines 138283 138316 +33
Branches 39323 39344 +21
==========================================
+ Hits 50799 50833 +34
+ Misses 87484 87483 -1 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 81 Total Test Services: 0 Failed, 79 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (7)
|
libs/auth-api-lib/src/lib/resources/admin/admin-scope.service.ts
Outdated
Show resolved
Hide resolved
libs/auth-api-lib/src/lib/resources/admin/admin-scope.service.ts
Outdated
Show resolved
Hide resolved
GunnlaugurG
added
the
deprecated:automerge
What
Added General mandate delegation type with 1 to 1 connection to custom delegations.
Why
So we can use this for 'Allsherjarumboð' instead of the Custom delegation type
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
AuthDelegationTypeenum to include the new "GeneralMandate" type for better categorization.