Skip to content
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): Add temporary application type and remove "Moving abroad" #17556

Merged
merged 18 commits into from
Jan 23, 2025

Conversation

veronikasif
Copy link
Member

@veronikasif veronikasif commented Jan 17, 2025

[TS-967] Add mock application type
[TS-953] Update Reason page

What

Added a new page to select application type, this page is temporary

Screenshots / Gifs

image

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Formatting passes locally with my changes
  • I have rebased against main before asking for a review

Summary by CodeRabbit

Release Notes

  • New Features

    • Added application type selection for primary school applications.
    • Introduced two application types: New Primary School and Enrollment in Primary School.
  • Improvements

    • Simplified form logic and rendering conditions.
    • Removed country-specific application requirements.
    • Enhanced form section visibility based on application type.
    • Streamlined data schema and messaging.
  • Changes

    • Removed "Moving Abroad" option from application reasons.
    • Updated review screen components to support new application types.
    • Modified form subsections to be more flexible.
    • Adjusted layout and text in various components for improved clarity.
  • User Experience

    • More intuitive application process.
    • Clearer application type selection.
    • Consistent form layout across different application scenarios.

@veronikasif veronikasif requested a review from a team as a code owner January 17, 2025 16:50
Copy link
Contributor

coderabbitai bot commented Jan 17, 2025

Walkthrough

This pull request introduces significant changes to the New Primary School application template, focusing on simplifying and standardizing the application process. The primary modifications include the introduction of a new ApplicationType concept, the removal of country-specific logic for moving abroad, and the streamlining of rendering and validation of application sections. The changes aim to create a more consistent and flexible application flow by introducing more granular control over application types and reducing conditional rendering.

Changes

File Path Change Summary
libs/application/template-api-modules/src/lib/modules/templates/new-primary-school/new-primary-school.utils.ts Simplified DTO transformation by removing conditional logic for registration and social properties.
libs/application/templates/new-primary-school/src/fields/Review/* Updated rendering logic to use applicationType, simplified GridColumn span definitions.
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/* Removed reason-based conditions, added application type checks for section visibility.
libs/application/templates/new-primary-school/src/lib/constants.ts Added ApplicationType enum, removed MOVING_ABROAD from ReasonForApplicationOptions.
libs/application/templates/new-primary-school/src/lib/dataSchema.ts Added applicationType property, removed movingAbroad validation.
libs/application/templates/new-primary-school/src/lib/messages.ts Added new messages for application type, removed country-related messages.
libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts Updated getApplicationAnswers to include applicationType, added determineNameFromApplicationAnswers function.

Sequence Diagram

sequenceDiagram
    participant User
    participant ApplicationForm
    participant ApplicationType
    participant Sections

    User->>ApplicationForm: Start Application
    ApplicationForm->>ApplicationType: Select Application Type
    ApplicationType-->>ApplicationForm: Validate Type
    ApplicationForm->>Sections: Render Sections Based on Type
    Sections-->>ApplicationForm: Display Relevant Sections
    User->>ApplicationForm: Complete Application
Loading

Possibly related PRs

Suggested Labels

automerge, deploy-feature

Suggested Reviewers

  • helgifr

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 647d5f0 and c20a2c4.

📒 Files selected for processing (2)
  • libs/application/templates/new-primary-school/src/forms/Prerequisites/applicationTypeSubSection.ts (1 hunks)
  • libs/application/templates/new-primary-school/src/forms/Prerequisites/index.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • libs/application/templates/new-primary-school/src/forms/Prerequisites/index.ts
  • libs/application/templates/new-primary-school/src/forms/Prerequisites/applicationTypeSubSection.ts

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

codecov bot commented Jan 17, 2025

Codecov Report

Attention: Patch coverage is 23.68421% with 29 lines in your changes missing coverage. Please review.

Project coverage is 35.57%. Comparing base (0dd2fc9) to head (edba9a3).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...tes/new-primary-school/src/fields/Review/index.tsx 0.00% 7 Missing ⚠️
...ew-primary-school/src/lib/newPrimarySchoolUtils.ts 44.44% 5 Missing ⚠️
...-school/src/fields/Review/review-groups/School.tsx 0.00% 3 Missing ⚠️
...NeedsSection/allergiesAndIntolerancesSubSection.ts 0.00% 3 Missing ⚠️
...rm/primarySchoolSection/currentSchoolSubSection.ts 0.00% 3 Missing ⚠️
...c/forms/Prerequisites/applicationTypeSubSection.ts 0.00% 3 Missing ⚠️
...m/primarySchoolSection/startingSchoolSubSection.ts 0.00% 2 Missing ⚠️
...tes/new-primary-school/new-primary-school.utils.ts 0.00% 1 Missing ⚠️
...elds/Review/review-groups/ReasonForApplication.tsx 0.00% 1 Missing ⚠️
...arySchoolSection/reasonForApplicationSubSection.ts 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #17556   +/-   ##
=======================================
  Coverage   35.57%   35.57%           
=======================================
  Files        7031     7032    +1     
  Lines      150510   150498   -12     
  Branches    42978    42970    -8     
=======================================
+ Hits        53537    53538    +1     
+ Misses      96973    96960   -13     
Flag Coverage Δ
air-discount-scheme-backend 48.15% <ø> (ø)
air-discount-scheme-web 0.00% <ø> (ø)
api 3.33% <ø> (ø)
api-catalogue-services 75.81% <ø> (ø)
api-domains-air-discount-scheme 37.90% <ø> (ø)
api-domains-assets 26.71% <ø> (ø)
api-domains-auth-admin 48.49% <ø> (ø)
api-domains-communications 39.49% <ø> (ø)
api-domains-criminal-record 47.81% <ø> (ø)
api-domains-driving-license 44.77% <ø> (ø)
api-domains-education 31.09% <ø> (ø)
api-domains-health-insurance 35.19% <ø> (ø)
api-domains-mortgage-certificate 34.96% <ø> (ø)
api-domains-payment-schedule 42.04% <ø> (ø)
application-api-files 61.80% <ø> (ø)
application-core 75.68% <ø> (ø)
application-system-api 38.68% <46.66%> (+0.03%) ⬆️
application-template-api-modules 27.57% <0.00%> (+<0.01%) ⬆️
application-templates-accident-notification 27.60% <ø> (ø)
application-templates-car-recycling 3.12% <ø> (ø)
application-templates-criminal-record 25.79% <ø> (ø)
application-templates-driving-license 18.15% <ø> (ø)
application-templates-estate 13.58% <ø> (ø)
application-templates-example-payment 24.66% <ø> (ø)
application-templates-financial-aid 14.99% <ø> (ø)
application-templates-general-petition 23.11% <ø> (ø)
application-templates-inheritance-report 6.59% <ø> (ø)
application-templates-marriage-conditions 14.80% <ø> (ø)
application-templates-mortgage-certificate 43.73% <ø> (ø)
application-templates-new-primary-school 20.86% <9.67%> (+0.08%) ⬆️
application-templates-parental-leave 29.92% <ø> (ø)
application-types 6.47% <ø> (ø)
application-ui-components 1.17% <ø> (ø)
application-ui-shell 21.99% <ø> (ø)
auth-admin-web 2.43% <ø> (ø)
auth-nest-tools 31.69% <ø> (ø)
auth-shared 75.00% <ø> (ø)
clients-charge-fjs-v2 28.88% <ø> (ø)
clients-driving-license 40.68% <ø> (ø)
clients-driving-license-book 43.75% <ø> (ø)
clients-financial-statements-inao 49.48% <ø> (ø)
clients-license-client 1.26% <ø> (ø)
clients-middlewares 72.49% <ø> (-0.33%) ⬇️
clients-regulations 42.75% <ø> (ø)
clients-rsk-company-registry 31.18% <ø> (ø)
clients-rsk-personal-tax-return 38.32% <ø> (ø)
clients-smartsolutions 12.77% <ø> (ø)
clients-syslumenn 49.18% <ø> (ø)
clients-zendesk 49.88% <ø> (ø)
cms 0.39% <ø> (ø)
cms-translations 38.81% <ø> (ø)
content-search-index-manager 95.65% <ø> (ø)
content-search-toolkit 8.16% <ø> (ø)
contentful-apps 4.56% <ø> (ø)
dokobit-signing 61.66% <ø> (ø)
email-service 59.68% <ø> (ø)
feature-flags 90.40% <ø> (ø)
file-storage 45.32% <ø> (ø)
financial-aid-backend 51.38% <ø> (ø)
financial-aid-shared 17.88% <ø> (ø)
icelandic-names-registry-backend 54.44% <ø> (ø)
infra-nest-server 48.06% <ø> (ø)
infra-tracing 69.94% <ø> (ø)
island-ui-core 30.32% <ø> (ø)
judicial-system-api 20.07% <ø> (ø)
judicial-system-audit-trail 68.53% <ø> (ø)
judicial-system-backend 55.81% <ø> (ø)
judicial-system-formatters 78.86% <ø> (ø)
judicial-system-message 66.29% <ø> (ø)
judicial-system-message-handler 47.89% <ø> (ø)
judicial-system-scheduler 71.24% <ø> (ø)
judicial-system-types 37.77% <ø> (ø)
judicial-system-web 27.97% <ø> (ø)
license-api 42.88% <ø> (ø)
localization 10.15% <ø> (ø)
logging 58.02% <ø> (ø)
message-queue 67.05% <ø> (ø)
nest-audit 65.78% <ø> (ø)
nest-aws 51.93% <ø> (ø)
nest-config 76.05% <ø> (ø)
nest-core 53.16% <ø> (ø)
nest-feature-flags 50.69% <ø> (ø)
nest-problem 45.70% <ø> (ø)
nest-sequelize 94.44% <ø> (ø)
portals-admin-regulations-admin 1.80% <ø> (ø)
portals-core 19.60% <ø> (ø)
regulations 16.78% <ø> (ø)
residence-history 85.00% <ø> (ø)
services-auth-admin-api 52.49% <ø> (ø)
services-auth-delegation-api 58.41% <ø> (-0.09%) ⬇️
services-auth-ids-api 52.51% <ø> (-0.01%) ⬇️
services-auth-public-api 49.38% <ø> (ø)
services-sessions 65.35% <ø> (+0.04%) ⬆️
services-university-gateway 49.39% <ø> (ø)
services-user-notification 46.49% <ø> (-0.04%) ⬇️
services-user-profile 56.93% <ø> (+0.06%) ⬆️
shared-components 29.47% <ø> (ø)
shared-form-fields 33.36% <ø> (ø)
shared-mocking 58.64% <ø> (ø)
shared-pii 92.85% <ø> (ø)
shared-problem 88.00% <ø> (ø)
shared-utils 28.67% <ø> (ø)
skilavottord-ws 23.97% <ø> (ø)
web 2.39% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...y-school/src/fields/Review/review-groups/Child.tsx 0.00% <ø> (ø)
...chool/src/fields/Review/review-groups/Contacts.tsx 0.00% <ø> (ø)
.../src/fields/Review/review-groups/CurrentSchool.tsx 0.00% <ø> (ø)
...hool/src/fields/Review/review-groups/Languages.tsx 0.00% <ø> (ø)
...school/src/fields/Review/review-groups/Parents.tsx 0.00% <ø> (ø)
...chool/src/fields/Review/review-groups/Siblings.tsx 0.00% <ø> (ø)
...ewPrimarySchoolForm/differentNeedsSection/index.ts 0.00% <ø> (ø)
...NewPrimarySchoolForm/primarySchoolSection/index.ts 0.00% <ø> (ø)
...olForm/primarySchoolSection/newSchoolSubSection.ts 0.00% <ø> (ø)
...ew-primary-school/src/forms/Prerequisites/index.ts 0.00% <ø> (ø)
... and 14 more

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0dd2fc9...edba9a3. Read the comment docs.

@datadog-island-is
Copy link

datadog-island-is bot commented Jan 17, 2025

Datadog Report

All test runs c0f8f14 🔗

10 Total Test Services: 0 Failed, 10 Passed
❄️ 1 with New Flaky
🔻 Test Sessions change in coverage: 1 decreased (-0.04%), 4 increased, 194 no change

Test Services
This report shows up to 10 services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Code Coverage Change Test Service View
services-auth-delegation-api 0 0 1 270 0 3m 36.98s 1 increased (+0.04%) Link
air-discount-scheme-backend 0 0 0 63 0 17.78s 1 no change Link
air-discount-scheme-web 0 0 0 2 0 6.58s 1 no change Link
api 0 0 0 4 0 9.23s 1 no change Link
api-catalogue-services 0 0 0 23 0 10.74s 1 no change Link
api-domains-air-discount-scheme 0 0 0 6 0 31.2s N/A Link
api-domains-assets 0 0 0 3 0 9.62s 1 no change Link
api-domains-auth-admin 0 0 0 18 0 21.96s 1 no change Link
api-domains-communications 0 0 0 5 0 33.19s N/A Link
api-domains-criminal-record 0 0 0 5 0 8.57s 1 no change Link

🔻 Code Coverage Decreases vs Default Branch (1)

  • license-api - jest 34.07% (-0.04%) - Details

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (1)
libs/application/templates/new-primary-school/src/fields/Review/review-groups/Child.tsx (1)

105-105: 🛠️ Refactor suggestion

Maintain consistency in responsive layouts.

Similar to the previous instance, removing responsive spans here could impact the layout adaptability across different screen sizes.

🧹 Nitpick comments (4)
libs/application/templates/new-primary-school/src/fields/Review/review-groups/ReasonForApplication.tsx (1)

51-51: Consider maintaining consistent GridColumn span patterns.

The GridColumn span is defined as a string "12/12", while other GridColumns in the file use array notation ['12/12', '12/12', '12/12', '5/12']. Consider maintaining consistency in how spans are defined across the component.

-            <GridColumn span="12/12">
+            <GridColumn span={['12/12', '12/12', '12/12', '12/12']}>
libs/application/templates/new-primary-school/src/forms/Prerequisites/applicationTypeSubSection.ts (1)

9-27: Add aria-label for better accessibility.

The radio field implementation looks good, but could benefit from accessibility improvements.

Add aria-label to improve screen reader experience:

 buildRadioField({
   id: 'applicationType',
   title: newPrimarySchoolMessages.pre.applicationTypeSubSectionTitle,
   description:
     newPrimarySchoolMessages.pre.applicationTypeSubSectionDescription,
+  ariaLabel: newPrimarySchoolMessages.pre.applicationTypeSubSectionTitle,
   options: [
     {
       value: ApplicationType.NEW_PRIMARY_SCHOOL,
       dataTestId: 'new-primary-school',
       label: newPrimarySchoolMessages.shared.applicationName,
     },
     // ...
   ],
   required: true,
 })
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/currentSchoolSubSection.ts (1)

33-33: Consider adding a description message.

The title change makes it more concise, but consider adding a description field to provide more context about what "current school" means.

libs/application/templates/new-primary-school/src/fields/Review/index.tsx (1)

173-176: Consider extracting repeated conditions into constants.

The condition for siblings could be extracted into a constant at the top of the component for better maintainability.

+ const shouldShowSiblings = reasonForApplication === 
+   ReasonForApplicationOptions.SIBLINGS_IN_SAME_SCHOOL;

  // Later in the JSX
- {reasonForApplication === 
-   ReasonForApplicationOptions.SIBLINGS_IN_SAME_SCHOOL && (
+ {shouldShowSiblings && (
    <Siblings {...childProps} />
  )}
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0a548dd and ed8e16d.

📒 Files selected for processing (24)
  • 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/Review/index.tsx (3 hunks)
  • libs/application/templates/new-primary-school/src/fields/Review/review-groups/Child.tsx (2 hunks)
  • libs/application/templates/new-primary-school/src/fields/Review/review-groups/Contacts.tsx (1 hunks)
  • libs/application/templates/new-primary-school/src/fields/Review/review-groups/CurrentSchool.tsx (2 hunks)
  • libs/application/templates/new-primary-school/src/fields/Review/review-groups/Languages.tsx (4 hunks)
  • libs/application/templates/new-primary-school/src/fields/Review/review-groups/Parents.tsx (1 hunks)
  • libs/application/templates/new-primary-school/src/fields/Review/review-groups/ReasonForApplication.tsx (2 hunks)
  • libs/application/templates/new-primary-school/src/fields/Review/review-groups/School.tsx (3 hunks)
  • libs/application/templates/new-primary-school/src/fields/Review/review-groups/Siblings.tsx (1 hunks)
  • libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/allergiesAndIntolerancesSubSection.ts (2 hunks)
  • libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/index.ts (0 hunks)
  • libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/currentSchoolSubSection.ts (2 hunks)
  • libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/index.ts (1 hunks)
  • libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/newSchoolSubSection.ts (0 hunks)
  • libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/reasonForApplicationSubSection.ts (1 hunks)
  • libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/startingSchoolSubSection.ts (1 hunks)
  • libs/application/templates/new-primary-school/src/forms/Prerequisites/applicationTypeSubSection.ts (1 hunks)
  • libs/application/templates/new-primary-school/src/forms/Prerequisites/index.ts (2 hunks)
  • libs/application/templates/new-primary-school/src/lib/NewPrimarySchoolTemplate.ts (1 hunks)
  • libs/application/templates/new-primary-school/src/lib/constants.ts (1 hunks)
  • libs/application/templates/new-primary-school/src/lib/dataSchema.ts (2 hunks)
  • libs/application/templates/new-primary-school/src/lib/messages.ts (3 hunks)
  • libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts (3 hunks)
💤 Files with no reviewable changes (2)
  • libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/index.ts
  • libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/newSchoolSubSection.ts
✅ Files skipped from review due to trivial changes (5)
  • libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/index.ts
  • libs/application/templates/new-primary-school/src/fields/Review/review-groups/Languages.tsx
  • libs/application/templates/new-primary-school/src/fields/Review/review-groups/Contacts.tsx
  • libs/application/templates/new-primary-school/src/fields/Review/review-groups/Siblings.tsx
  • libs/application/templates/new-primary-school/src/fields/Review/review-groups/Parents.tsx
🧰 Additional context used
📓 Path-based instructions (17)
libs/application/templates/new-primary-school/src/forms/Prerequisites/applicationTypeSubSection.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/startingSchoolSubSection.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/Child.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/Prerequisites/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/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/lib/constants.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/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/Review/review-groups/School.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/fields/Review/review-groups/CurrentSchool.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/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/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/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/fields/Review/review-groups/ReasonForApplication.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/primarySchoolSection/reasonForApplicationSubSection.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 (5)
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/startingSchoolSubSection.ts (1)
Learnt from: veronikasif
PR: island-is/island.is#16096
File: libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/newSchoolSubSection.ts:81-90
Timestamp: 2024-11-12T15:15:20.157Z
Learning: When checking conditions involving variables like `schoolMunicipality` and `newSchoolHiddenInput`, ensure to verify that `schoolMunicipality` has a value before comparing it to `newSchoolHiddenInput`.
libs/application/templates/new-primary-school/src/lib/constants.ts (1)
Learnt from: veronikasif
PR: island-is/island.is#16304
File: libs/application/templates/new-primary-school/src/lib/constants.ts:29-31
Timestamp: 2024-11-12T15:15:11.835Z
Learning: In `libs/application/templates/new-primary-school/src/lib/constants.ts`, the enum string values in `ReasonForApplicationOptions` may match server-provided values, even if they appear to have typos. Avoid flagging these as typos.
libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/currentSchoolSubSection.ts (1)
Learnt from: veronikasif
PR: island-is/island.is#16096
File: libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/newSchoolSubSection.ts:81-90
Timestamp: 2024-11-12T15:15:20.157Z
Learning: When checking conditions involving variables like `schoolMunicipality` and `newSchoolHiddenInput`, ensure to verify that `schoolMunicipality` has a value before comparing it to `newSchoolHiddenInput`.
libs/application/templates/new-primary-school/src/lib/dataSchema.ts (2)
Learnt from: veronikasif
PR: island-is/island.is#16304
File: libs/application/templates/new-primary-school/src/lib/dataSchema.ts:79-79
Timestamp: 2024-11-12T15:15:20.157Z
Learning: In `libs/application/templates/new-primary-school/src/lib/dataSchema.ts`, for the `reasonForApplication` field, since the options come from the server, we cannot use `z.nativeEnum` or `z.enum`, and should use `z.string()` instead.
Learnt from: veronikasif
PR: island-is/island.is#16304
File: libs/application/templates/new-primary-school/src/lib/dataSchema.ts:103-103
Timestamp: 2024-11-12T15:15:20.157Z
Learning: In `libs/application/templates/new-primary-school/src/lib/dataSchema.ts`, when comparing the `reason` field in `reasonForApplication`, which comes from the server as a string, avoid comparing it directly to `ReasonForApplicationOptions` enum values, as the enum may not include all server-provided values.
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 (21)
libs/application/templates/new-primary-school/src/fields/Review/review-groups/ReasonForApplication.tsx (2)

Line range hint 1-26: LGTM! Well-structured imports and props.

The code follows best practices with:

  • Named imports enabling effective tree-shaking
  • Proper TypeScript usage for props
  • Reusable components from shared libraries

Line range hint 27-93: LGTM! Well-implemented review component.

The implementation:

  • Properly handles loading and error states
  • Uses shared UI components effectively
  • Correctly implements conditional rendering based on application type
  • Successfully removes the "Moving abroad" related logic as per PR objectives
libs/application/templates/new-primary-school/src/fields/Review/review-groups/Child.tsx (1)

Line range hint 1-138: Verify responsive behavior across all review group components.

The changes to fixed spans appear to be part of a broader pattern across review group components. This could affect the overall responsive design strategy.

Let's check for similar changes in other review group components:

✅ Verification successful

Responsive behavior is consistent across review group components

The span properties follow a consistent pattern across all review group components, with appropriate responsive breakpoints and column widths. The changes in the Child component align with the established responsive design strategy.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for similar span property changes in other review group components
# Look for both array-based and string-based span properties to assess consistency

echo "Checking for array-based spans..."
rg "span=\{\[.*\]\}" "libs/application/templates/new-primary-school/src/fields/Review/review-groups/"

echo "Checking for string-based spans..."
rg "span=\"\d+/\d+\"" "libs/application/templates/new-primary-school/src/fields/Review/review-groups/"

Length of output: 6352

libs/application/templates/new-primary-school/src/forms/Prerequisites/index.ts (1)

4-4: LGTM! Logical ordering of subsections.

The changes maintain a clear separation of concerns and follow a logical flow: application type selection, followed by external data, and then children information.

Also applies to: 18-22

libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/startingSchoolSubSection.ts (1)

Line range hint 24-30: Verify date validation across timezones.

The minDate validation using new Date() might behave differently across timezones.

Consider using a more specific date validation:

-minDate: () => new Date(),
+minDate: () => {
+  const today = new Date()
+  today.setHours(0, 0, 0, 0)
+  return today
+},
libs/application/templates/new-primary-school/src/lib/constants.ts (1)

57-61: LGTM! Well-structured enum definition.

The new ApplicationType enum follows TypeScript best practices with clear, descriptive string literal values.

libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/currentSchoolSubSection.ts (1)

20-24: LGTM! Clear conditional rendering logic.

The condition effectively controls section visibility based on application type, with good documentation.

libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/primarySchoolSection/reasonForApplicationSubSection.ts (1)

82-87: LGTM! Simplified condition logic.

The alert message condition has been streamlined to focus solely on MOVING_MUNICIPALITY, improving code clarity.

libs/application/templates/new-primary-school/src/fields/Review/review-groups/School.tsx (2)

92-101: LGTM! Well-structured conditional rendering.

The date field is now correctly shown only for NEW_PRIMARY_SCHOOL applications, improving component reusability.


53-53: Verify responsive behavior.

The GridColumn span has been simplified from an array to a string. Please ensure this doesn't affect the component's responsive behavior across different screen sizes.

✅ Verification successful

Fixed span "12/12" is correct for the heading section

The simplified span is intentional and appropriate as this GridColumn contains a section heading that should span the full width across all breakpoints. The file correctly implements responsive behavior where needed, using array notation for content sections that require different spans across breakpoints.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other GridColumn components to verify consistent span usage
rg -A 1 "GridColumn.*span=" --type tsx

Length of output: 68


Script:

#!/bin/bash
# Search for GridColumn span patterns in TypeScript/React files
rg -g '*.{ts,tsx}' -B 1 -A 2 'GridColumn.*span='

# Alternative search to find all GridColumn usages
rg -g '*.{ts,tsx}' -B 1 -A 2 '<GridColumn'

Length of output: 69510


Script:

#!/bin/bash
# Check the file history for the specific line
git log -L :GridColumn:libs/application/templates/new-primary-school/src/fields/Review/review-groups/School.tsx

Length of output: 0


Script:

#!/bin/bash
# Find and show the specific file content
rg -l "review-groups/School" -g "*.tsx"
cat libs/application/templates/new-primary-school/src/fields/Review/review-groups/School.tsx

Length of output: 3779

libs/application/templates/new-primary-school/src/lib/dataSchema.ts (1)

20-23: LGTM! Type-safe application type validation.

The addition of the applicationType field with strict enum validation ensures type safety and prevents invalid values.

libs/application/templates/new-primary-school/src/forms/NewPrimarySchoolForm/differentNeedsSection/allergiesAndIntolerancesSubSection.ts (1)

19-23: LGTM! Clear conditional rendering logic.

The condition is well-implemented and clearly documented, ensuring the section is only displayed for enrollment applications.

libs/application/templates/new-primary-school/src/fields/Review/index.tsx (2)

168-170: LGTM! Appropriate conditional rendering.

The CurrentSchool component is correctly rendered only for new primary school applications.


179-181: LGTM! Clear enrollment-specific component rendering.

The AllergiesAndIntolerances component is correctly rendered only for enrollment applications.

libs/application/templates/new-primary-school/src/lib/NewPrimarySchoolTemplate.ts (1)

34-37: LGTM! Good improvement in application name handling.

The change from static to dynamic application name determination improves flexibility and context-awareness of the template.

Also applies to: 45-45

libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts (3)

20-21: LGTM! Clean implementation of application type handling.

The addition of application type extraction follows the established pattern and user preferences for type casting.

Also applies to: 24-28


163-163: LGTM! Consistent return object update.

The applicationType is properly added to the return object, maintaining alphabetical ordering.


343-351: LGTM! Well-structured new function.

The determineNameFromApplicationAnswers function is clear, focused, and follows the single responsibility principle.

libs/application/templates/new-primary-school/src/lib/messages.ts (3)

13-17: LGTM! Well-structured message definition.

The new enrollment application name message follows the established pattern with clear ID and description.


123-132: LGTM! Clear and user-friendly application type messages.

The new application type messages are well-structured and provide clear guidance to users.


391-392: LGTM! Simplified message key.

The rename from currentSchoolInfo to currentSchool improves clarity while maintaining the same content.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (3)
libs/application/templates/new-primary-school/src/forms/Prerequisites/index.ts (1)

22-28: LGTM! Consider enhancing type safety.

The conditional section inclusion is well-implemented. For additional type safety, consider explicitly typing the children array.

-      children: [
+      children: Array<typeof applicationTypeSubSection | typeof externalDataSubSection | typeof childrenSubSection> = [
libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts (2)

24-30: Consider adding type guard for applicationType.

The implementation looks good and maintains backward compatibility. However, consider adding a type guard to ensure type safety:

-  let applicationType = getValueViaPath(
+  let applicationType = getValueViaPath<ApplicationType>(
     answers,
     'applicationType',
   ) as ApplicationType

Also applies to: 165-165


345-353: Add JSDoc documentation for better code maintainability.

The function implementation is clean and follows best practices. Consider adding JSDoc documentation to improve maintainability:

+/**
+ * Determines the application name based on the application type.
+ * @param application - The application object containing answers
+ * @returns The appropriate application name message based on the application type
+ */
 export const determineNameFromApplicationAnswers = (
   application: Application,
 ) => {
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed8e16d and 647d5f0.

📒 Files selected for processing (2)
  • libs/application/templates/new-primary-school/src/forms/Prerequisites/index.ts (2 hunks)
  • libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
libs/application/templates/new-primary-school/src/forms/Prerequisites/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."
📓 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 (4)
libs/application/templates/new-primary-school/src/forms/Prerequisites/index.ts (2)

3-3: LGTM! Well-structured imports.

The imports follow good practices by:

  • Utilizing shared utilities for environment checks
  • Maintaining proper module boundaries for form sections

Also applies to: 5-5


9-10: Verify testing coverage across environments.

While the environment-based feature flag is clear, ensure that testing strategies account for both production and non-production scenarios to maintain quality across environments.

Consider:

  1. Adding tests for both environmental conditions
  2. Documenting the rationale for production exclusion
  3. Planning for eventual production release
libs/application/templates/new-primary-school/src/lib/newPrimarySchoolUtils.ts (2)

12-12: LGTM: Import statements are properly organized.

The new imports are correctly placed and used within the file.

Also applies to: 20-20


Line range hint 1-353: LGTM: Code complies with coding guidelines.

The implementation adheres to all specified guidelines:

  • Uses TypeScript effectively for type definitions
  • Functions are reusable across different NextJS apps
  • Follows proper export patterns for effective tree-shaking

Copy link
Member

@helgifr helgifr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@veronikasif veronikasif enabled auto-merge January 21, 2025 13:08
@veronikasif veronikasif disabled auto-merge January 21, 2025 17:59
@veronikasif veronikasif enabled auto-merge January 21, 2025 17:59
@veronikasif veronikasif added the autoupdate Branch gets auto updated label Jan 22, 2025
veronikasif and others added 2 commits January 22, 2025 14:24
…i-domains-air-discount-scheme,api-domains-application,api-domains-auth,api-domains-auth-admin,api-domains-document-provider,api-domains-endorsement-system update dirty files
@andes-it andes-it requested a review from a team as a code owner January 22, 2025 15:19
@datadog-island-is
Copy link

datadog-island-is bot commented Jan 23, 2025

Datadog Report

All test runs 5eab82c 🔗

10 Total Test Services: 0 Failed, 10 Passed
🔻 Test Sessions change in coverage: 1 decreased (-0.04%), 2 increased, 195 no change

Test Services
This report shows up to 10 services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Code Coverage Change Test Service View
air-discount-scheme-backend 0 0 0 63 0 28.89s N/A Link
air-discount-scheme-web 0 0 0 2 0 7.06s 1 no change Link
api 0 0 0 4 0 2.95s 1 no change Link
api-catalogue-services 0 0 0 23 0 11.22s N/A Link
api-domains-air-discount-scheme 0 0 0 6 0 17.02s N/A Link
api-domains-assets 0 0 0 3 0 10.79s N/A Link
api-domains-auth-admin 0 0 0 18 0 16.73s 1 no change Link
api-domains-communications 0 0 0 5 0 43.93s 1 no change Link
api-domains-criminal-record 0 0 0 5 0 8.74s N/A Link
api-domains-driving-license 0 0 0 23 0 31.21s 1 no change Link

🔻 Code Coverage Decreases vs Default Branch (1)

  • services-auth-delegation-api - jest 50.42% (-0.04%) - Details

@veronikasif veronikasif added this pull request to the merge queue Jan 23, 2025
Merged via the queue into main with commit 3d3e949 Jan 23, 2025
137 checks passed
@veronikasif veronikasif deleted the feat/new-primary-school-mock-application-type branch January 23, 2025 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autoupdate Branch gets auto updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants