-
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 page #17384
base: main
Are you sure you want to change the base?
feat(new-primary-school): Implement school page #17384
Conversation
WalkthroughThis pull request introduces a new school subsection to the primary school application form. The changes include adding a new module for the school subsection, updating the primary school section to include this new subsection, extending the data schema to capture school application preferences, and adding corresponding message translations. The implementation allows users to choose between applying to their neighborhood school or another school, with dynamic visibility based on the application's context. Changes
Sequence DiagramsequenceDiagram
participant User
participant Form
participant SchoolSubSection
participant DataSchema
User->>Form: Starts Primary School Application
Form->>SchoolSubSection: Render School Subsection
SchoolSubSection->>User: Display School Selection Options
User->>SchoolSubSection: Choose School Preference
SchoolSubSection->>DataSchema: Validate School Selection
DataSchema-->>SchoolSubSection: Validation Result
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (4)libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/index.ts (1)Pattern
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/schoolSubSection.ts (1)Pattern
libs/application/templates/new-primary-school/src/lib/dataSchema.ts (1)Pattern
libs/application/templates/new-primary-school/src/lib/messages.ts (1)Pattern
📓 Learnings (3)libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/index.ts (1)
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/schoolSubSection.ts (2)
libs/application/templates/new-primary-school/src/lib/dataSchema.ts (2)
🔇 Additional comments (7)
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 ✅ 8 Total Test Services: 0 Failed, 7 Passed Test Services
|
Datadog ReportAll test runs ✅ 4 Total Test Services: 0 Failed, 4 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (1)
|
...on/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/index.ts
Outdated
Show resolved
Hide resolved
...s/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/schoolSubSection.ts
Outdated
Show resolved
Hide resolved
...s/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/schoolSubSection.ts
Outdated
Show resolved
Hide resolved
...rimary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/currentSchoolSubSection.ts
Outdated
Show resolved
Hide resolved
…ub-section' into feat/new-primary-school-school-sub-section
neighborhoodSchool: { | ||
id: 'nps.application:overview.neigborhood.school', | ||
defaultMessage: 'Hverfisskóli', | ||
description: 'Neighborhoodschool', |
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 translation
description: 'Neighborhoodschool', | |
description: 'Neighborhood school', |
@@ -6,7 +6,7 @@ import { | |||
buildTextField, | |||
coreMessages, | |||
} from '@island.is/application/core' | |||
import { Application } from '@island.is/application/types' | |||
import { Application, NO } from '@island.is/application/types' |
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.
Remove unused import (NO
)
return applicationType === ApplicationType.NEW_PRIMARY_SCHOOL | ||
}, | ||
|
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.
Remove new line
export const schoolSubSection = buildSubSection({ | ||
id: 'schoolSubSection', | ||
title: newPrimarySchoolMessages.primarySchool.schoolSubSectionTitle, | ||
|
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.
Remove new line
import { newPrimarySchoolMessages } from '../../../lib/messages' | ||
import { getNeighborhoodSchoolName } from '../../../lib/newPrimarySchoolUtils' | ||
|
||
export const schoolSubSection = buildSubSection({ |
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.
Add condition to subSection, should only display section if ENROLLMENT_IN_PRIMARY_SCHOOL
const { applicationType } = getApplicationAnswers(answers) | ||
// Only display section if application type is "Application for a new primary school" |
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.
Why move the comment?
defaultMessage: 'Skóli', | ||
description: 'School', | ||
}, | ||
schoolDescription: { |
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.
This should be schoolSubSectionDescription
schoolDescription: { | |
schoolSubSectionDescription: { |
@@ -446,6 +446,42 @@ export const newPrimarySchoolMessages: MessageDir = { | |||
description: '{grade} grade', | |||
}, | |||
|
|||
// School page |
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.
Differences in how "neighborhood" is written ("neighborhood" / "neighbourhood")
The correct spelling is “neighborhood” in American English and “neighbourhood” in British English.
@@ -446,6 +446,42 @@ export const newPrimarySchoolMessages: MessageDir = { | |||
description: '{grade} grade', | |||
}, | |||
|
|||
// School page |
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.
Difference in how "enrolment" is written ("enrolment" / "enrollment").
British English, the correct spelling is enrolment, and in American English, it is enrollment.
const applyForNeighbourhoodSchool = getValueViaPath( | ||
answers, | ||
'school.applyForNeighbourhoodSchool', | ||
YES, |
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.
Why set defaultValue to YES?
TS-947
What
Implement new school page
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
New Features
Documentation
Data Schema