fix(service-portal): owner UI updates + paper signees #16067
Conversation
WalkthroughThe pull request introduces enhancements to the signature collection feature, including the addition of pagination and new fields in GraphQL queries, the introduction of a 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: 8
Outside diff range and nitpick comments (3)
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/PaperSignees.tsx (1)
19-124: Enhance TypeScript types for better maintainabilityConsider adding explicit TypeScript types to the
PaperSigneescomponent and its state variables. This will improve code readability and help catch potential type errors during development.Example:
export const PaperSignees: React.FC = () => { ... const [nationalIdInput, setNationalIdInput] = useState<string>('') const [nationalIdTypo, setNationalIdTypo] = useState<boolean>(false) const [name, setName] = useState<string>('') ...libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx (1)
90-90: Handle undefined 'list.area?.name' in headingThe
headingprop useslist.area?.name, which could beundefined. This might lead to unintended UI rendering issues.Provide a default value to ensure the heading is always defined:
- heading={list.area?.name} + heading={list.area?.name || formatMessage(m.unknownArea)}Ensure that
m.unknownAreais defined in your messages.libs/service-portal/signature-collection/src/lib/messages.ts (1)
54-54: Inconsistent punctuation in 'defaultMessage'The
defaultMessageforcopyLinkDescriptiondoes not end with a period, while other messages in the file do. This might lead to inconsistency in the UI text.Consider adding a period at the end for consistency:
defaultMessage: 'Hér getur þú afritað hlekk á þitt framboð til að deila', + // Add period at the end for consistency. + defaultMessage: 'Hér getur þú afritað hlekk á þitt framboð til að deila.',
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (9)
- libs/service-portal/signature-collection/src/hooks/graphql/queries.ts (1 hunks)
- libs/service-portal/signature-collection/src/lib/messages.ts (4 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/PaperSignees.tsx (1 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/index.tsx (2 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx (6 hunks)
- libs/service-portal/signature-collection/src/screens/Presidential/OwnerView/CancelCollection/index.tsx (1 hunks)
- libs/service-portal/signature-collection/src/screens/Presidential/OwnerView/index.tsx (1 hunks)
- libs/service-portal/signature-collection/src/screens/shared/SignedList/index.tsx (2 hunks)
- libs/service-portal/signature-collection/src/screens/shared/SigneeView/index.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
- libs/service-portal/signature-collection/src/screens/Presidential/OwnerView/index.tsx
Additional context used
Path-based instructions (8)
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/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/service-portal/signature-collection/src/hooks/graphql/queries.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/service-portal/signature-collection/src/screens/Presidential/OwnerView/CancelCollection/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/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/PaperSignees.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/service-portal/signature-collection/src/screens/shared/SigneeView/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/service-portal/signature-collection/src/screens/shared/SignedList/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/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/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/service-portal/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."
Additional comments not posted (9)
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/index.tsx (1)
8-8: Integration ofPaperSigneescomponent looks goodThe
PaperSigneescomponent is correctly imported and used in theViewListcomponent. This enhances the functionality as intended.Also applies to: 57-57
libs/service-portal/signature-collection/src/hooks/graphql/queries.ts (1)
73-73: Confirm availability ofsignedDatein GraphQL schemaThe
signedDatefield has been added to theGetSignedListquery. Ensure that this field exists and is correctly exposed by the GraphQL API.You can run the following script to verify the presence of
signedDatein the schema:libs/service-portal/signature-collection/src/screens/Presidential/OwnerView/CancelCollection/index.tsx (1)
11-11: Removal oflistIdprop simplifies the componentThe
CancelCollectioncomponent no longer requires thelistIdprop, streamlining its usage. Ensure that this change does not affect any other components or places whereCancelCollectionis used.You can run the following script to check for other usages of
CancelCollectionwith props:Verification successful
Removal of
listIdprop is consistent with component usageThe verification process confirms that the
CancelCollectioncomponent is indeed used without any props in the codebase. The only usage found is inlibs/service-portal/signature-collection/src/screens/Presidential/OwnerView/index.tsx, where it's rendered as<CancelCollection />. This is consistent with the removal of thelistIdprop and the simplification of the component.No instances of
listIdor any other props being passed toCancelCollectionwere found, indicating that this change does not affect other components or places whereCancelCollectionis used.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find usages of 'CancelCollection' component with props. # Test: Search for 'CancelCollection' component usages. # Expect: No instances where 'CancelCollection' is passed props. rg --type tsx --fixed-strings '<CancelCollection' -A 5Length of output: 84
Script:
#!/bin/bash # Description: Find usages of 'CancelCollection' component in TypeScript and TypeScript React files. # Find all .ts and .tsx files and search for 'CancelCollection' usages fd -e ts -e tsx | xargs rg --fixed-strings '<CancelCollection' -C 2 # Specifically search for 'listId' prop being passed to 'CancelCollection' fd -e ts -e tsx | xargs rg --fixed-strings '<CancelCollection' -C 2 | rg 'listId'Length of output: 748
libs/service-portal/signature-collection/src/screens/shared/SigneeView/index.tsx (1)
62-63: Title and area display updates are appropriateUpdating the
headingandeyebrowproperties to display the sanitized title and area name enhances the clarity of theActionCardcomponent.libs/service-portal/signature-collection/src/screens/shared/SignedList/index.tsx (2)
73-74: Simplification of title display is effectiveUsing only the first part of the
list.titlefor theheadingimproves readability in theActionCard.
103-111: Consistent styling for signature tagsChanging the
variantto'blueberry'and settingoutlinedtotruefor both digital and paper signatures ensures a consistent visual style.libs/service-portal/signature-collection/src/lib/messages.ts (3)
102-106: New message keys added correctlyThe new message
digitalSignaturehas been added with appropriateid,defaultMessage, anddescription. This follows the existing pattern.
192-196: Consistent naming in 'cancelCollectionModalCancelButton'The
idanddefaultMessageforcancelCollectionModalCancelButtonare consistent with the naming conventions used throughout the messages file.
269-298: New messages for paper signees added accuratelyThe messages related to paper signees (
paperSigneesHeader,paperNumber,paperSigneeName,signPaperSigneeButton,paperSigneeTypoTitle,paperSigneeTypoMessage) have been added correctly with appropriateids anddefaultMessages.
...ce-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/PaperSignees.tsx
Outdated
Show resolved
Hide resolved
...ce-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/PaperSignees.tsx
Outdated
Show resolved
Hide resolved
...ce-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/PaperSignees.tsx
Outdated
Show resolved
Hide resolved
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx
Outdated
Show resolved
Hide resolved
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx
Show resolved
Hide resolved
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx
Show resolved
Hide resolved
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx
Show resolved
Hide resolved
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16067 +/- ##
==========================================
- Coverage 36.66% 36.63% -0.03%
==========================================
Files 6748 6748
Lines 138726 138828 +102
Branches 39404 39468 +64
==========================================
+ Hits 50862 50865 +3
- Misses 87864 87963 +99
Flags with carried forward coverage won't be shown. Click here to find out more. see 22 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 5 Total Test Services: 0 Failed, 5 Passed Test Services
|
albinagu
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
libs/service-portal/signature-collection/src/lib/messages.ts (1)
54-54: Ensure punctuation consistency across messages.The change is acceptable. However, consider maintaining consistent punctuation across all messages to provide a polished user experience.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- libs/service-portal/signature-collection/src/hooks/graphql/mutations.ts (1 hunks)
- libs/service-portal/signature-collection/src/lib/constants.ts (0 hunks)
- libs/service-portal/signature-collection/src/lib/messages.ts (5 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/AddConstituency/index.tsx (2 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx (6 hunks)
Files not reviewed due to no reviewable changes (1)
- libs/service-portal/signature-collection/src/lib/constants.ts
Additional context used
Path-based instructions (4)
libs/service-portal/signature-collection/src/hooks/graphql/mutations.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/service-portal/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/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/AddConstituency/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/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/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 not posted (21)
libs/service-portal/signature-collection/src/hooks/graphql/mutations.ts (1)
21-30: LGTM! The new mutation follows best practices and enhances functionality.The
addConstituencymutation is a great addition to the GraphQL API. It adheres to the following best practices:
- Uses the
gqltemplate literal tag for defining the mutation.- Follows the naming convention for GraphQL mutations.
- Accepts a typed input parameter.
- Returns an object with appropriate fields for success status and additional context.
The mutation expands the capabilities of the service by allowing the addition of constituencies to the signature collection.
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/AddConstituency/index.tsx (7)
1-12: LGTM!The added imports are relevant and necessary for the component's functionality. The import statements follow the correct syntax and naming conventions.
16-23: LGTM!The added props
collection,candidateId, andrefetchare relevant for managing the state and behavior of the constituency addition process. The prop types are correctly defined, and the prop names are descriptive and follow the naming conventions.
26-31: LGTM!The updated logic for filtering constituencies ensures that only relevant constituencies are displayed based on the current selection. The filtering is performed correctly by comparing the constituency names, and the filtered constituencies are correctly typed as
SignatureCollectionArea[].
38-59: LGTM!The
addNewConstituencymutation is correctly set up using theuseMutationhook and theaddConstituencymutation. TheonCompletedandonErrorhandlers provide appropriate user feedback via toast notifications. TheonAddConstituencyfunction correctly executes the mutation with the required variables.
90-107: LGTM!The code correctly maps over the filtered constituencies and renders a checkbox for each constituency. The checkbox label displays the constituency name, and the checkbox value is set to the constituency ID. The checkbox state is correctly managed by adding or removing the constituency ID from the
selectedConstituenciesarray.
115-117: LGTM!The button for adding constituencies is correctly linked to the
onAddConstituencyfunction, which triggers the mutation to add the selected constituencies. The button's loading state is correctly managed using theloadingvariable from the mutation, improving the user experience.
Line range hint
1-125: Review the component for adherence to additional instructions.The component adheres to the following guidelines:
- It is written in TypeScript and defines prop types, adhering to the TypeScript usage guideline.
However, there are a few points to consider:
- The component is specific to the service portal and may not be directly reusable across different NextJS apps without modifications. Consider extracting reusable parts into separate components or hooks to improve reusability.
- The adherence to effective tree-shaking and bundling practices cannot be determined from this file alone. Ensure that the component and its dependencies are properly tree-shakable and bundled efficiently.
To verify the reusability and bundling practices, run the following script:
Verification successful
Component is specific to the service portal but follows good practices
The verification process confirms that the AddConstituencyModal component is specific to the service portal and is not directly reused in other NextJS apps. However, it adheres to several good practices:
- Written in TypeScript with well-defined prop types
- Uses functional component style with React hooks
- Modular and self-contained design
- Follows React best practices (e.g., using hooks and context)
To improve reusability:
- Consider extracting generic UI elements or logic into separate, reusable components
- Minimize dependencies on service-portal specific imports where possible
- Document the component's usage and requirements for potential future reuse
The component's current implementation is suitable for its specific use case within the service portal. While not directly reusable across different NextJS apps, it maintains good coding practices that facilitate maintenance and potential future refactoring.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the reusability and bundling practices of the component. # Test 1: Search for usages of the component in other NextJS apps. # Expect: No direct usages found. rg --type tsx $'AddConstituencyModal' $(fd --type d --glob 'apps/*/') # Test 2: Analyze the bundle size and tree-shaking of the component. # Expect: The component and its dependencies are properly tree-shaken and bundled efficiently. # Note: This test assumes the existence of a bundle analysis tool like webpack-bundle-analyzer. npm run build:analyzeLength of output: 192
Script:
#!/bin/bash # Description: Verify the reusability and usage of the AddConstituencyModal component # Search for usages of AddConstituencyModal in all TypeScript and JavaScript files echo "Searching for AddConstituencyModal usages:" rg --type ts --type js "AddConstituencyModal" apps/ # Analyze the component's structure echo "Analyzing AddConstituencyModal structure:" ast-grep --lang typescript --pattern 'const AddConstituencyModal = ({ $$ }) => { $$ }' libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/AddConstituency/index.tsx # Check for default export of the component echo "Checking for default export:" ast-grep --lang typescript --pattern 'export default AddConstituencyModal' libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/AddConstituency/index.tsx # Analyze imports to assess dependencies echo "Analyzing imports:" ast-grep --lang typescript --pattern 'import { $$ } from "$_"' libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/AddConstituency/index.tsxLength of output: 893
Script:
#!/bin/bash # Description: Verify the existence and content of the AddConstituencyModal component file # Check if the file exists echo "Checking if the file exists:" ls -l libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/AddConstituency/index.tsx # If the file exists, display its content echo "File content (if exists):" cat libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/AddConstituency/index.tsx # Search for the component definition echo "Searching for AddConstituencyModal definition:" rg --type ts "const AddConstituencyModal" libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/AddConstituency/index.tsx # Search for export statement echo "Searching for export statement:" rg --type ts "export default AddConstituencyModal" libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/AddConstituency/index.tsx # Analyze imports echo "Analyzing imports:" rg --type ts "^import" libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/AddConstituency/index.tsxLength of output: 5265
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/index.tsx (10)
8-12: LGTM!The new imports from
@island.is/island-ui/coreare valid and relevant to the changes made in the file.
19-22: LGTM!The new imports from
@island.is/api/schemaare valid and relevant to the changes made in the file.
26-28: LGTM!The new imports are valid and relevant to the changes made in the file:
useMutationandcancelCollectionMutationare likely used for the new mutation to cancel a collection.copyToClipboardis likely used for the new feature to copy a link.
37-38: LGTM!The destructuring of
listsForOwner,loadingOwnerLists, andrefetchListsForOwnerfrom theuseGetListsForOwnerhook is valid and relevant to the changes made in the file. These variables are likely used in the component to display and manage the lists for the owner.
40-50: LGTM!The new
cancelCollectionmutation is defined correctly using theuseMutationhook and thecancelCollectionMutation. TheonCompletedandonErrorcallbacks handle the success and error scenarios appropriately by displaying toasts and refetching data.
53-62: LGTM!The new
onCancelCollectionfunction is defined correctly to handle the cancellation of a collection. It calls thecancelCollectionmutation with the appropriate input variables (collectionIdandlistIds).
76-84: LGTM!The conditional rendering of the
AddConstituencycomponent is implemented correctly based on theloadingOwnerListsstate and the comparison oflistsForOwnerlength withcurrentCollection?.areas.length. The necessary props are passed to the component.
Line range hint
95-156: LGTM!The modifications made to the
ActionCardcomponent within thelistsForOwner.maploop are implemented correctly:
- The
headingandeyebrowprops are set appropriately based on thelistdata.- The new
tagprop is added to render theDialogPromptcomponent for canceling the collection.- The
DialogPromptcomponent is configured correctly with the necessary props and callbacks, including theonConfirmcallback that calls theonCancelCollectionfunction with thelist.id.
189-218: LGTM!The new section for copying a link is implemented correctly:
- The
Box,Text, andButtoncomponents are used appropriately to create the desired layout and functionality.- The styling of the
Boxcomponent is suitable for the intended appearance.- The
onClickhandler of theButtoncomponent is implemented correctly to copy the link usingcopyToClipboardand display success or error toasts based on the result.- The link being copied is constructed properly using
document.location.originandlistsForOwner[0].slug.
203-210: Check for empty 'listsForOwner' before accessing elementsAccessing
listsForOwner[0]without ensuring the array is not empty can cause runtime errors iflistsForOwneris empty.Add a condition to verify
listsForOwnerhas at least one element:const copied = copyToClipboard( - `${document.location.origin}${listsForOwner[0].slug}`, + `${ + document.location.origin + }${listsForOwner?.[0]?.slug || ''}`, ) - if (!copied) { + if (!copied || !listsForOwner?.[0]?.slug) {Also, handle the case where
slugmight beundefined.Likely invalid or redundant comment.
libs/service-portal/signature-collection/src/lib/messages.ts (3)
102-106: LGTM!The new message
digitalSignaturefollows the existing format and enhances the user interface. Good addition!
189-194: Improved cancel collection modal buttons.The modifications to the cancel collection modal button messages enhance clarity and usability. The changes are well-structured and follow the existing format.
269-298: Comprehensive paper signee messages.The addition of messages related to paper signees is thorough and well-structured. The messages cover all necessary aspects of the user interface, ensuring a comprehensive user experience. Great job in providing localized text for headers, labels, buttons, and error messages!
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (5)
libs/service-portal/signature-collection/src/hooks/graphql/mutations.ts (2)
21-30: LGTM!The
addConstituencymutation is well-defined and aligns with the PR objectives. The naming is clear, and the input and return types provide relevant information.Suggestions to further improve reusability and type safety:
- Consider exporting the
SignatureCollectionAddListsInputandSignatureCollectionAddAreasResponsetypes from a shared types file to promote reusability across different parts of the application.- If the
reasonsfield in the response can have specific values, consider defining an enum for those values to enhance type safety.
32-41: Looks good!The
uploadPaperSignaturemutation is well-structured and fits the purpose of the PR. The naming is intuitive, and the input and return types are appropriate.Suggestions to enhance reusability and type safety:
- Consider moving the
SignatureCollectionUploadPaperSignatureInputandSignatureCollectionUploadPaperSignatureResponsetypes to a shared types file to facilitate reuse across the codebase.- If the
reasonsfield in the response has a predefined set of possible values, consider creating an enum to represent those values, improving type safety.libs/service-portal/signature-collection/src/hooks/graphql/queries.ts (2)
48-48: LGTM!The addition of the
pageNumberfield to theGetListSignaturesquery enables pagination, which can improve the handling of large datasets.To further enhance the pagination functionality, consider the following:
- Ensure that the corresponding resolver function correctly handles the
pageNumberargument and returns the appropriate subset of results.- Consider adding additional arguments such as
pageSizeto allow clients to control the number of results per page.- Update the documentation to reflect the new pagination functionality and provide guidance on how to use it effectively.
161-165: Great addition!The new
GetCanSignquery expands the capabilities of the service by allowing clients to determine the signability of a collection based on the provided input. This can enable new features and enhance the functionality of the application.To further improve the query, consider adding error handling to provide meaningful error messages to the clients in case of invalid input or other exceptional scenarios. This can be done by defining custom error types in the GraphQL schema and throwing them from the resolver function when necessary.
libs/service-portal/signature-collection/src/lib/messages.ts (1)
54-54: Nitpick: Consider keeping the period for consistency.The period at the end of the
copyLinkDescriptionmessage has been removed. For consistency with other messages, consider keeping the period.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- libs/service-portal/signature-collection/src/hooks/graphql/mutations.ts (1 hunks)
- libs/service-portal/signature-collection/src/hooks/graphql/queries.ts (3 hunks)
- libs/service-portal/signature-collection/src/hooks/index.ts (2 hunks)
- libs/service-portal/signature-collection/src/lib/messages.ts (5 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/Signees/PaperSignees.tsx (1 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/Signees/index.tsx (5 hunks)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/index.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/index.tsx
Additional context used
Path-based instructions (6)
libs/service-portal/signature-collection/src/hooks/graphql/mutations.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/service-portal/signature-collection/src/hooks/graphql/queries.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/service-portal/signature-collection/src/hooks/index.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/service-portal/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/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/Signees/PaperSignees.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/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/Signees/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 not posted (18)
libs/service-portal/signature-collection/src/hooks/graphql/queries.ts (1)
74-74: Looks good!The addition of the
signedDatefield to theGetSignedListquery provides valuable information about when a signature was made. This can enhance the functionality related to signature management and enable clients to implement new features or improve existing ones.libs/service-portal/signature-collection/src/hooks/index.ts (1)
152-166: LGTM!The
useGetCanSignhook is a well-implemented addition to the signature collection functionality. It effectively utilizes theuseQueryhook to fetch data from theGetCanSignGraphQL query and conditionally executes the query based on the validity of the providedsigneeId.The hook follows good practices by:
- Accepting typed parameters (
signeeId: string,isValidId: boolean) for clarity and type safety.- Returning a typed object (
{ canSign, loadingCanSign }) for easy consumption by components.- Skipping the query execution when the
signeeIdis invalid or not provided, optimizing performance.The hook is reusable across different components within the signature collection feature, promoting code reuse and maintainability.
Great job on the implementation!
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/Signees/index.tsx (6)
7-7: LGTM!The
Iconcomponent import is a valid addition.
10-10: Verify the updated import path for themobject.Please confirm that the new import path
'../../../../../lib/messages'is correct and themobject is being imported from the intended location after the directory structure change.
13-14: Refactoring improves functionality and code organization.The code changes enhance the component's functionality and organization:
- Using
useParamssimplifies the logic for obtaining the list identifier from the URL.- The
useGetListSigneeshook fetches the list signees based on theid.- Importing the
SkeletonTablecomponent suggests it will be used for loading states, improving the user experience.- Including the
PaperSigneescomponent within theSigneescomponent enhances the functionality related to paper signees.- Sorting the
listSigneesby thecreateddate in descending order ensures the most recent signees appear first.These changes contribute to improved code maintainability and usability.
Also applies to: 16-18, 24-28, 36-40
63-70: UI enhancements improve user experience and information presentation.The code changes enhance the user interface and information presentation:
- The
FilterInputcomponent allows users to easily search for signees by name or national ID, improving the user experience.- The
Tablecomponent effectively displays the list of signees with relevant information.- The new column in the table, showing a document icon and page number for non-digital signees, provides additional context to the user.
- The
Paginationcomponent enables users to navigate through the list of signees conveniently.- The adjusted margin of the
Paginationcomponent improves the visual spacing and overall appearance.These enhancements contribute to a more user-friendly and informative interface.
Also applies to: 74-81, 90-110, 117-117
148-148: Including thePaperSigneescomponent enhances functionality.The inclusion of the
PaperSigneescomponent within theSigneescomponent brings several benefits:
- It allows for the display and management of paper signees, providing a comprehensive view of all signees.
- Passing the
listIdprop enables thePaperSigneescomponent to fetch the relevant paper signees for the current list.- The
refetchSigneesprop allows thePaperSigneescomponent to trigger a refetch of the signees when necessary, ensuring up-to-date information is displayed.This integration improves the overall functionality and user experience related to paper signees.
Line range hint
1-153: Adherence to additional instructions for files in thelibsdirectory.The component adheres to some of the guidelines mentioned in the additional instructions:
- The use of TypeScript for defining props and exporting types aligns with the recommended practices, promoting type safety and better documentation.
However, there are a few points to consider:
While the location of the file in the
libsdirectory suggests it is intended for reusability across different NextJS apps, the component's specificity to the signature collection feature within the service portal may limit its reusability in other contexts. Consider evaluating if the component can be further generalized or if it is appropriate to keep it specific to the signature collection feature.The effective tree-shaking and bundling practices cannot be fully assessed without more information about the build process and configuration. It would be beneficial to review the build setup to ensure optimal bundling and tree-shaking are in place.
Overall, the component partially aligns with the additional instructions, but there are opportunities for further evaluation and potential improvements in terms of reusability and bundling optimization.
libs/service-portal/signature-collection/src/screens/Parliamentary/OwnerView/ViewList/Signees/PaperSignees.tsx (4)
1-21: LGTM!The imports are well-organized, follow a consistent naming convention, and cover the necessary dependencies for the component's functionality. There are no unused imports.
23-38: LGTM!The component props are typed correctly and serve the necessary purpose. The local state variables are initialized with appropriate default values and have clear and descriptive names.
39-60: LGTM!The component efficiently fetches the necessary data based on the
nationalIdInputand handles the case when the input is invalid or has an incorrect length. TheuseEffecthook correctly updates the state variables based on the changes in dependencies.
62-188: LGTM!The upload paper signature mutation is set up correctly with the necessary input variables, and the success and error handling is appropriate. The
onClearFormfunction correctly resets the form and clears the state variables. The form rendering is structured well using theGridContainerandGridRowcomponents, and the error messages are conditionally rendered based on the state variables.libs/service-portal/signature-collection/src/lib/messages.ts (6)
102-106: LGTM!The
digitalSignaturemessage has been added correctly and enhances the digital signature functionality.
189-191: LGTM!The
cancelCollectionModalConfirmButtonmessage has been modified to streamline the wording, which improves clarity.
192-194: LGTM!The
cancelCollectionModalCancelButtonmessage has been added correctly and enhances the cancel collection modal functionality.
269-323: LGTM!The new messages related to paper signees have been added correctly and significantly enhance the paper-based signature collection functionality. The messages cover various aspects such as headers, buttons, and error handling, providing a comprehensive user experience.
393-403: LGTM!The
addConstituencySuccessandaddConstituencyErrormessages have been added correctly and enhance the user feedback for the add constituency functionality. They provide clear and localized messages for success and error scenarios.
Line range hint
1-403: Verify the usage of new and modified messages.Several new messages have been added, and a few existing messages have been modified. To ensure a smooth user experience, please verify that all the new and modified messages are being used correctly in the codebase.
Run the following script to search for the usage of the messages:
Expect: The new and modified messages should be used in the relevant components and functions.
Verification successful
New and modified messages are correctly implemented and used.
The verification process has confirmed that all new and modified messages in the
libs/service-portal/signature-collection/src/lib/messages.tsfile are being used appropriately in the codebase. The new messages related to paper signees, digital signatures, and constituency management are implemented in the relevant components and screens. The modified messages for copy link description and cancel collection modal buttons are also correctly updated and used in their respective contexts.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of new and modified messages in the codebase. # Test: Search for the usage of the new messages. rg --type typescript -A 5 -e 'digitalSignature|paperSigneesHeader|paperNumber|paperSigneeName|signPaperSigneeButton|paperSigneeTypoTitle|paperSigneeTypoMessage|paperSigneeCantSignTitle|paperSigneeCantSignMessage|paperSigneeSuccess|paperSigneeError|addConstituencySuccess|addConstituencyError' # Test: Search for the usage of the modified messages. rg --type typescript -A 5 -e 'copyLinkDescription|cancelCollectionModalConfirmButton|cancelCollectionModalCancelButton'Length of output: 486
Script:
#!/bin/bash # Description: Verify the usage of new and modified messages in TypeScript files. # Search for the usage of new messages (part 1) echo "Searching for new messages (part 1):" fd '\.(ts|tsx)$' | xargs rg -n -A 5 'digitalSignature|paperSigneesHeader|paperNumber|paperSigneeName|signPaperSigneeButton|paperSigneeTypoTitle' # Search for the usage of new messages (part 2) echo "Searching for new messages (part 2):" fd '\.(ts|tsx)$' | xargs rg -n -A 5 'paperSigneeTypoMessage|paperSigneeCantSignTitle|paperSigneeCantSignMessage|paperSigneeSuccess|paperSigneeError' # Search for the usage of new messages (part 3) echo "Searching for new messages (part 3):" fd '\.(ts|tsx)$' | xargs rg -n -A 5 'addConstituencySuccess|addConstituencyError' # Search for the usage of modified messages echo "Searching for modified messages:" fd '\.(ts|tsx)$' | xargs rg -n -A 5 'copyLinkDescription|cancelCollectionModalConfirmButton|cancelCollectionModalCancelButton'Length of output: 31028
albinagu
added
the
deprecated:automerge
Checklist:
Summary by CodeRabbit
New Features
pageNumberfield to theGetListSignaturesquery for improved pagination.GetCanSignquery to determine if a signature can be made.signedDatefield to theGetSignedListquery for enhanced data retrieval.PaperSigneescomponent for national ID validation and name display.AddConstituencyModalfor adding constituencies with better user feedback.Bug Fixes
Refactor
CancelCollectioncomponent.SignedListandSigneeViewcomponents for better clarity.