fix(portals-admin): allow delete list & double tag #16686
Conversation
WalkthroughThe changes in this pull request involve the addition of a new internationalized message 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
|
Datadog ReportAll test runs ✅ 2 Total Test Services: 0 Failed, 1 Passed Test Services
|
![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 (1)
libs/portals/admin/signature-collection/src/lib/messages.ts (1)
514-518: LGTM! Consider adding a description.The new message definition follows the established pattern and is properly placed with other list-related messages. However, adding a description would improve maintainability by providing context for translators and developers.
listOpen: { id: 'admin-portal.signature-collection:listOpened', defaultMessage: 'Söfnun í gangi', - description: '', + description: 'Status message indicating that a signature collection is currently active and accepting signatures', },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
libs/portals/admin/signature-collection/src/lib/messages.ts(1 hunks)libs/portals/admin/signature-collection/src/screens-parliamentary/Constituency/index.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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."
🔇 Additional comments (2)
libs/portals/admin/signature-collection/src/screens-parliamentary/Constituency/index.tsx (2)
Line range hint 33-44: LGTM! Clean type definitions and proper hook usage.
The component follows TypeScript best practices and maintains reusability across NextJS apps.
209-230: Consider handling delete operations atomically.
The sequential delete operations for list and candidate could lead to an inconsistent state if the second operation fails. Consider:
- Handling both operations in a single mutation on the backend
- Adding error handling for the candidate deletion
- Rolling back the list deletion if candidate deletion fails
libs/portals/admin/signature-collection/src/screens-parliamentary/Constituency/index.tsx
Outdated
Show resolved
Hide resolved
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/admin/signature-collection/src/screens-parliamentary/Constituency/index.tsx(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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."
🔇 Additional comments (2)
libs/portals/admin/signature-collection/src/screens-parliamentary/Constituency/index.tsx (2)
18-18: LGTM: Import changes improve type safety and bundle size.
The explicit import of TagVariant and SignatureCollectionList types, along with the removal of unused imports, enhances type safety and supports effective tree-shaking.
Also applies to: 34-34
75-100: LGTM: Well-structured helper function implementation.
The getTagConfig function effectively simplifies the tag rendering logic as suggested in the previous review. The implementation is:
- Type-safe with proper TagVariant typing
- Well-documented with clear state descriptions
- Pure and reusable
libs/portals/admin/signature-collection/src/screens-parliamentary/Constituency/index.tsx
Show resolved
Hide resolved
albinagu
added
automerge
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16686 +/- ##
==========================================
+ Coverage 36.63% 36.68% +0.04%
==========================================
Files 6873 6858 -15
Lines 143132 142756 -376
Branches 40805 40701 -104
==========================================
- Hits 52436 52363 -73
+ Misses 90696 90393 -303
Flags with carried forward coverage won't be shown. Click here to find out more. see 36 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Checklist:
Summary by CodeRabbit
New Features
Improvements
Constituencycomponent by simplifying data handling and rendering logic for collection management.DialogPromptfor list removal, with updated confirmation button text.