fix(admin): already signed message & create collection available #16537
Conversation
WalkthroughThe pull request introduces several new message definitions and modifies existing ones in 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
|
![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 (6)
libs/portals/admin/signature-collection/src/shared-components/completeReview/index.tsx (2)
Line range hint
13-16: Consider improving type safety for listStatus prop.The
listStatusprop is typed as string but appears to only accept specific values fromListStatusenum.- listStatus: string + listStatus: ListStatus
Line range hint
1-117: Enhance component reusability with proper documentation.As this component resides in the
libs/directory and could be reused across different NextJS apps, consider adding:
- JSDoc documentation for the component and its props
- Exported TypeScript interfaces for the component props
Example addition:
export interface ActionReviewCompleteProps { /** Unique identifier for the list */ listId: string /** Current status of the list */ listStatus: ListStatus } /** * A component that handles the review completion action for lists. * Provides functionality to toggle list review status with appropriate user feedback. */ const ActionReviewComplete = ({ listId, listStatus }: ActionReviewCompleteProps) => { // ... existing implementation }libs/portals/admin/signature-collection/src/screens-parliamentary/List/paperSignees/index.tsx (1)
78-84: LGTM! Consider enhancing error handling further.The error handling implementation is correct and improves user feedback. However, consider these enhancements:
- Extract error reasons to TypeScript enum/constants
- Add type definitions for error responses
- Consider handling multiple error reasons if applicable
Example enhancement:
enum SignatureErrors { ALREADY_SIGNED = 'alreadySigned', // ... other error types } type UploadResponse = { reasons?: SignatureErrors[]; success: boolean; }libs/portals/admin/signature-collection/src/screens-parliamentary/Constituency/index.tsx (1)
134-139: Consider adding status validation in CreateCollection.To maintain data integrity while keeping the parent component simple, consider moving the status validation into the CreateCollection component itself.
Example approach:
interface CreateCollectionProps { collectionId: string; areaId: string; // Add optional status prop for internal validation collectionStatus?: CollectionStatus; }libs/portals/admin/signature-collection/src/lib/messages.ts (2)
504-508: Add a description to help translators.The message definition is good, but the empty description field could be improved to help translators understand the context.
toggleReviewSuccessToggleBack: { id: 'admin-portal.signature-collection:toggleReviewSuccessToggleBack', defaultMessage: 'Tókst að opna fyrir úrvinnslu', - description: '', + description: 'Success message shown when reopening a signature collection for review', },
762-766: Enhance error message clarity and add description.The error message could be more specific, and the description field should be filled to help translators understand the context.
paperSigneeErrorAlreadySigned: { id: 'admin-portal.signature-collection:paperSigneeErrorAlreadySigned', - defaultMessage: 'Kennitala er þegar á lista', - description: '', + defaultMessage: 'Kennitala er þegar skráð á meðmælalista', + description: 'Error message shown when attempting to add a national ID that is already registered on a signature collection list', },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
- libs/portals/admin/signature-collection/src/lib/messages.ts (2 hunks)
- libs/portals/admin/signature-collection/src/screens-parliamentary/Constituency/index.tsx (1 hunks)
- libs/portals/admin/signature-collection/src/screens-parliamentary/List/paperSignees/index.tsx (1 hunks)
- libs/portals/admin/signature-collection/src/shared-components/completeReview/index.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
libs/portals/admin/signature-collection/src/lib/messages.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/portals/admin/signature-collection/src/screens-parliamentary/Constituency/index.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."
libs/portals/admin/signature-collection/src/screens-parliamentary/List/paperSignees/index.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."
libs/portals/admin/signature-collection/src/shared-components/completeReview/index.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 (3)
libs/portals/admin/signature-collection/src/shared-components/completeReview/index.tsx (1)
38-42: LGTM! Toast notification logic improvement.The conditional toast message based on list status provides better user feedback.
libs/portals/admin/signature-collection/src/screens-parliamentary/List/paperSignees/index.tsx (1)
83-83: Verify the message key exists.Ensure that the
paperSigneeErrorAlreadySignedmessage key is properly defined in the messages file.✅ Verification successful
Let me check the actual message definition to ensure it's properly defined.
Message key
paperSigneeErrorAlreadySignedis properly definedThe message key is correctly defined in
libs/portals/admin/signature-collection/src/lib/messages.tswith a proper ID, default message, and description fields. The usage in the code is valid.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the paperSigneeErrorAlreadySigned message definition rg "paperSigneeErrorAlreadySigned" --type tsLength of output: 440
Script:
#!/bin/bash # Get the full context of the message definition rg "paperSigneeErrorAlreadySigned" -A 3 libs/portals/admin/signature-collection/src/lib/messages.tsLength of output: 285
libs/portals/admin/signature-collection/src/screens-parliamentary/Constituency/index.tsx (1)
134-139: Verify the removal of CollectionStatus check.The simplified condition makes the component more reusable, but please verify that removing the CollectionStatus dependency doesn't allow collection creation in invalid states.
✅ Verification successful
After analyzing the CreateCollection component implementation, I can now provide a final response regarding the removal of the CollectionStatus check.
Removal of CollectionStatus check is safe - component has internal validation
The CreateCollection component has robust internal validation mechanisms that make the CollectionStatus check redundant:
- It performs candidate lookup and validation via
candidateLookupquery- Uses
canCreatestate to control whether collection creation is allowed- Button is disabled unless both
canCreateis true and a validnameexists- Displays error messages when creation is not allowed via
canCreateErrorReason🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for CollectionStatus usage in CreateCollection component ast-grep --pattern 'const CreateCollection = $_' -A 20Length of output: 22554
Datadog ReportBranch report: ✅ 0 Failed, 4 Passed, 0 Skipped, 3.29s Total Time |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16537 +/- ##
========================================
Coverage 36.76% 36.77%
========================================
Files 6851 6850 -1
Lines 142120 142000 -120
Branches 40542 40493 -49
========================================
- Hits 52250 52219 -31
+ Misses 89870 89781 -89
Flags with carried forward coverage won't be shown. Click here to find out more. see 20 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Improvements
CreateCollectioncomponent, allowing easier access when processing is permitted.ActionReviewCompletecomponent to reflect the current review status of lists.