fix(service-portal): Filtering mailbox documents #16219
Conversation
WalkthroughThe changes in this pull request involve significant updates to the document handling and user management functionalities within the application. Key modifications include altering method signatures to accept user objects instead of national IDs, enhancing validation checks for document actions, and refining document retrieval logic based on user roles. Additionally, some properties and utility functions related to user data have been removed or simplified, streamlining the overall structure while maintaining existing functionalities. 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: 1
🧹 Outside diff range and nitpick comments (2)
libs/auth/react/src/lib/auth/Auth.state.ts (1)
Line range hint
1-89: Overall assessment: Improved code simplicity with potential implications.The changes in this file consistently remove the
formatUserfunction and simplify user info assignment. These modifications align well with the coding guidelines for thelibsdirectory, improving code simplicity and maintainability.However, it's crucial to consider the following:
- The removal of the
dateOfBirthproperty might affect other parts of the codebase that expect this field to be present.- TypeScript types may need to be updated to reflect the new structure of the
userInfoobject.To ensure a smooth transition:
- Conduct a thorough review of components and functions that interact with the
userInfoobject, particularly those that might have relied on thedateOfBirthproperty.- Update relevant documentation to reflect the changes in the
userInfostructure.- Consider adding a migration guide or update notes for other developers working on the project.
libs/api/domains/documents/src/lib/documentV2.resolver.ts (1)
Line range hint
146-151: Improved input validation for postMailActionThe addition of input validation for
input.documentIdsis a great improvement. It prevents unnecessary processing when no document IDs are provided and adds a helpful warning log for debugging purposes.To maintain consistency with the rest of the codebase, consider using a constant for the log category:
this.logger.warn('No document ids provided for posting action', { category: LOG_CATEGORY, action: input.action, })This change adheres to the coding guidelines by enhancing the overall quality and maintainability of the code.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
- libs/api/domains/documents/src/lib/documentV2.resolver.ts (1 hunks)
- libs/api/domains/documents/src/lib/documentV2.service.ts (2 hunks)
- libs/api/domains/documents/src/lib/models/v2/documents.input.ts (0 hunks)
- libs/auth/react/src/lib/auth/Auth.state.ts (1 hunks)
- libs/auth/react/src/lib/utils/getBirthday.spec.ts (0 hunks)
- libs/service-portal/documents/src/hooks/useDocumentList.ts (0 hunks)
💤 Files with no reviewable changes (3)
- libs/api/domains/documents/src/lib/models/v2/documents.input.ts
- libs/auth/react/src/lib/utils/getBirthday.spec.ts
- libs/service-portal/documents/src/hooks/useDocumentList.ts
🧰 Additional context used
📓 Path-based instructions (3)
libs/api/domains/documents/src/lib/documentV2.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/api/domains/documents/src/lib/documentV2.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/react/src/lib/auth/Auth.state.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/auth/react/src/lib/auth/Auth.state.ts (2)
52-53: LGTM! Simplified user info assignment.The direct assignment of
action.payloadtouserInfosimplifies the code and aligns with the coding guidelines for reusability and effective tree-shaking. This change removes the previously addeddateOfBirthproperty derived from the national ID.To ensure this change doesn't introduce unexpected behavior, please run the following script to check for any usage of
dateOfBirthin the codebase:#!/bin/bash # Description: Check for usage of dateOfBirth property in the codebase # Search for dateOfBirth usage in TypeScript and JavaScript files rg --type-add 'web:*.{ts,tsx,js,jsx}' -t web 'dateOfBirth' -C 3If there are any occurrences, make sure to update those parts of the code accordingly.
61-61: LGTM! Consistent simplification of user info assignment.This change is consistent with the modification in the SIGNIN_SUCCESS case, maintaining the simplified approach to user info assignment. It adheres to the coding guidelines for the
libsdirectory.To ensure type safety, please verify and update the TypeScript types if the structure of
userInfohas changed. Run the following script to check the User type definition:If the
Usertype includes adateOfBirthproperty, consider updating it to reflect the new structure.libs/api/domains/documents/src/lib/documentV2.resolver.ts (2)
78-78: Improved flexibility by passing the entire user objectThis change enhances the
listDocumentsmethod's flexibility by passing the entireuserobject instead of just thenationalId. This modification allows for potential future expansions where additional user information might be needed without requiring further changes to the method signature.The change adheres to the coding guidelines for the
libsdirectory by improving the reusability of the component across different NextJS apps.
Line range hint
1-180: Overall assessment: Positive improvements to code flexibility and robustnessThe changes in this file align well with the PR objectives and enhance the overall quality of the code. The modifications to the
documentsV2method improve flexibility, while the added validation inpostMailActionincreases robustness. These changes adhere to the coding guidelines for thelibsdirectory, promoting reusability and maintainability.No major issues were identified during the review. The suggested minor improvement for log consistency is optional and does not impact the overall quality of the changes.
libs/api/domains/documents/src/lib/documentV2.service.ts (1)
90-99: Logic for hiding health data is correctThe implementation correctly determines whether to hide health data based on the user's delegation type and the child's age over 15.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16219 +/- ##
==========================================
- Coverage 36.80% 36.70% -0.10%
==========================================
Files 6854 6847 -7
Lines 142228 141732 -496
Branches 40559 40434 -125
==========================================
- Hits 52342 52019 -323
+ Misses 89886 89713 -173 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 110 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 23 Total Test Services: 0 Failed, 21 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
disaerna
changed the title
* fix: health delegation bug and move logic to service * feat: hide law and order and health data if certain delegation * console * fix: add correct id * remove birthday use (v1 is deprecated) --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* fix(clients): prod path (#16554) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(my-pages): Law and Order - error handling (#16506) * fix: remove button if error * feat: audit and log error if lawyer list fails * feat: detailed logs * fix: remove noice * revert message prop --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(service-portal): Filtering mailbox documents (#16219) * fix: health delegation bug and move logic to service * feat: hide law and order and health data if certain delegation * console * fix: add correct id * remove birthday use (v1 is deprecated) --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(my-pages): display error message on urgent files on dashboard (#16572) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Mailbox - Health documents
What
Why
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Chores