-
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): Current school page #17451
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThis pull request introduces enhancements to the new primary school application template, focusing on improving the representation of current school information. A new Changes
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 ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (2) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17451 +/- ##
==========================================
- Coverage 35.67% 35.66% -0.01%
==========================================
Files 6936 6941 +5
Lines 148940 149169 +229
Branches 42560 42778 +218
==========================================
+ Hits 53127 53203 +76
- Misses 95813 95966 +153
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 51 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: 1
🧹 Nitpick comments (2)
libs/application/templates/new-primary-school/src/fields/Grade/index.tsx (1)
15-45
: Add error handling for external data.The component looks well-structured, but consider adding error handling for cases where
childGradeLevel
might be undefined from external data.Consider adding a fallback:
const { childGradeLevel } = getApplicationExternalData( application.externalData, ) + if (childGradeLevel === undefined) { + return ( + <TextFormField + application={application} + showFieldName + field={{ + ...field, + disabled: true, + defaultValue: formatMessage(newPrimarySchoolMessages.shared.notAvailable), + }} + /> + ) + }libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/currentSchoolSubSection.ts (1)
13-114
: Consider splitting the form into smaller components.The current structure has a large number of fields in a single subsection. Consider splitting this into smaller, more focused components for better maintainability.
For example:
- TeacherInfoComponent
- ProsperityContactComponent
- CaseManagerComponent
This would make the code more modular and easier to maintain.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
libs/application/templates/new-primary-school/src/fields/Grade/index.tsx
(1 hunks)libs/application/templates/new-primary-school/src/fields/index.ts
(1 hunks)libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/childrenNParentsSection/childInfoSubSection.ts
(0 hunks)libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/currentSchoolSubSection.ts
(1 hunks)libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/index.ts
(2 hunks)libs/application/templates/new-primary-school/src/lib/messages.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/childrenNParentsSection/childInfoSubSection.ts
🧰 Additional context used
📓 Path-based instructions (5)
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/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/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/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/forms/NewPrimarySchoolForm/primarySchoolSection/currentSchoolSubSection.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/Grade/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 (3)
libs/application/templates/new-primary-school/src/fields/index.ts (1)
2-2
: LGTM! Export follows consistent pattern.The new Grade export follows the established barrel file pattern.
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/index.ts (1)
3-3
: LGTM! Logical placement of the new section.The currentSchoolSubSection is appropriately imported and placed in a logical order within the children array, between siblingsSubSection and newSchoolSubSection.
Also applies to: 15-15
libs/application/templates/new-primary-school/src/lib/messages.ts (1)
423-454
: Well-structured message definitions!The new message definitions for the "Current school" section follow best practices:
- Consistent naming pattern for message IDs
- Clear English descriptions for translators
- Proper TypeScript usage with
defineMessages
- Modular organization enabling effective tree-shaking
// TODO: Use correct value | ||
defaultValue: (application: Application) => | ||
getCurrentSchoolName(application), |
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.
Fix placeholder values for contact information.
Multiple fields are currently using getCurrentSchoolName
as a placeholder value for different contact information (supervising teacher, prosperity contact, case manager). This needs to be updated with the correct data retrieval functions.
Would you like me to help create the appropriate data retrieval functions for each contact type? This would include:
- Supervising teacher details
- Prosperity contact details
- Case manager details
Also applies to: 61-63, 75-77, 84-86, 98-100, 107-109
...
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
Changes
The updates provide more detailed and user-friendly school information capture in the application process.