Fix blank string validation#491
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR systematically adds Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~18 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@fdm-app/app/components/blocks/fields-new/schema.tsx`:
- Around line 17-19: b_lu_catalogue currently only enforces type but allows
blank/whitespace strings; change its Zod validator to trim input and require
non-empty content (e.g., replace z.string({ error: "Hoofdgewas is verplicht" })
with a trimmed + non-empty check such as using .trim().min(1, { message:
"Hoofdgewas is verplicht" }) or .trim().nonempty({ message: "Hoofdgewas is
verplicht" })) so whitespace-only payloads are rejected; update the validator
for the b_lu_catalogue field in schema.tsx accordingly.
- Around line 12-16: Normalize empty-string inputs for the b_area field by
preprocessing them to undefined before coercion: update the b_area validator
(the z.preprocess / b_area schema that currently uses
z.coerce.number(...).optional()) to first convert "" to undefined and then apply
z.coerce.number({ error: "Oppervlakte van perceel is verplicht" }).optional(),
preserving the existing error message and optional behavior.
In `@fdm-app/app/routes/farm.create._index.tsx`:
- Around line 89-94: The validation for b_businessid_farm fails for
whitespace-only input because Zod's .trim() doesn't apply across union branches;
the .or(z.literal("")) branch sees the original untrimmed value. Update the
b_businessid_farm schema to first normalize the input (trim or transform to ""),
then perform a post-transform check with .refine() that allows an empty string
or enforces the /^\d{8}$/ regex; reference the existing symbol
b_businessid_farm, the .trim()/.or(z.literal("")) union, and replace that logic
with a single-string pipeline that trims/normalizes and then .refine()s the
trimmed value to accept either "" or 8 digits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 282496e2-6c28-4976-a6d0-30b5123e60e1
📒 Files selected for processing (12)
fdm-app/app/components/blocks/auth/auth-formschema.tsxfdm-app/app/components/blocks/cultivation/schema.tsfdm-app/app/components/blocks/fertilizer-applications/formschema.tsxfdm-app/app/components/blocks/fertilizer/formschema.tsxfdm-app/app/components/blocks/field/schema.tsxfdm-app/app/components/blocks/fields-new/schema.tsxfdm-app/app/components/blocks/organization/schema.tsfdm-app/app/lib/schemas/access.schema.tsfdm-app/app/routes/farm.$b_id_farm.$calendar.field.$b_id.overview.tsxfdm-app/app/routes/farm.$b_id_farm.settings.properties.tsxfdm-app/app/routes/farm.create._index.tsxfdm-app/app/routes/welcome.tsx
| b_area: z.coerce | ||
| .number({ | ||
| required_error: "Oppervlakte van perceel is verplicht", | ||
| error: "Oppervlakte van perceel is verplicht", | ||
| }) | ||
| .optional(), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "schema.tsx" | head -20Repository: nmi-agro/fdm
Length of output: 209
🏁 Script executed:
# Get the file size first to understand what we're dealing with
wc -l fdm-app/app/components/blocks/fields-new/schema.tsxRepository: nmi-agro/fdm
Length of output: 110
🏁 Script executed:
# Read the target file around lines 12-16
cat -n fdm-app/app/components/blocks/fields-new/schema.tsx | head -30Repository: nmi-agro/fdm
Length of output: 734
🏁 Script executed:
# Search for similar preprocessing patterns in schema files to confirm the pattern usage
rg "z.preprocess.*z.coerce.number" --type typescriptRepository: nmi-agro/fdm
Length of output: 90
🏁 Script executed:
# Search for z.preprocess patterns more broadly
rg "z.preprocess" fdm-app/app/components/blocks/Repository: nmi-agro/fdm
Length of output: 8696
🏁 Script executed:
# Check the field/schema.tsx to see how optional numeric fields are handled there
cat -n fdm-app/app/components/blocks/field/schema.tsxRepository: nmi-agro/fdm
Length of output: 1316
🏁 Script executed:
# Check harvest/schema.tsx too
cat -n fdm-app/app/components/blocks/harvest/schema.tsx | head -50Repository: nmi-agro/fdm
Length of output: 2111
Add empty-string normalization to b_area field to match the established pattern for optional numeric fields.
The b_area field should preprocess empty input to undefined before numeric coercion, consistent with how other optional numeric fields are handled throughout the codebase (e.g., b_lu_yield in harvest schema). Without this, empty strings may be coerced improperly.
Proposed fix
- b_area: z.coerce
- .number({
- error: "Oppervlakte van perceel is verplicht",
- })
- .optional(),
+ b_area: z.preprocess(
+ (val) => (val === "" ? undefined : val),
+ z.coerce.number({
+ error: "Oppervlakte van perceel is verplicht",
+ }).optional(),
+ ),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| b_area: z.coerce | |
| .number({ | |
| required_error: "Oppervlakte van perceel is verplicht", | |
| error: "Oppervlakte van perceel is verplicht", | |
| }) | |
| .optional(), | |
| b_area: z.preprocess( | |
| (val) => (val === "" ? undefined : val), | |
| z.coerce.number({ | |
| error: "Oppervlakte van perceel is verplicht", | |
| }).optional(), | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fdm-app/app/components/blocks/fields-new/schema.tsx` around lines 12 - 16,
Normalize empty-string inputs for the b_area field by preprocessing them to
undefined before coercion: update the b_area validator (the z.preprocess /
b_area schema that currently uses z.coerce.number(...).optional()) to first
convert "" to undefined and then apply z.coerce.number({ error: "Oppervlakte van
perceel is verplicht" }).optional(), preserving the existing error message and
optional behavior.
| b_lu_catalogue: z.string({ | ||
| required_error: "Hoofdgewas is verplicht", | ||
| error: "Hoofdgewas is verplicht", | ||
| }), |
There was a problem hiding this comment.
b_lu_catalogue is marked required but still allows blank/whitespace payloads.
Line 17–19 currently only enforces string type. Add trim + non-empty validation to fully align with this PR’s blank-string objective.
Proposed fix
- b_lu_catalogue: z.string({
- error: "Hoofdgewas is verplicht",
- }),
+ b_lu_catalogue: z
+ .string({
+ error: "Hoofdgewas is verplicht",
+ })
+ .trim()
+ .min(1, "Hoofdgewas is verplicht"),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| b_lu_catalogue: z.string({ | |
| required_error: "Hoofdgewas is verplicht", | |
| error: "Hoofdgewas is verplicht", | |
| }), | |
| b_lu_catalogue: z | |
| .string({ | |
| error: "Hoofdgewas is verplicht", | |
| }) | |
| .trim() | |
| .min(1, "Hoofdgewas is verplicht"), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fdm-app/app/components/blocks/fields-new/schema.tsx` around lines 17 - 19,
b_lu_catalogue currently only enforces type but allows blank/whitespace strings;
change its Zod validator to trim input and require non-empty content (e.g.,
replace z.string({ error: "Hoofdgewas is verplicht" }) with a trimmed +
non-empty check such as using .trim().min(1, { message: "Hoofdgewas is
verplicht" }) or .trim().nonempty({ message: "Hoofdgewas is verplicht" })) so
whitespace-only payloads are rejected; update the validator for the
b_lu_catalogue field in schema.tsx accordingly.
| b_businessid_farm: z | ||
| .string() | ||
| .trim() | ||
| .regex(/^\d{8}$/, "KvK-nummer moet uit 8 cijfers bestaan") | ||
| .optional() | ||
| .or(z.literal("")), |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Zod v4 z.string().trim().regex().optional().or(z.literal("")) transformation behavior union
💡 Result:
In Zod v4:
z.string().trim()is a string transformation (the parsed output is trimmed). (zodjs.netlify.app)- A union made via
.or(...)/z.union([...])is evaluated option-by-option in order, and Zod returns the first option that successfully parses. (odocs-zod.vercel.app)
So with:
z.string().trim().regex(R).optional().or(z.literal(""))What actually happens
-
This is a union with (roughly) these branches:
- Branch A:
z.string().trim().regex(R).optional()(i.e., “string matching R” ORundefined) - Branch B:
z.literal("")
- Branch A:
-
Transforms/refinements do not “carry over” between union branches. Each branch gets the original input.
- If the input is
"": Branch A fails the regex, Branch B succeeds → output is""(notundefined). - If the input is
" ": Branch A trims to""then fails regex; Branch B checks the original" "(not"") and fails → overall fails (this surprises people).
- If the input is
This explains why “allow empty string” via .or(z.literal("")) makes validation pass, but you then store empty strings in the DB. (stackoverflow.com)
Common fixes (depending on desired output)
A) Treat "" as “missing” (undefined)
const emptyToUndefined = z.literal("").transform(() => undefined);
const s = z.string().trim().regex(R).optional().or(emptyToUndefined);(So "" parses to undefined.) (gist.github.com)
B) Make trimming happen before the union (so " " becomes "" and can match the empty-string branch)
Use a top-level preprocess/transform that trims first, then union. (Conceptually: “normalize input, then validate”.) (zodjs.netlify.app)
Citations:
- 1: https://zodjs.netlify.app/guide/strings?utm_source=openai
- 2: https://odocs-zod.vercel.app/?utm_source=openai
- 3: https://stackoverflow.com/questions/76294208/validate-optional-or-nullish-method?utm_source=openai
- 4: https://gist.github.com/bennettdams/463c804fcfde0eaa888eaa4851c668a1?utm_source=openai
- 5: https://zodjs.netlify.app/guide/strings?utm_source=openai
Fix validation logic for whitespace-only KvK input.
In Zod v4, transformations like .trim() do not carry over between union branches. Each branch in .or(z.literal("")) receives the original, untrimmed input. For input like " ", the string branch trims to "" and fails the regex, but the z.literal("") branch checks the original " " and also fails—causing validation to reject whitespace-only strings.
Use .refine() to apply validation after trimming has occurred:
Suggested fix
- b_businessid_farm: z
- .string()
- .trim()
- .regex(/^\d{8}$/, "KvK-nummer moet uit 8 cijfers bestaan")
- .optional()
- .or(z.literal("")),
+ b_businessid_farm: z
+ .string()
+ .trim()
+ .refine(
+ (value) => value === "" || /^\d{8}$/.test(value),
+ "KvK-nummer moet uit 8 cijfers bestaan",
+ )
+ .optional(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| b_businessid_farm: z | |
| .string() | |
| .trim() | |
| .regex(/^\d{8}$/, "KvK-nummer moet uit 8 cijfers bestaan") | |
| .optional() | |
| .or(z.literal("")), | |
| b_businessid_farm: z | |
| .string() | |
| .trim() | |
| .refine( | |
| (value) => value === "" || /^\d{8}$/.test(value), | |
| "KvK-nummer moet uit 8 cijfers bestaan", | |
| ) | |
| .optional(), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@fdm-app/app/routes/farm.create._index.tsx` around lines 89 - 94, The
validation for b_businessid_farm fails for whitespace-only input because Zod's
.trim() doesn't apply across union branches; the .or(z.literal("")) branch sees
the original untrimmed value. Update the b_businessid_farm schema to first
normalize the input (trim or transform to ""), then perform a post-transform
check with .refine() that allows an empty string or enforces the /^\d{8}$/
regex; reference the existing symbol b_businessid_farm, the
.trim()/.or(z.literal("")) union, and replace that logic with a single-string
pipeline that trims/normalizes and then .refine()s the trimmed value to accept
either "" or 8 digits.
2 participants
Fixes
extractFormValuesFromRequestthus failed min length and similar checks. The form that the user is seeing wouldn't show any errors because the string still had length to it, but once the string is trimmed server-side, the same check would fail there.Summary by CodeRabbit
Release Notes
Bug Fixes