-
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 school meal page #17513
Conversation
WalkthroughThis pull request introduces a feature for managing free school meal options in a primary school application. The changes encompass multiple files to implement a new subsection in the application form that captures details about free school lunch acceptance and special needs. This includes the addition of a review component, state management actions, data schema validation, and localized messages to support the new functionality. Changes
Sequence DiagramsequenceDiagram
participant User
participant Form
participant FreeSchoolMealSection
participant StateManager
User->>Form: Starts application
Form->>FreeSchoolMealSection: Render free school meal fields
FreeSchoolMealSection-->>User: Display acceptance and special needs options
User->>FreeSchoolMealSection: Select options
FreeSchoolMealSection->>StateManager: Update application state
StateManager->>Form: Validate input
Form-->>User: Confirm or request corrections
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
Datadog ReportBranch report: ✅ 0 Failed, 4 Passed, 0 Skipped, 3.1s Total Time |
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: 1
🧹 Nitpick comments (6)
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/index.ts (1)
20-20
: Consider reordering subsections for better user experience.The placement of
freeSchoolMealSubSection
betweenlanguageSubSection
andallergiesAndIntolerancesSubSection
might not provide the most intuitive flow, as food allergies are closely related to school meals. Consider moving it afterallergiesAndIntolerancesSubSection
for a more logical grouping.children: [ languageSubSection, - freeSchoolMealSubSection, allergiesAndIntolerancesSubSection, + freeSchoolMealSubSection, supportSubSection, ],libs/application/templates/new-primary-school/src/fields/Review/review-groups/FreeSchoolMeal.tsx (1)
39-96
: Consider adding loading states for all conditional renders.The loading state is only shown when both
acceptFreeSchoolLunch
andhasSpecialNeeds
areYES
. Consider showing loading states for each conditional block to improve user experience during data fetching.return ( <ReviewGroup isEditable={editable} editAction={() => goToScreen?.('freeSchoolMeal')} > <Stack space={2}> - {acceptFreeSchoolLunch === YES && hasSpecialNeeds === YES && loading ? ( + {loading ? ( <SkeletonLoader height={40} width="80%" borderRadius="large" /> ) : ( <> <GridRow>libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/freeSchoolMealSubSection.ts (1)
82-94
: Consider moving the alert message before the special needs type field.The alert message about food allergies should appear before asking for the special needs type to better inform the user's selection.
children: [ buildRadioField({...}), buildRadioField({...}), + buildAlertMessageField({ + id: 'freeSchoolMeal.foodAlergiesAlertMessage', + title: newPrimarySchoolMessages.shared.alertTitle, + message: + newPrimarySchoolMessages.differentNeeds.foodAlergiesAlertMessage, + doesNotRequireAnswer: true, + alertType: 'info', + condition: (answers) => { + const { acceptFreeSchoolLunch, hasSpecialNeeds } = + getApplicationAnswers(answers) + + return acceptFreeSchoolLunch === YES && hasSpecialNeeds === YES + }, + }), buildCustomField({...}), - buildAlertMessageField({...}), ],libs/application/templates/new-primary-school/src/fields/Review/index.tsx (1)
170-170
: Consider grouping related review sections.The
FreeSchoolMeal
component should be placed afterAllergiesAndIntolerances
for a more logical flow, as they are related concerns.<School {...childProps} /> <Languages {...childProps} /> - <FreeSchoolMeal {...childProps} /> <AllergiesAndIntolerances {...childProps} /> + <FreeSchoolMeal {...childProps} /> <Support {...childProps} />libs/application/templates/new-primary-school/src/lib/dataSchema.ts (1)
157-180
: Add error messages to refinement rules and simplify the second refinement.The schema structure is well-defined, but the refinement rules could be improved:
- Add error messages to help users understand validation failures
- Simplify the second refinement by combining conditions
freeSchoolMeal: z .object({ acceptFreeSchoolLunch: z.enum([YES, NO]), hasSpecialNeeds: z.string().optional(), specialNeedsType: z.string().optional(), }) .refine( ({ acceptFreeSchoolLunch, hasSpecialNeeds }) => acceptFreeSchoolLunch === YES ? !!hasSpecialNeeds && hasSpecialNeeds.length > 0 : true, { path: ['hasSpecialNeeds'], + params: errorMessages.hasSpecialNeedsRequired, }, ) .refine( - ({ acceptFreeSchoolLunch, hasSpecialNeeds, specialNeedsType }) => - acceptFreeSchoolLunch === YES && hasSpecialNeeds === YES - ? !!specialNeedsType && specialNeedsType.length > 0 - : true, + ({ hasSpecialNeeds, specialNeedsType }) => + hasSpecialNeeds === YES + ? !!specialNeedsType && specialNeedsType.length > 0 + : true, { path: ['specialNeedsType'], + params: errorMessages.specialNeedsTypeRequired, }, ),libs/application/templates/new-primary-school/src/lib/NewPrimarySchoolTemplate.ts (1)
251-264
: Add a descriptive comment for the clearFreeSchoolMeal action.The implementation is correct, but adding a comment would improve code maintainability by explaining the purpose and conditions of the field clearing logic.
+ // Clear special needs fields if free school lunch is not accepted + // Clear special needs type if special needs is not indicated clearFreeSchoolMeal: assign((context) => { const { application } = context const { acceptFreeSchoolLunch, hasSpecialNeeds } =
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
libs/application/templates/new-primary-school/src/fields/Review/index.tsx
(2 hunks)libs/application/templates/new-primary-school/src/fields/Review/review-groups/FreeSchoolMeal.tsx
(1 hunks)libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/freeSchoolMealSubSection.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
(2 hunks)libs/application/templates/new-primary-school/src/lib/dataSchema.ts
(1 hunks)libs/application/templates/new-primary-school/src/lib/messages.ts
(1 hunks)libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
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/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/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/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/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/fields/Review/review-groups/FreeSchoolMeal.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/freeSchoolMealSubSection.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."
📓 Learnings (1)
libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts (1)
Learnt from: veronikasif
PR: island-is/island.is#17477
File: libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts:82-115
Timestamp: 2025-01-13T14:20:07.540Z
Learning: In the new-primary-school application template, the user prefers to keep the type casting approach for extracting allergies data from application answers, rather than introducing additional type safety measures.
🔇 Additional comments (9)
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/index.ts (1)
6-6
: LGTM!The import statement follows the established pattern and maintains consistency with other imports.
libs/application/templates/new-primary-school/src/fields/Review/review-groups/FreeSchoolMeal.tsx (2)
1-22
: LGTM!The imports are well-organized and follow the project's conventions. Props are properly typed using the shared
ReviewGroupProps
interface.
33-37
: Update the options type for special needs.The TODO comment indicates that the wrong options type is being used (
ALLERGY
). This should be updated when the correct key-options are available.✅ Verification successful
Temporary usage of ALLERGY options type is already tracked
The review comment is correct in identifying the issue. However, this is already being tracked by TODO comments, indicating that the team is aware and waiting for proper key-options to be added by Júní. No immediate action is needed until the new options are available.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are other components using ALLERGY options type rg "OptionsType\.ALLERGY" -A 2 -B 2Length of output: 3088
Script:
#!/bin/bash # Check for all available OptionsType values rg "OptionsType\." -g "!*.test.*" -g "!*.spec.*" -g "!*.d.ts" -A 0 -B 0 | sort -u # Check for recent changes in options-related files fd "options?" --type f --exec ls -l {} \;Length of output: 6749
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/freeSchoolMealSubSection.ts (2)
1-12
: LGTM!The imports are well-organized and follow the project's conventions.
75-75
: Update the options type for special needs.The TODO comment indicates that the wrong options type is being used (
ALLERGY
). This should be updated when the correct key-options are available.libs/application/templates/new-primary-school/src/fields/Review/index.tsx (1)
21-21
: LGTM!The import statement follows the established pattern and maintains consistency with other review group imports.
libs/application/templates/new-primary-school/src/lib/NewPrimarySchoolTemplate.ts (1)
97-97
: LGTM!The clearFreeSchoolMeal action is correctly added to the exit array, following the established pattern of other clear actions.
libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts (2)
82-96
: LGTM!The implementation follows the established pattern and correctly uses type casting as preferred in this codebase.
180-182
: LGTM!The new variables are correctly added to the return object, maintaining alphabetical order and consistent structure.
libs/application/templates/new-primary-school/src/lib/messages.ts
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17513 +/- ##
==========================================
+ Coverage 35.60% 35.63% +0.03%
==========================================
Files 7016 7017 +1
Lines 150332 150191 -141
Branches 42947 42921 -26
==========================================
- Hits 53533 53528 -5
+ Misses 96799 96663 -136
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 16 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
libs/application/templates/new-primary-school/src/lib/messages.ts
Outdated
Show resolved
Hide resolved
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: 1
🧹 Nitpick comments (1)
libs/application/templates/new-primary-school/src/lib/dataSchema.ts (1)
159-176
: Add error messages to refinement rules.The validation rules are logically correct, but they lack error messages to guide users when validation fails.
Add error messages to both refinement rules:
.refine( ({ acceptFreeSchoolLunch, hasSpecialNeeds }) => acceptFreeSchoolLunch === YES ? !!hasSpecialNeeds && hasSpecialNeeds.length > 0 : true, { path: ['hasSpecialNeeds'], + params: errorMessages.specialNeedsRequired, }, ) .refine( ({ acceptFreeSchoolLunch, hasSpecialNeeds, specialNeedsType }) => acceptFreeSchoolLunch === YES && hasSpecialNeeds === YES ? !!specialNeedsType && specialNeedsType.length > 0 : true, { path: ['specialNeedsType'], + params: errorMessages.specialNeedsTypeRequired, }, ),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/freeSchoolMealSubSection.ts
(1 hunks)libs/application/templates/new-primary-school/src/lib/dataSchema.ts
(1 hunks)libs/application/templates/new-primary-school/src/lib/messages.ts
(1 hunks)libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts
- libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/freeSchoolMealSubSection.ts
🧰 Additional context used
📓 Path-based instructions (2)
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."
🔇 Additional comments (1)
libs/application/templates/new-primary-school/src/lib/messages.ts (1)
452-494
: LGTM! The message definitions are well-structured.The new messages for the free school meal section are properly defined with:
- Consistent ID naming pattern
- Clear descriptions in English
- Appropriate translations in Icelandic
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
[TS-954] Implement school meal 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
Documentation
Improvements