fix: trim whitespace-only input in settings definition forms

[Valyrian-Code]

Fixes #16427

Proposed Changes

The required free-text fields in three facility-settings "definition" forms use z.string().min(1), which accepts a value made up entirely of spaces. A whitespace-only value (e.g. " ") passes validation, letting a definition be saved with an effectively blank title/description/usage.

This continues the same fix as the recent Charge Item Definition title trim, and matches the repo convention already used elsewhere (e.g. FacilityForm, ValueSetForm, slug_value in these same schemas, which all .trim()).

Forms fixed (required text fields only — selects/enums/slugs untouched):

• ObservationDefinitionForm — title, description
• SpecimenDefinitionForm — title, description
• ActivityDefinitionForm — title, description, usage

// before
title: z.string().min(1, t("field_required")),
// after
title: z.string().trim().min(1, t("field_required")),

• 7 one-line schema changes across the 3 forms.
• Adds a Playwright regression test to each form's existing spec: fill the title with only spaces, submit, and assert the required-field error appears. Verified fail-before / pass-after (on min(1) the whitespace title is accepted; with .trim() it is rejected).

Reproduce

1. Facility → Settings → (Observation / Specimen / Activity) Definitions → create new.
2. Enter only spaces in a required text field (e.g. Title) and submit.
3. Before: validation passes for that field. After: "Field is required" is shown.

Tagging: @ohcnetwork/care-fe-code-reviewers

Merge Checklist

• Add specs that demonstrate the bug or test the new feature. (Playwright regression test per form, verified fail-then-pass.)
• Update product documentation. (N/A — validation fix, no doc change.)
• Ensure that UI text is placed in I18n files. (Reuses the existing field_required key; no new strings.)
• Prepare a screenshot or demo video for the changelog entry. (N/A — input-validation fix; repro steps above.)
• Request peer reviews.
• Complete QA on mobile devices. (Covered by automated E2E; manual device QA not performed.)
• Complete QA on desktop devices. (Covered by automated E2E; manual device QA not performed.)
• Add or update Playwright tests for related changes.

Summary by CodeRabbit

• Bug Fixes
  • Required text fields now trim leading/trailing whitespace before validation, so whitespace-only Titles, Descriptions, slug values, and other required text inputs are correctly rejected across activity, observation, and specimen definition forms.
  • Slug inputs now have trimming applied before enforcing min/max length checks.
• Tests
  • Added automated tests and a shared test helper to assert whitespace-only inputs are rejected and display validation errors.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6846dc9d-04ce-4042-99bf-d6244cb05782

📥 Commits

Reviewing files that changed from the base of the PR and between 7ea5430 and 4bd22a3.

📒 Files selected for processing (1)

• src/pages/Facility/settings/specimen-definitions/SpecimenDefinitionForm.tsx

💤 Files with no reviewable changes (1)

• src/pages/Facility/settings/specimen-definitions/SpecimenDefinitionForm.tsx

---

Walkthrough

Three facility definition forms (Activity, Observation, Specimen) now trim whitespace from required string fields using Zod's .trim() before applying minimum-length validation. Playwright tests and a shared helper verify that whitespace-only input is rejected with visible validation errors.

Changes

Whitespace input validation for facility definition forms

Layer / File(s)	Summary
Activity definition whitespace trimming src/pages/Facility/settings/activityDefinition/ActivityDefinitionForm.tsx, tests/facility/settings/activityDefinition/activityDefinitionCreate.spec.ts	Form schema for title, description, usage, and slug_value now calls .trim() before enforcing minimum/length validation. Test verifies that a whitespace-only Title/Description/Usage submission shows visible validation errors.
Observation definition whitespace trimming src/pages/Facility/settings/observationDefinition/ObservationDefinitionForm.tsx, tests/facility/settings/observationDefinition/observationDefinition.spec.ts	Form schema for title, description, and slug_value now calls .trim() before required/length validation. Test import and new test case verify whitespace-only Title and Description submissions show validation errors.
Specimen definition whitespace trimming src/pages/Facility/settings/specimen-definitions/SpecimenDefinitionForm.tsx, tests/facility/settings/specimenDefinitions/specimenDefinitionCreate.spec.ts	Form schema for title, slug_value, and description now calls .trim() before enforcing existing validations. Test verifies whitespace-only Title and Description submissions show validation errors.
Shared test helper tests/helper/error.ts	Adds expectWhitespaceRejected(fields, submit) which fills provided fields with whitespace, runs the submit callback, and asserts each field's form-message is visible and contains the expected error.

• Possibly related PRs:

  • ohcnetwork/care_fe#16404: Similar change applying .trim() to Zod form validations and adjusting Playwright tests to reject whitespace-only titles.

• Suggested labels:
  needs testing, needs review

• Suggested reviewers:

  • rithviknishad
  • nihal467

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title clearly and accurately summarizes the main change: adding .trim() to validation schemas in three definition forms to reject whitespace-only input.
Description check	✅ Passed	The PR description covers the core issue, shows before/after code examples, lists affected forms/fields, and includes Playwright test details. The author properly filled the template with detailed context and rationale.
Linked Issues check	✅ Passed	The code changes fully address #16427: .trim() was applied to all required text fields in the three forms as specified, with corresponding Playwright regression tests for each form.
Out of Scope Changes check	✅ Passed	All changes are scope-appropriate: schema trim edits target only required text fields, helper/test additions support the validation fix, and out-of-scope optional URL trimming was explicitly reverted.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds stricter validation to prevent whitespace-only values in facility settings forms, and introduces end-to-end coverage to ensure whitespace-only titles are rejected.

Changes:

• Trim required text fields in Zod schemas so whitespace-only input fails .min(1) validation.
• Add Playwright tests asserting whitespace-only titles are rejected across multiple definition forms.
• Add missing test import for getFieldErrorMessage where needed.

Reviewed changes

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

Show a summary per file

File	Description
tests/facility/settings/specimenDefinitions/specimenDefinitionCreate.spec.ts	Adds an E2E test that verifies whitespace-only titles are rejected.
tests/facility/settings/observationDefinition/observationDefinition.spec.ts	Adds getFieldErrorMessage import and an E2E test for whitespace-only title rejection.
tests/facility/settings/activityDefinition/activityDefinitionCreate.spec.ts	Adds an E2E test that verifies whitespace-only titles are rejected.
src/pages/Facility/settings/specimen-definitions/SpecimenDefinitionForm.tsx	Trims title and description before required validation.
src/pages/Facility/settings/observationDefinition/ObservationDefinitionForm.tsx	Trims title and description before required validation.
src/pages/Facility/settings/activityDefinition/ActivityDefinitionForm.tsx	Trims title, description, and usage before required validation.

[greptile-apps]

Greptile Summary

This PR fixes a validation gap where required text fields in three facility-settings definition forms accepted whitespace-only strings by adding .trim() before .min(1) in the Zod schemas, matching the convention already used in FacilityForm, ValueSetForm, and the slug fields in these same schemas.

• Schema fix (3 files): title, description, and usage fields in ActivityDefinitionForm; title and description in ObservationDefinitionForm and SpecimenDefinitionForm all gain .trim(), which both rejects blank-whitespace submissions and silently trims leading/trailing whitespace from valid input before the API call.
• Tests (3 files): Each form gains one Playwright regression test that fills the title with spaces, submits, and asserts the required-field error is shown. The tests are well-placed and correctly import getFieldErrorMessage.

Confidence Score: 4/5

Safe to merge; the schema changes are minimal, targeted, and consistent with existing codebase patterns.

The core fix is correct and well-scoped. The only gap is that the new Playwright tests only exercise the title field for whitespace rejection, leaving description and usage (which are also fixed by the schema change) without dedicated regression coverage.

The three test files could benefit from additional whitespace assertions on description and usage; the source files are clean.

Important Files Changed

Filename	Overview
src/pages/Facility/settings/activityDefinition/ActivityDefinitionForm.tsx	Adds .trim() before .min(1) on title, description, and usage — correctly rejects whitespace-only input and trims valid input before the API call.
src/pages/Facility/settings/observationDefinition/ObservationDefinitionForm.tsx	Adds .trim() before .min(1) on title and description — consistent with the same pattern applied in the other two forms.
src/pages/Facility/settings/specimen-definitions/SpecimenDefinitionForm.tsx	Adds .trim() before .min(1) on title and description — mirrors the fix applied to the other definition forms.
tests/facility/settings/activityDefinition/activityDefinitionCreate.spec.ts	Adds a whitespace-only title regression test; description and usage whitespace cases are not covered.
tests/facility/settings/observationDefinition/observationDefinition.spec.ts	Adds import for getFieldErrorMessage and a whitespace-only title regression test inside the existing beforeEach-navigated describe block; placement is correct.
tests/facility/settings/specimenDefinitions/specimenDefinitionCreate.spec.ts	Adds a whitespace-only title regression test; description whitespace is not exercised.

_{Reviews (1): Last reviewed commit: "fix: trim whitespace-only input in setti..." | Re-trigger Greptile}

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

[Valyrian-Code]

Addressed both issues:

• The helper now asserts the expected error text, with an overridable matcher when a different assertion is needed.

• For the locators, I initially tried using a uniform ^Title\b regex, but in the specimen form it matched two fields because of the nested container.description field. Instead, I now target the specific required-field label used by each form.

• The observation form intentionally renders without an asterisk, which is consistent with its existing tests.

[parvathyns-creator]

@nihal467 this looks fine

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

[nihal467]

link the issue to the PR

[github-actions]

🚀 Preview Deployment Ready!

🔗 Preview URL: https://pr-16413.care-preview-a7w.pages.dev

📱 Mobile Access:
Scan the QR code below to open the preview on your mobile device.

---

This preview will be automatically updated when you push new commits to this PR.

[github-actions]

🎭 Playwright Test Results

Status: ✅ Passed
Test Shards: 3

Metric	Count
Total Tests	323
✅ Passed	323
❌ Failed	0
⏭️ Skipped	0

📊 Detailed results are available in the playwright-final-report artifact.

_{Run: #9396}

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

[Valyrian-Code]

Linked — created #16427 and added Fixes #16427 to the description. Thanks @nihal467!

[Valyrian-Code]

Thanks for the review. On the Copilot comments:

• derived_from_uri trim (both comments): reverted it (4bd22a3) back to z.string().url().optional(). It's an optional URL field, and as noted, trimming makes whitespace-only collapse to "" which then fails .url() on an optional field — a separate concern from this PR's required-text whitespace handling. Keeping the PR focused on the required fields (title/description/usage and the slugs).
• expectWhitespaceRejected default /required/i: left as-is. It's a test-only helper and the suite runs against the English locale, where t("field_required") = "This field is required" matches /required/i reliably; the matcher is already overridable for any caller that needs a different message, so a default avoids repeating it everywhere. If the wording ever changes, the test failing is the intended signal.

Also linked the tracking issue (#16427) per @nihal467's request.