fix(delegations): Fix delete for GeneralMandate delegations #16212
Conversation
WalkthroughThe changes in the pull request focus on the Changes
Possibly related PRs
Suggested labels
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: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- libs/portals/shared-modules/delegations/src/components/access/AccessDeleteModal/AccessDeleteModal.tsx (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/portals/shared-modules/delegations/src/components/access/AccessDeleteModal/AccessDeleteModal.tsx (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 (1)
libs/portals/shared-modules/delegations/src/components/access/AccessDeleteModal/AccessDeleteModal.tsx (1)
45-45: Robust handling of undefined delegation domainGood use of the nullish coalescing operator to handle cases where
delegation.domainmight be undefined, ensuring thatdomainis set tonullwhen necessary.
...als/shared-modules/delegations/src/components/access/AccessDeleteModal/AccessDeleteModal.tsx
Outdated
Show resolved
Hide resolved
Datadog ReportAll test runs ❌ 68 Total Test Services: 1 Failed, 65 Passed Test ServicesThis report shows up to 10 services
❌ Failed Tests (1)
🔻 Code Coverage Decreases vs Default Branch (2) |
...als/shared-modules/delegations/src/components/access/AccessDeleteModal/AccessDeleteModal.tsx
Outdated
Show resolved
Hide resolved
...als/shared-modules/delegations/src/components/access/AccessDeleteModal/AccessDeleteModal.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
libs/portals/shared-modules/delegations/src/components/access/AccessDeleteModal/AccessDeleteModal.tsx (1)
Line range hint
43-50: Improve null safety in useEffect conditionThe condition for fetching the auth scope tree has been improved, but we can further enhance it using optional chaining for better null safety.
Apply this diff to improve the condition:
- if (delegation && delegation.domain?.name) { + if (delegation?.domain?.name) { getAuthScopeTree({ variables: { input: { domain: delegation.domain.name, lang, }, }, }) }This change ensures that we safely access the
domainproperty ofdelegationusing optional chaining, making the code more robust against potential null or undefined values.🧰 Tools
🪛 Biome
[error] 43-43: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- libs/portals/shared-modules/delegations/src/components/access/AccessDeleteModal/AccessDeleteModal.tsx (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/portals/shared-modules/delegations/src/components/access/AccessDeleteModal/AccessDeleteModal.tsx (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/portals/shared-modules/delegations/src/components/access/AccessDeleteModal/AccessDeleteModal.tsx
[error] 43-43: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
libs/portals/shared-modules/delegations/src/components/access/AccessDeleteModal/AccessDeleteModal.tsx (4)
Line range hint
1-20: LGTM: Import statements are correctly updated and ordered.The addition of
AuthDelegationTypeimport is appropriate for the changes made in the component. The import order follows the coding guidelines.
138-160: LGTM: Improved rendering logic for different delegation typesThe changes effectively address the need to handle different delegation types, particularly distinguishing between GeneralMandate and other types. This aligns well with the PR objectives to fix deletion for GeneralMandate delegations.
The conditional rendering is clear, concise, and follows React best practices. Good job on improving the component's flexibility and accuracy in displaying delegation information.
138-138: LGTM: Correct usage of AuthDelegationType enumThe change from string literal to
AuthDelegationType.GeneralMandatefor type comparison is excellent. This ensures type safety and maintains consistency throughout the codebase.
Line range hint
1-179: LGTM: Overall improvements to AccessDeleteModal componentThe changes in this file successfully address the PR objectives and previous review comments. Key improvements include:
- Correct handling of different delegation types, especially GeneralMandate.
- Enhanced conditional rendering based on delegation type.
- Improved null safety in the useEffect hook (with a suggested further improvement).
- Proper use of AuthDelegationType enum for type comparisons.
These changes enhance the component's functionality and maintainability. Great job on the improvements!
* Fix delete * fix hardcoded value * fix pr comments --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Fix the AccessDeleteModal to handle general mandate delegations
Why
So all type of delegations can be deleted
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes