-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(new-primary-school): Implement allergies and intolerances page #17477
feat(new-primary-school): Implement allergies and intolerances page #17477
Conversation
WalkthroughThis pull request introduces enhancements to the new primary school application template, focusing on managing allergies and intolerances. The changes span multiple files, including updates to Jest configuration, form structures, data schemas, message definitions, and review components. A new subsection for capturing detailed health-related information has been added, alongside a new component for displaying allergies and intolerances. These modifications ensure comprehensive data collection regarding food allergies, other allergies, medical conditions, and medication requirements. Changes
Sequence DiagramsequenceDiagram
participant User
participant Form
participant AllergiesSection
participant DataValidation
participant Review
User->>Form: Starts application
Form->>AllergiesSection: Presents allergies questions
User->>AllergiesSection: Provides allergy details
AllergiesSection->>DataValidation: Validate inputs
DataValidation-->>AllergiesSection: Validate success
AllergiesSection->>Form: Save allergy information
Form->>Review: Display review section
Review->>Review: Render AllergiesAndIntolerances component
Possibly related PRs
Suggested Labels
Suggested Reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (2)libs/application/templates/new-primary-school/src/lib/NewPrimarySchoolTemplate.ts (1)Pattern
libs/application/templates/new-primary-school/src/lib/messages.ts (1)Pattern
🔇 Additional comments (4)
Finishing Touches
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
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17477 +/- ##
==========================================
- Coverage 35.65% 35.62% -0.04%
==========================================
Files 6979 7011 +32
Lines 149841 150204 +363
Branches 42836 42953 +117
==========================================
+ Hits 53427 53510 +83
- Misses 96414 96694 +280
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 45 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
libs/application/templates/new-primary-school/src/fields/FriggOptionsAsyncSelectField/index.tsx (1)
Line range hint
16-23
: Add type definition for marginBottom prop.The
marginBottom
prop is used but not defined in theFriggOptionsAsyncSelectFieldProps
interface.Apply this diff:
type FriggOptionsAsyncSelectFieldProps = { field: { props: { optionsType: OptionsType placeholder: FormText isMulti?: boolean - } + } + marginBottom?: number | string } }
🧹 Nitpick comments (8)
libs/application/templates/new-primary-school/src/fields/FriggOptionsAsyncSelectField/index.tsx (1)
Line range hint
25-89
: Consider memoizing the component for better performance.Since this component handles async operations and potentially frequent re-renders due to form updates, consider using React.memo to prevent unnecessary re-renders.
Apply this diff:
-const FriggOptionsAsyncSelectField: FC< +const FriggOptionsAsyncSelectField: FC< React.PropsWithChildren<FieldBaseProps & FriggOptionsAsyncSelectFieldProps> > = ({ error, field, application }) => { // ... component implementation } -export default FriggOptionsAsyncSelectField +export default React.memo(FriggOptionsAsyncSelectField)libs/application/templates/new-primary-school/src/fields/Review/review-groups/AllergiesAndIntolerances.tsx (2)
58-62
: Simplify loading state condition.The loading state check can be simplified for better readability.
Apply this diff:
- {(hasFoodAllergiesOrIntolerances.includes(YES) || - hasOtherAllergies.includes(YES)) && - (foodAllergiesOrIntolerancesLoading || otherAllergiesLoading) ? ( + {((hasFoodAllergiesOrIntolerances.includes(YES) && foodAllergiesOrIntolerancesLoading) || + (hasOtherAllergies.includes(YES) && otherAllergiesLoading)) ? (
80-84
: Enhance error messages with specific details.The error messages could be more specific about which data provider failed.
Apply this diff:
error={ foodAllergiesOrIntolerancesError - ? formatMessage(coreErrorMessages.failedDataProvider) + ? formatMessage( + { id: coreErrorMessages.failedDataProvider.id }, + { provider: 'food allergies' } + ) : undefined }And similarly for otherAllergiesError:
error={ otherAllergiesError - ? formatMessage(coreErrorMessages.failedDataProvider) + ? formatMessage( + { id: coreErrorMessages.failedDataProvider.id }, + { provider: 'other allergies' } + ) : undefined }Also applies to: 104-108
libs/application/templates/new-primary-school/src/lib/dataSchema.ts (1)
157-195
: Add type safety for allergy-related data.Consider adding TypeScript types for the allergy-related values to improve type safety and developer experience.
+ export type AllergiesAndIntolerances = { + hasFoodAllergiesOrIntolerances: string[] + foodAllergiesOrIntolerances?: string[] + hasOtherAllergies: string[] + otherAllergies?: string[] + usesEpiPen?: string + hasConfirmedMedicalDiagnoses: 'YES' | 'NO' + requestMedicationAssistance: 'YES' | 'NO' + }libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/allergiesAndIntolerancesSubSection.ts (2)
48-49
: Consider adding aria-label for accessibility.The FriggOptionsAsyncSelectField should include an aria-label for better accessibility.
component: 'FriggOptionsAsyncSelectField', marginBottom: 3, + ariaLabel: newPrimarySchoolMessages.differentNeeds.typeOfFoodAllergiesOrIntolerancesAriaLabel,
95-113
: Enhance alert message visibility conditions.The alert message visibility condition could be simplified for better maintainability.
- condition: (answers) => { - const { hasFoodAllergiesOrIntolerances, hasOtherAllergies } = - getApplicationAnswers(answers) - - return ( - hasFoodAllergiesOrIntolerances?.includes(YES) || - hasOtherAllergies?.includes(YES) - ) - }, + condition: (answers) => { + const { hasFoodAllergiesOrIntolerances, hasOtherAllergies } = + getApplicationAnswers(answers) + return [hasFoodAllergiesOrIntolerances, hasOtherAllergies] + .some(arr => arr?.includes(YES)) + },libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts (1)
165-171
: Group related properties in the return object.Consider grouping allergy-related properties into a nested object for better organization.
return { // ... other properties - hasFoodAllergiesOrIntolerances, - foodAllergiesOrIntolerances, - hasOtherAllergies, - otherAllergies, - usesEpiPen, - hasConfirmedMedicalDiagnoses, - requestMedicationAssistance, + allergies: { + hasFoodAllergiesOrIntolerances, + foodAllergiesOrIntolerances, + hasOtherAllergies, + otherAllergies, + usesEpiPen, + hasConfirmedMedicalDiagnoses, + requestMedicationAssistance, + }, // ... rest of the properties }libs/application/templates/new-primary-school/src/lib/messages.ts (1)
467-551
: Consider adding a free-text field for additional allergy details.The allergies section is well-structured, but it might be beneficial to add messages for a free-text field where parents can provide additional important details about their child's allergies or specific instructions for handling allergic reactions.
Add these message descriptors:
+ allergyAdditionalDetails: { + id: 'nps.application:different.needs.allergies.and.intolerances.additional.details', + defaultMessage: 'Nánari upplýsingar um ofnæmi', + description: 'Additional details about allergies', + }, + allergyAdditionalDetailsPlaceholder: { + id: 'nps.application:different.needs.allergies.and.intolerances.additional.details.placeholder', + defaultMessage: 'T.d. upplýsingar um alvarleika ofnæmis eða sérstök fyrirmæli', + description: 'E.g., information about allergy severity or special instructions', + },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
libs/application/templates/new-primary-school/jest.config.ts
(1 hunks)libs/application/templates/new-primary-school/src/fields/FriggOptionsAsyncSelectField/index.tsx
(2 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/AllergiesAndIntolerances.tsx
(1 hunks)libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/allergiesAndIntolerancesSubSection.ts
(1 hunks)libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/index.ts
(2 hunks)libs/application/templates/new-primary-school/src/lib/NewPrimarySchoolTemplate.ts
(3 hunks)libs/application/templates/new-primary-school/src/lib/dataSchema.ts
(1 hunks)libs/application/templates/new-primary-school/src/lib/messages.ts
(3 hunks)libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- libs/application/templates/new-primary-school/jest.config.ts
🧰 Additional context used
📓 Path-based instructions (9)
libs/application/templates/new-primary-school/src/fields/FriggOptionsAsyncSelectField/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/forms/NewPrimarySchoolForm/differentNeedsSection/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/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/lib/NewPrimarySchoolTemplate.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/AllergiesAndIntolerances.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/forms/NewPrimarySchoolForm/differentNeedsSection/allergiesAndIntolerancesSubSection.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/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/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/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."
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (web)
- GitHub Check: tests (judicial-system-backend)
🔇 Additional comments (3)
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/index.ts (1)
17-21
: LGTM! Well-structured section configuration.The changes maintain modularity and follow the established pattern for section configuration.
libs/application/templates/new-primary-school/src/fields/Review/index.tsx (1)
169-171
: LGTM! Proper component integration.The AllergiesAndIntolerances component is correctly integrated with consistent props passing and conditional rendering.
libs/application/templates/new-primary-school/src/lib/messages.ts (1)
759-768
: LGTM! Error messages are clear and well-structured.The error messages for allergy-related validations are properly defined and provide clear feedback to users.
libs/application/templates/new-primary-school/src/fields/Review/index.tsx
Show resolved
Hide resolved
...on/templates/new-primary-school/src/fields/Review/review-groups/AllergiesAndIntolerances.tsx
Show resolved
Hide resolved
...on/templates/new-primary-school/src/fields/Review/review-groups/AllergiesAndIntolerances.tsx
Show resolved
Hide resolved
...l/src/forms/NewPrimarySchoolForm/differentNeedsSection/allergiesAndIntolerancesSubSection.ts
Show resolved
Hide resolved
libs/application/templates/new-primary-school/src/lib/NewPrimarySchoolTemplate.ts
Outdated
Show resolved
Hide resolved
libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts
Show resolved
Hide resolved
libs/application/templates/new-primary-school/src/lib/messages.ts
Outdated
Show resolved
Hide resolved
Datadog ReportAll test runs ✅ 4 Total Test Services: 0 Failed, 4 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (1)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/application/templates/new-primary-school/src/lib/messages.ts (1)
Line range hint
1-768
: Consider extracting common health-related messages.Since this is in the
libs
directory, consider extracting common health-related messages (allergies, medical conditions) into a shared message file. This would improve reusability across different application templates that need to collect health information.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/application/templates/new-primary-school/src/lib/NewPrimarySchoolTemplate.ts
(3 hunks)libs/application/templates/new-primary-school/src/lib/messages.ts
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/application/templates/new-primary-school/src/lib/NewPrimarySchoolTemplate.ts
🧰 Additional context used
📓 Path-based instructions (1)
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 (3)
467-551
: LGTM! Well-structured messages for allergies and intolerances section.The messages are comprehensive, well-organized, and follow consistent naming patterns. The descriptions are clear and helpful for translators.
671-675
: Fix duplicate message ID for EpiPen usage.There's a duplicate message ID 'nps.application:overview.uses.epi.pen' used for both 'usesEpiPen' and a previous message. This will cause conflicts in the internationalization system.
759-768
: LGTM! Well-defined error messages.The error messages are clear, user-friendly, and follow the established pattern for validation errors.
[TS-950] Implement allergies and intolerances page
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
New Features
Bug Fixes
Documentation