Hotfix/20250603#153
Conversation
…r succesfull submission
🦋 Changeset detectedLatest commit: a452ac8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis update includes patch releases across multiple packages, correcting a typo in a fertilizer property name ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FertilizerForm
participant SoilForm
participant FieldForm
participant Types
User->>FertilizerForm: Submit fertilizer application
FertilizerForm->>FertilizerForm: Validate & process input (handle empty/NaN)
FertilizerForm->>FertilizerForm: Reset form on success
User->>SoilForm: Submit soil analysis
SoilForm->>SoilForm: Validate fields (improved error messages, handle empty strings)
SoilForm->>SoilForm: Enforce required fields
User->>FieldForm: Open/add field dialog
FieldForm->>FieldForm: Sync form state with props, reset on close
FertilizerForm->>Types: Use updated property name p_cl_rt
SoilForm->>Types: Use updated property name p_cl_rt
FieldForm->>Types: Use updated property name p_cl_rt
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (8)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #153 +/- ##
=======================================
Coverage 93.07% 93.07%
=======================================
Files 68 68
Lines 8061 8061
Branches 957 957
=======================================
Hits 7503 7503
Misses 556 556
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.changeset/tired-cloths-kneel.md (1)
5-5: Minor grammar improvement suggested.The changeset description is clear, but could benefit from improved grammar for better readability.
Consider this minor grammar improvement:
-Fix exception when clicking on field to add a new field +Fix exception when clicking on a field to add a new field🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: You might be missing the article “a” here.
Context: ...tch --- Fix exception when clicking on field to add a new field(AI_EN_LECTOR_MISSING_DETERMINER_A)
.changeset/khaki-snails-doubt.md (1)
5-5: Grammar improvement and clarity enhancement suggested.The changeset description could benefit from better grammar and more specific details about the change.
Consider this improvement for clarity and grammar:
-Remove the incorrect limit value for nitrogen at fertilizer application cards +Remove the incorrect limit value for nitrogen on fertilizer application cardsAlternatively, for even better clarity, consider:
-Remove the incorrect limit value for nitrogen at fertilizer application cards +Remove the incorrect nitrogen limit value from fertilizer application cards🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: The preposition “on” seems more likely in this position.
Context: ... the incorrect limit value for nitrogen at fertilizer application cards(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
.changeset/many-rats-check.md (1)
5-5: Fix grammatical and spelling errors in changeset description.The changeset description contains grammatical and spelling errors that should be corrected for better documentation quality.
Apply this diff to fix the errors:
-Clear value for p_app_amount at fertilizer application form after succesfull submission +Clear value for p_app_amount on fertilizer application form after successful submission🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: The preposition “on” seems more likely in this position.
Context: ...patch --- Clear value for p_app_amount at fertilizer application form after succe...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.changeset/khaki-snails-doubt.md(1 hunks).changeset/many-rats-check.md(1 hunks).changeset/rotten-camels-rest.md(1 hunks).changeset/tired-cloths-kneel.md(1 hunks)fdm-app/app/components/custom/fertilizer-applications/cards.tsx(1 hunks)fdm-app/app/components/custom/fertilizer-applications/form.tsx(2 hunks)fdm-app/app/components/custom/field/form.tsx(3 hunks)fdm-data/src/fertilizers/d.ts(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/khaki-snails-doubt.md
[uncategorized] ~5-~5: The preposition “on” seems more likely in this position.
Context: ... the incorrect limit value for nitrogen at fertilizer application cards
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
.changeset/tired-cloths-kneel.md
[uncategorized] ~5-~5: You might be missing the article “a” here.
Context: ...tch --- Fix exception when clicking on field to add a new field
(AI_EN_LECTOR_MISSING_DETERMINER_A)
.changeset/many-rats-check.md
[uncategorized] ~5-~5: The preposition “on” seems more likely in this position.
Context: ...patch --- Clear value for p_app_amount at fertilizer application form after succe...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION)
🔇 Additional comments (8)
.changeset/rotten-camels-rest.md (1)
1-6: Well-documented changeset.The changeset clearly describes the typo fix with specific details about the affected type and property name. This provides good traceability for the breaking change.
fdm-app/app/components/custom/fertilizer-applications/cards.tsx (1)
104-104: Correct implementation of nitrogen limit removal.The change to set
limit: undefinedproperly removes the incorrect nitrogen limit value. The component's conditional rendering logic (line 36) correctly handles this by hiding the limit-related UI elements when the limit is undefined.fdm-app/app/components/custom/fertilizer-applications/form.tsx (2)
50-52: LGTM! Code style improvement.Good addition of semicolons for consistency and proper formatting of the dependency array.
93-101: LGTM! Robust numeric input handling.The updated input handling logic properly manages edge cases:
- Displays empty string for undefined/null/NaN values instead of just zero
- Converts empty input to undefined rather than leaving invalid state
- Uses parseFloat for proper numeric conversion
This ensures the form state accurately reflects the input state and handles edge cases gracefully.
fdm-app/app/components/custom/field/form.tsx (4)
51-51: LGTM! Simplified initialization.Setting the default value directly from the computed
b_lu_catalogueeliminates the need for separate state synchronization.
60-63: LGTM! Improved form reset logic.Adding form reset when the dialog closes (
!open) prevents stale data from persisting when the dialog is reopened.
115-115: LGTM! Centralized state management.Passing the entire
formobject to the Combobox component centralizes state management within the form hook, eliminating the need for separate local state handling.
65-71:Details
❓ Verification inconclusive
Consider UX implications of props-based form reset.
While this ensures the form reflects updated props, resetting form values whenever
fieldNameDefaultorb_lu_cataloguechanges could disrupt user input if these props change while the user is actively editing the form.Consider whether these props can change during active form editing, and if so, whether a more selective reset strategy might be better (e.g., only reset when dialog opens).
🏁 Script executed:
#!/bin/bash # Description: Check if fieldNameDefault or b_lu_catalogue can change during active form editing # Look for parent components that might update these props dynamically # Search for components that render FieldDetailsDialog ast-grep --pattern 'FieldDetailsDialog' # Look for dynamic prop updates rg -A 5 -B 5 'fieldNameDefault|b_lu_catalogue' --type tsx --type tsLength of output: 455
🏁 Script executed:
#!/bin/bash # Search for definitions and updates of `fieldNameDefault` and `b_lu_catalogue` in the parent route rg -n 'fieldNameDefault' fdm-app/app/routes/farm.*.tsx rg -n 'b_lu_catalogue' fdm-app/app/routes/farm.*.tsx # Inspect the context around the FieldDetailsDialog invocation to see how props are passed sed -n '300,360p' fdm-app/app/routes/farm.$b_id_farm.$calendar.field.new.tsxLength of output: 23212
Verify props-driven form reset timing
The
useEffectin fdm-app/app/components/custom/field/form.tsx resets the form any timefieldNameDefaultorb_lu_cataloguechanges:useEffect(() => { form.reset({ b_name: fieldNameDefault, b_lu_catalogue, }); }, [fieldNameDefault, b_lu_catalogue, form.reset]);• These props originate from the loader in
fdm-app/app/routes/.../field.new.tsxand, under typical Remix routing, don’t update while the dialog remains open.
• Please confirm that neitherloaderData.fieldNameDefaultnorloaderData.b_lu_cataloguecan change during an active form session.If there’s a chance they could change mid-edit, consider guarding the reset so it only runs when the dialog first opens, for example:
useEffect(() => { - form.reset({ b_name: fieldNameDefault, b_lu_catalogue }); + if (isOpen) { + form.reset({ b_name: fieldNameDefault, b_lu_catalogue }); + } }, [isOpen, fieldNameDefault, b_lu_catalogue, form.reset]);–
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/violet-readers-share.md (1)
5-5: Refine change description grammar.Consider adding the definite article for clarity. For example:
- “Fix the typo in the parameter name
p_cl_rtin theFertilizertype”🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ...venvw/fdm-core": patch --- Fix typo in parameter namep_cl_rtin the type Fertilizer(AI_EN_LECTOR_MISSING_DETERMINER_THE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.changeset/khaki-snails-doubt.md(1 hunks).changeset/many-rats-check.md(1 hunks).changeset/tired-cloths-kneel.md(1 hunks).changeset/violet-readers-share.md(1 hunks)fdm-core/src/fertilizer.d.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- fdm-core/src/fertilizer.d.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- .changeset/khaki-snails-doubt.md
- .changeset/many-rats-check.md
- .changeset/tired-cloths-kneel.md
🧰 Additional context used
🪛 LanguageTool
.changeset/violet-readers-share.md
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ...venvw/fdm-core": patch --- Fix typo in parameter name p_cl_rt in the type Fertilizer
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/clear-planets-read.md(1 hunks)fdm-app/app/components/custom/soil/formschema.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/clear-planets-read.md
🔇 Additional comments (5)
fdm-app/app/components/custom/soil/formschema.tsx (5)
5-12: LGTM! String validation improvements look good.The updates to the
a_sourcefield validation are well-implemented:
- Clear, user-friendly error messages in Dutch
- Updated refine logic to reject
"nl-other-nmi"instead of"nmi"- Proper case-insensitive validation using
toLowerCase()
13-16: Good addition of explicit error messages for date field.The
b_sampling_datefield now has clear, descriptive error messages for both invalid type and required field scenarios.
34-305: Excellent systematic improvement to numeric field validation.The consistent use of
z.preprocessacross all numeric fields provides several benefits:
- Empty strings are properly converted to
undefinedfor optional fields- Clear error messages improve user experience
- Consistent validation ranges with appropriate min/max constraints
- Proper handling of optional fields
310-325: Good cross-field validation logic.The depth validation ensures logical consistency between upper and lower depth values, with appropriate error messaging and path specification.
309-309:Details
✅ Verification successful
Verify the impact of making fields required by default.
Commenting out
.partial()means all fields in the schema are now required unless explicitly marked as.optional(). This could be a breaking change if the form previously allowed partial submissions.Verify that this change aligns with the intended form behavior:
🏁 Script executed:
#!/bin/bash # Description: Check if there are any form components or tests that might be affected by making fields required # Expected: Find usage patterns that might need updating # Search for references to this FormSchema rg -A 5 -B 5 "FormSchema" --type tsx --type ts # Look for form submission or validation logic that might assume partial data rg -A 10 "partial|optional" --type tsx --type ts | grep -i "soil\|form" # Check for any existing tests that might fail with required fields fd -e test.ts -e test.tsx -e spec.ts -e spec.tsx --exec rg -l "soil.*form\|FormSchema" {}Length of output: 270
🏁 Script executed:
#!/bin/bash # Improved search for FormSchema usages and optional fields in TS/TSX # 1. Find all references to FormSchema in .ts and .tsx files rg -g '*.ts' -g '*.tsx' -n -C5 "FormSchema" # 2. Look for any .optional() calls that might indicate fields marked optional rg -g '*.ts' -g '*.tsx' -n -C5 "\.optional\(\)" # 3. Scan test/spec files for schema references or soil form usage rg -g '*.test.ts' -g '*.test.tsx' -g '*.spec.ts' -g '*.spec.tsx' -n "FormSchema|soil"Length of output: 72670
🏁 Script executed:
#!/bin/bash # Inspect the soil FormSchema for .partial() and optional fields # 1. Show the beginning of the soil formschema to see its shape head -n 50 fdm-app/app/components/custom/soil/formschema.tsx # 2. Search for any commented or active .partial() call rg -n "partial" fdm-app/app/components/custom/soil/formschema.tsx # 3. Check which fields are marked optional (if any) rg -n "\.optional" fdm-app/app/components/custom/soil/formschema.tsx # 4. Locate the a_id definition to see if it's required rg -n "a_id" -n 1 fdm-app/app/components/custom/soil/formschema.tsxLength of output: 3530
🏁 Script executed:
#!/bin/bash # Inspect soil form default values and UI registrations # 1. Find defaultValues assignment in form component rg -n "defaultValues" -C5 fdm-app/app/components/custom/soil/form.tsx # 2. Locate registration of a_source field rg -n "register.*a_source" -C5 fdm-app/app/components/custom/soil/form.tsx # 3. Locate registration of b_sampling_date field rg -n "register.*b_sampling_date" -C5 fdm-app/app/components/custom/soil/form.tsxLength of output: 1187
Confirmed removal of
.partial()is safe and intentional.I verified that the soil FormSchema now makes only these fields required (all others explicitly marked with
.optional()):
- a_source
- b_sampling_date
- a_depth_lower
Every other parameter in the schema is already
.optional(), matching the defaultValues logic in the form (which supplies values orundefined). No tests or form consumers assume a truly “partial” schema, and all routes importing this schema handle missing values via.optional()or default fallbacks.No follow-up changes are needed here.
1 participant
Summary by CodeRabbit
Bug Fixes
Chores