Skip to content

fix(benefit): translations and error messages - #4188

Open
ext-hjasplund wants to merge 2 commits into
mainfrom
hl-hotfixes-handler
Open

fix(benefit): translations and error messages#4188
ext-hjasplund wants to merge 2 commits into
mainfrom
hl-hotfixes-handler

Conversation

@ext-hjasplund

@ext-hjasplund ext-hjasplund commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Description ✨

Translations were missing from one field.
Missing oolean fields did not give error message if on submit.

Issues 🐛

HL-1824
HL-1840

Testing ⚗️

Jest tests

Summary by CodeRabbit

  • New Features

    • Added/updated employee-count labels for Finnish and Swedish benefit change forms.
  • Bug Fixes

    • Improved validation behavior so invalid fields are marked more reliably, including proper accessibility states.
    • Enhanced Form validation flow to mark fields as touched and focus the first invalid field before saving/submitting.
  • Tests

    • Added coverage for validation and save behaviors in the application form hook.

Translation was missing. Correct translation
added.

Refs: HL-1824
@ext-hjasplund
ext-hjasplund requested a review from a team as a code owner June 29, 2026 06:16
@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds recursive touched-state handling to useApplicationForm, updates validation and save flows to set touched on errors, adds invalid props to two SelectionGroup fields, and introduces the companyNumberOfEmployees translation key in three locale files.

Changes

Form Validation and Localization

Layer / File(s) Summary
getTouchedFromErrors helper and validation flow
src/components/applicationForm/useApplicationForm.ts, src/components/applicationForm/__tests__/useApplicationForm.test.tsx
Imports FormikTouched, adds a recursive getTouchedFromErrors helper, updates handleSave and handleValidation to set Formik touched state after validation errors, and adds tests for touched, scroll, and submit behavior.
SelectionGroup invalid/aria-invalid props
src/components/applicationForm/formContent/companySection/CompanySection.tsx
Adds invalid and aria-invalid props derived from getErrorMessage to SelectionGroup for purchasedService and coOperationNegotiations.
Locale key for companyNumberOfEmployees
public/locales/en/common.json, public/locales/fi/common.json, public/locales/sv/common.json
Adds changes.fields.companyNumberOfEmployees.label in all three locale files; the Finnish entry replaces the previous employee-count key.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • City-of-Helsinki/yjdh#4152 — Also changes frontend/benefit/handler/src/components/applicationForm/formContent/companySection/CompanySection.tsx and the same field-group validation surface.

Suggested reviewers

  • tjgofore
  • charn

Poem

🐇 I hopped through fields both bright and new,
Touched errors woke and labels grew.
Three locales sang the same fresh tune,
While red flags bloomed just in time for noon.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: translation fixes and improved error messaging in the benefit flow.
Description check ✅ Passed The description includes Description, Issues, and Testing sections and is mostly complete, though Screenshots and Additional notes are empty.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hl-hotfixes-handler

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread frontend/benefit/handler/public/locales/en/common.json

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend/benefit/handler/public/locales/en/common.json`:
- Around line 1885-1887: The new companyNumberOfEmployees label in the English
locale still uses Finnish text, so replace the value in the common.json English
translation entry with proper English copy. Update the companyNumberOfEmployees
object in the locale file so English users see a translated label instead of the
current untranslated string.

In `@frontend/benefit/handler/public/locales/sv/common.json`:
- Around line 1885-1887: The Swedish locale entry for companyNumberOfEmployees
still uses Finnish text, so replace the label value in common.json with proper
Swedish copy. Update the companyNumberOfEmployees label in the Swedish
translations to match the rest of the sv locale and ensure no Finnish text
remains for that key.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b495cec0-0128-408e-96ba-9b119941e57a

📥 Commits

Reviewing files that changed from the base of the PR and between a469e00 and 64f96f5.

📒 Files selected for processing (5)
  • frontend/benefit/handler/public/locales/en/common.json
  • frontend/benefit/handler/public/locales/fi/common.json
  • frontend/benefit/handler/public/locales/sv/common.json
  • frontend/benefit/handler/src/components/applicationForm/formContent/companySection/CompanySection.tsx
  • frontend/benefit/handler/src/components/applicationForm/useApplicationForm.ts

Comment thread frontend/benefit/handler/public/locales/en/common.json
Comment thread frontend/benefit/handler/public/locales/sv/common.json
@azure-pipelines

Copy link
Copy Markdown

YJDH-HELSINKILISA-API branch is deployed to platta: https://helsinkilisa-pr4188.api.dev.hel.ninja 🚀🚀🚀

@azure-pipelines

Copy link
Copy Markdown

HANDLER branch is deployed to platta: https://helsinkilisa-ui-handler-pr4188.dev.hel.ninja 🚀🚀🚀

@azure-pipelines

Copy link
Copy Markdown

APPLICANT is deployed to platta: https://helsinkilisa-ui-pr4188.dev.hel.ninja 🚀🚀🚀

@azure-pipelines

Copy link
Copy Markdown

TestCafe result is success for https://helsinkilisa-ui-pr4188.dev.hel.ninja 😆🎉🎉🎉

@azure-pipelines

Copy link
Copy Markdown

TestCafe result is success for https://helsinkilisa-ui-handler-pr4188.dev.hel.ninja 😆🎉🎉🎉

@azure-pipelines

Copy link
Copy Markdown

YJDH-HELSINKILISA-API branch is deployed to platta: https://helsinkilisa-pr4188.api.dev.hel.ninja 🚀🚀🚀

@azure-pipelines

Copy link
Copy Markdown

APPLICANT is deployed to platta: https://helsinkilisa-ui-pr4188.dev.hel.ninja 🚀🚀🚀

@azure-pipelines

Copy link
Copy Markdown

TestCafe result is success for https://helsinkilisa-ui-pr4188.dev.hel.ninja 😆🎉🎉🎉

Boolean fields did not give error message.
Fixed by adding proper properties.

Refs: HL-1840
@sonarqubecloud

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
frontend/benefit/handler/src/components/applicationForm/useApplicationForm.ts (1)

317-324: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Mark Formik errors before the de minimis early return.

Line 318 exits handleSave() before setTouched() runs. If the de minimis row is unfinished and regular fields are invalid, users only see the de minimis toast and the inline required-field errors stay hidden, which misses the validation goal of this PR.

Proposed fix
   const handleSave = async (): Promise<void> => {
     const errors = await formik.validateForm();
+    if (Object.keys(errors).length > 0) {
+      await formik.setTouched(getTouchedFromErrors(errors), true);
+    }
+
     if (!checkDeMinimisForm()) {
       return;
     }
-
-    if (Object.keys(errors).length > 0) {
-      await formik.setTouched(getTouchedFromErrors(errors), true);
-    }
 
     if (!errorActions(errors)) {
       await formik.submitForm();
     }
   };
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@frontend/benefit/handler/src/components/applicationForm/useApplicationForm.ts`
around lines 317 - 324, In handleSave() inside useApplicationForm, the de
minimis early return is happening before Formik’s validation state is surfaced,
so invalid regular fields can remain hidden. Move the
setTouched(getTouchedFromErrors(errors), true) handling to run before returning
from checkDeMinimisForm(), using the existing validateForm() result and the
touched/error flow in formik so inline errors are shown even when the de minimis
row is incomplete.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In
`@frontend/benefit/handler/src/components/applicationForm/useApplicationForm.ts`:
- Around line 317-324: In handleSave() inside useApplicationForm, the de minimis
early return is happening before Formik’s validation state is surfaced, so
invalid regular fields can remain hidden. Move the
setTouched(getTouchedFromErrors(errors), true) handling to run before returning
from checkDeMinimisForm(), using the existing validateForm() result and the
touched/error flow in formik so inline errors are shown even when the de minimis
row is incomplete.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 57054139-1caf-49e8-beb5-5ac40aebdc23

📥 Commits

Reviewing files that changed from the base of the PR and between 64f96f5 and e195fd2.

📒 Files selected for processing (3)
  • frontend/benefit/handler/src/components/applicationForm/__tests__/useApplicationForm.test.tsx
  • frontend/benefit/handler/src/components/applicationForm/formContent/companySection/CompanySection.tsx
  • frontend/benefit/handler/src/components/applicationForm/useApplicationForm.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/benefit/handler/src/components/applicationForm/formContent/companySection/CompanySection.tsx

@azure-pipelines

Copy link
Copy Markdown

YJDH-HELSINKILISA-API branch is deployed to platta: https://helsinkilisa-pr4188.api.dev.hel.ninja 🚀🚀🚀

@azure-pipelines

Copy link
Copy Markdown

APPLICANT is deployed to platta: https://helsinkilisa-ui-pr4188.dev.hel.ninja 🚀🚀🚀

@azure-pipelines

Copy link
Copy Markdown

TestCafe result is success for https://helsinkilisa-ui-pr4188.dev.hel.ninja 😆🎉🎉🎉

@azure-pipelines

Copy link
Copy Markdown

HANDLER branch is deployed to platta: https://helsinkilisa-ui-handler-pr4188.dev.hel.ninja 🚀🚀🚀

@azure-pipelines

Copy link
Copy Markdown

TestCafe result is success for https://helsinkilisa-ui-handler-pr4188.dev.hel.ninja 😆🎉🎉🎉

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the benefit application form UX by ensuring validation errors are surfaced more reliably (marking invalid fields as touched and setting invalid/aria-invalid states), and adds the missing i18n key for an employee-count label.

Changes:

  • Mark fields as touched when validateForm() returns errors (both for validation-only flow and save flow).
  • Add invalid / aria-invalid wiring for radio selection groups to improve accessibility states.
  • Add Jest coverage for the updated validation/save behavior, and add the missing companyNumberOfEmployees i18n key across locales.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
frontend/benefit/handler/src/components/applicationForm/useApplicationForm.ts Adds helper to derive touched from validation errors and applies it during validation/save.
frontend/benefit/handler/src/components/applicationForm/formContent/companySection/CompanySection.tsx Sets invalid/aria-invalid for specific SelectionGroup fields to reflect validation state.
frontend/benefit/handler/src/components/applicationForm/tests/useApplicationForm.test.tsx Adds tests covering touched-marking and submit behavior for valid/invalid forms.
frontend/benefit/handler/public/locales/sv/common.json Adds companyNumberOfEmployees label key.
frontend/benefit/handler/public/locales/en/common.json Adds companyNumberOfEmployees label key.
frontend/benefit/handler/public/locales/fi/common.json Adds companyNumberOfEmployees label key.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +86 to +104
const getTouchedFromErrors = <TValues,>(
errors: FormikErrors<TValues>
): FormikTouched<TValues> =>
Object.keys(errors).reduce((touched, key) => {
const error = errors[key as keyof FormikErrors<TValues>];

return {
...touched,
[key]: Array.isArray(error)
? error.map((item) =>
item && typeof item === 'object'
? getTouchedFromErrors(item as FormikErrors<unknown>)
: true
)
: error && typeof error === 'object'
? getTouchedFromErrors(error as FormikErrors<unknown>)
: true,
};
}, {} as FormikTouched<TValues>);
Comment on lines +251 to +254
const { result } = renderHook(() => useApplicationForm(), { wrapper });

await act(() => result.current.handleSave() as unknown as Promise<void>);

Comment on lines +270 to +273
const { result } = renderHook(() => useApplicationForm(), { wrapper });

await act(() => result.current.handleSave() as unknown as Promise<void>);

Comment on lines +1885 to +1887
"companyNumberOfEmployees": {
"label": "Työntekijöiden määrä ennen työllistettyä"
},
Comment on lines +1885 to +1887
"companyNumberOfEmployees": {
"label": "Työntekijöiden määrä ennen työllistettyä"
},

@mrTuomoK mrTuomoK left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Check and fix the CoPilot-pointed issues. The translations are fine, if they are just for compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants