feat(new-primary-school): Contacts page #17466
Conversation
WalkthroughThe pull request introduces a comprehensive rename and restructure of the "Relatives" section to "Contacts" in the New Primary School application template. This change involves modifications across multiple files, including utility functions, type definitions, form configurations, and message translations. The transformation focuses on replacing "relatives" terminology with "contacts," adjusting the maximum number of entries from 6 to 4, and updating related components and schemas to reflect this new approach. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (50)
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 comments (1)
libs/application/templates/new-primary-school/src/fields/ContactsTableRepeater/index.tsx (1)
Line range hint
52-84: Update data test IDs to match new terminology.The data test IDs still use the "relative" prefix (e.g., 'relative-full-name', 'relative-phone-number', etc.) while the component has been renamed to use "contacts". This inconsistency should be fixed.
Apply this diff to update the data test IDs:
- dataTestId: 'relative-full-name', + dataTestId: 'contact-full-name', - dataTestId: 'relative-phone-number', + dataTestId: 'contact-phone-number', - dataTestId: 'relative-national-id', + dataTestId: 'contact-national-id', - dataTestId: 'relative-relation', + dataTestId: 'contact-relation',
🧹 Nitpick comments (2)
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/childrenNParentsSection/contactsSubSection.ts (1)
18-22: Consider using type-safe component referenceInstead of using a string literal for the component name, consider using a type-safe reference to ensure the component exists at compile time.
Example approach:
import { ContactsTableRepeater } from '../../../fields' // Then in buildCustomField component: ContactsTableRepeater,libs/application/templates/new-primary-school/src/types.ts (1)
Line range hint
7-12: Consider using a union type for the relation fieldThe
relationfield could benefit from a more specific type definition to constrain possible values and provide better type safety.Example improvement:
export type ContactRelationType = 'parent' | 'guardian' | 'emergency' | 'other'; export interface ContactsRow { fullName: string; phoneNumber: string; nationalId: string; relation: ContactRelationType; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
libs/application/template-api-modules/src/lib/modules/templates/new-primary-school/new-primary-school.utils.ts(2 hunks)libs/application/templates/new-primary-school/src/fields/ContactsTableRepeater/index.tsx(3 hunks)libs/application/templates/new-primary-school/src/fields/Review/index.tsx(2 hunks)libs/application/templates/new-primary-school/src/fields/Review/review-groups/Contacts.tsx(2 hunks)libs/application/templates/new-primary-school/src/fields/index.ts(1 hunks)libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/childrenNParentsSection/contactsSubSection.ts(1 hunks)libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/childrenNParentsSection/index.ts(1 hunks)libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/childrenNParentsSection/relativesSubSection.ts(0 hunks)libs/application/templates/new-primary-school/src/lib/dataSchema.ts(2 hunks)libs/application/templates/new-primary-school/src/lib/messages.ts(2 hunks)libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts(3 hunks)libs/application/templates/new-primary-school/src/types.ts(1 hunks)
💤 Files with no reviewable changes (1)
- libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/childrenNParentsSection/relativesSubSection.ts
🧰 Additional context used
📓 Path-based instructions (11)
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/childrenNParentsSection/contactsSubSection.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/application/templates/new-primary-school/src/types.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/application/templates/new-primary-school/src/fields/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/application/templates/new-primary-school/src/lib/dataSchema.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/application/template-api-modules/src/lib/modules/templates/new-primary-school/new-primary-school.utils.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/application/templates/new-primary-school/src/fields/Review/review-groups/Contacts.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/application/templates/new-primary-school/src/fields/Review/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/application/templates/new-primary-school/src/fields/ContactsTableRepeater/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/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.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/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/childrenNParentsSection/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/application/templates/new-primary-school/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 (14)
libs/application/templates/new-primary-school/src/fields/index.ts (1)
2-2: LGTM! Export change aligns with the renaming initiativeThe barrel export pattern is maintained, supporting effective tree-shaking.
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/childrenNParentsSection/index.ts (1)
5-5: LGTM! Section configuration updated consistentlyThe changes maintain the modular structure while implementing the rename from relatives to contacts.
Also applies to: 10-10
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/childrenNParentsSection/contactsSubSection.ts (1)
1-7: LGTM! Clean implementation using core buildersThe implementation follows the established patterns and properly separates concerns.
libs/application/templates/new-primary-school/src/fields/Review/review-groups/Contacts.tsx (2)
21-27: LGTM! Component and data structure renamed consistently.The renaming from
RelativestoContactsis implemented consistently across the component name and data destructuring.
43-60: LGTM! UI navigation and text changes are consistent.The screen navigation, section title, and table ID have been updated to use "contacts" terminology consistently.
libs/application/templates/new-primary-school/src/fields/ContactsTableRepeater/index.tsx (1)
46-46: Verify the impact of reducing maximum contacts.The maximum number of rows has been reduced from 6 to 4. Please ensure this change aligns with business requirements and won't impact existing applications.
Run this script to check for any applications that might be affected:
✅ Verification successful
UI-only change with no impact on existing applications
The reduction from 6 to 4 maximum contacts is a UI-only change that affects only new applications. No validation rules or business logic in the codebase depend on the previous limit of 6 contacts. Existing applications with more than 4 contacts will continue to work as expected.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any code that might depend on the previous maximum of 6 contacts rg -A 2 "maxRows.*6" # Search for any tests that might need updating rg -A 2 "contacts.*length.*6"Length of output: 54
Script:
#!/bin/bash # Search for contact-related files echo "=== Contact-related files ===" fd -t f -i contact # Search for validation schemas or rules related to contacts echo -e "\n=== Validation rules ===" rg -i "contacts.*validate|contacts.*schema" -A 3 # Search for test files with contact assertions echo -e "\n=== Test assertions ===" rg -t test -t spec "contacts.*expect|contacts.*assert|contacts.*should" -A 2 # Search for any number greater than 4 in contact-related files echo -e "\n=== Contact limits ===" fd -t f -i contact --exec rg -l "[5-9]|[1-9][0-9]"Length of output: 7256
Script:
#!/bin/bash # Check content of primary school contact-related files echo "=== ContactsTableRepeater implementation ===" cat "libs/application/templates/new-primary-school/src/fields/ContactsTableRepeater/index.tsx" echo -e "\n=== Contacts subsection validation ===" cat "libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/childrenNParentsSection/contactsSubSection.ts" echo -e "\n=== Review component ===" cat "libs/application/templates/new-primary-school/src/fields/Review/review-groups/Contacts.tsx"Length of output: 7222
libs/application/templates/new-primary-school/src/lib/dataSchema.ts (1)
Line range hint
63-75: LGTM! Schema changes maintain data validation integrity.The renaming from
relativestocontactsin the schema is consistent, and the validation rules remain intact.libs/application/template-api-modules/src/lib/modules/templates/new-primary-school/new-primary-school.utils.ts (1)
21-21: LGTM! DTO transformation maintains data integrity.The renaming from
relativestocontactsin the DTO transformation is consistent, and the mapping toagentsarray preserves the same structure.Also applies to: 66-70
libs/application/templates/new-primary-school/src/fields/Review/index.tsx (2)
23-23: LGTM! Import statement updated correctly.The import statement has been updated to reflect the new component name, maintaining consistency with the broader refactor from relatives to contacts.
158-158: LGTM! Component usage updated correctly.The
Contactscomponent is correctly integrated into the review screen, maintaining the same props structure as the previousRelativescomponent.libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts (2)
16-16: LGTM! Type import updated correctly.The import statement has been updated to use the new
ContactsRowtype, maintaining consistency with the broader refactor.
38-38: LGTM! Variable rename implemented correctly.The variable has been renamed from
relativestocontactsconsistently in both the declaration and return statement, maintaining the same functionality.Also applies to: 120-120
libs/application/templates/new-primary-school/src/lib/messages.ts (2)
249-285: LGTM! Message keys for contacts section added correctly.The new message keys maintain consistent naming patterns and provide clear, well-structured translations for the contacts section. The description text clearly communicates the requirement of at least one contact and the maximum limit of four contacts.
667-670: LGTM! Error message updated correctly.The error message for contacts validation maintains consistency with other validation messages in the file while clearly communicating the minimum requirement.
Datadog ReportBranch report: ✅ 0 Failed, 4 Passed, 0 Skipped, 2.94s Total Time |
helgifr
added
the
deprecated:automerge
kodiakhq
bot
removed
the
deprecated:automerge
|
This PR currently has a merge conflict. Please resolve this and then re-add the |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
libs/application/templates/new-primary-school/src/lib/messages.ts (1)
Line range hint
1-767: Well-structured message module following best practices.The file follows TypeScript best practices with proper type definitions and exports. The modular structure enables effective tree-shaking and reusability across different NextJS applications.
libs/application/templates/new-primary-school/src/fields/Review/review-groups/Contacts.tsx (2)
Line range hint
42-48: Add null checks and type guards for data transformation.The row mapping could be more robust with:
- Null/undefined checks for the contacts array
- Type guards for the contact object properties
- Error handling for invalid phone numbers or national IDs
Consider this improvement:
- const rows = contacts.map((r) => { + const rows = (contacts ?? []).map((r) => { + const phoneNumber = r.phoneNumber ? removeCountryCode(r.phoneNumber) : '' return [ - r.fullName, - formatPhoneNumber(removeCountryCode(r.phoneNumber ?? '')), - formatKennitala(r.nationalId), + r.fullName ?? '', + phoneNumber ? formatPhoneNumber(phoneNumber) : '', + r.nationalId ? formatKennitala(r.nationalId) : '', getSelectedOptionLabel(relationFriggOptions, r.relation) ?? '', ] })
Line range hint
67-94: Enhance accessibility and user experience.Consider these improvements:
- Add ARIA labels to the table for better screen reader support
- Include a message when there are no contacts (currently shows nothing)
- Add tooltips for formatted values (phone numbers, national IDs)
Add an empty state message:
{contacts?.length > 0 && ( <Box paddingTop={3}> <StaticTableFormField application={application} field={{ type: FieldTypes.STATIC_TABLE, component: FieldComponents.STATIC_TABLE, children: undefined, id: 'contactsTable', + aria-label: formatMessage(newPrimarySchoolMessages.childrenNParents.contactsSubSectionTitle), title: '', header: [/*...*/], rows, }} /> </Box> + )} + {(!contacts || contacts.length === 0) && ( + <Box paddingTop={3}> + <Text variant="small"> + {formatMessage(newPrimarySchoolMessages.shared.noContactsAdded)} + </Text> </Box> )}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
libs/application/template-api-modules/src/lib/modules/templates/new-primary-school/new-primary-school.utils.ts(2 hunks)libs/application/templates/new-primary-school/src/fields/ContactsTableRepeater/index.tsx(3 hunks)libs/application/templates/new-primary-school/src/fields/Review/index.tsx(2 hunks)libs/application/templates/new-primary-school/src/fields/Review/review-groups/Contacts.tsx(3 hunks)libs/application/templates/new-primary-school/src/lib/dataSchema.ts(2 hunks)libs/application/templates/new-primary-school/src/lib/messages.ts(2 hunks)libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts(3 hunks)libs/application/templates/new-primary-school/src/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- libs/application/templates/new-primary-school/src/lib/dataSchema.ts
- libs/application/templates/new-primary-school/src/types.ts
- libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts
- libs/application/templates/new-primary-school/src/fields/Review/index.tsx
- libs/application/template-api-modules/src/lib/modules/templates/new-primary-school/new-primary-school.utils.ts
- libs/application/templates/new-primary-school/src/fields/ContactsTableRepeater/index.tsx
🧰 Additional context used
📓 Path-based instructions (2)
libs/application/templates/new-primary-school/src/fields/Review/review-groups/Contacts.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/application/templates/new-primary-school/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 (3)
libs/application/templates/new-primary-school/src/lib/messages.ts (2)
262-298: LGTM! Well-structured message definitions.The contacts section messages are well-organized, properly typed, and follow consistent naming patterns. The translations and descriptions are clear and informative.
716-719: LGTM! Clear error message definition.The error message for contacts validation is well-defined and follows the established pattern for error messages in the file.
libs/application/templates/new-primary-school/src/fields/Review/review-groups/Contacts.tsx (1)
28-34: Verify component reusability across NextJS apps.Since this component is in the
libsdirectory, ensure that:
- The component and its types are properly exported for use in other NextJS apps
- The dependencies are tree-shakeable
- The component is not tightly coupled to the new-primary-school template
Run this script to check for proper exports and potential coupling:
helgifr
added
the
deprecated:automerge
helgifr
added
deprecated:automerge
...
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Improvements
User Experience