fix: enforce System presence validation for ChooseMultiple fields #55
Conversation
- Test verifies validation element should be created for System fields - Currently needs refinement to properly interact with Elm app - Will verify fix manually for now
- Add effectiveMin logic in validation element generation - When presence is System and minRequired is Nothing, treat as Just 1 - Creates validation element with min="1" for System fields - Aligns frontend validation with backend Go validation expectations - Fixes bug where System + ChooseMultiple fields allowed 0 selections
- Modified isChooseManyUsingMinMax to also check for System presence - System presence fields now get event handlers to track selections - Validation element value updates correctly when checkboxes are clicked - Added working E2E tests that verify the complete fix - Tests pass in Chrome, Firefox, and WebKit
Ensures generated tests/GoElmCrossValidationTestData.elm is properly formatted after generation, preventing formatting changes in git diff
✅ Deploy Preview for tiny-form-fields ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
| let | ||
| -- System presence implies minRequired = 1 if not explicitly set | ||
| effectiveMin = | ||
| case ( formField.presence, minRequired ) of | ||
| ( System, Nothing ) -> | ||
| Just 1 | ||
|
|
||
| _ -> | ||
| minRequired | ||
|
|
||
| -- Need validation if we have constraints | ||
| needsValidation = | ||
| effectiveMin /= Nothing || maxAllowed /= Nothing | ||
| in | ||
| -- Only apply validation in CollectData mode, not in Editor mode | ||
| if not disabledMode && (minRequired /= Nothing || maxAllowed /= Nothing) then | ||
| if not disabledMode && needsValidation then | ||
| [ input | ||
| [ type_ "number" | ||
| , required True | ||
| , attribute "value" (String.fromInt selectedCount) -- raw value for browser only | ||
| , attribute "min" (Maybe.map String.fromInt minRequired |> Maybe.withDefault "") | ||
| , attribute "min" (Maybe.map String.fromInt effectiveMin |> Maybe.withDefault "") | ||
| , attribute "max" (Maybe.map String.fromInt maxAllowed |> Maybe.withDefault "") | ||
| , attribute "class" "tff-visually-hidden" | ||
| ] |
There was a problem hiding this comment.
When presence == System and minRequired == Nothing, we treat it as minRequired == Just 1. This aligns frontend validation with backend expectations (Go's isRequired() returns true for System presence).
Without this, the validation element wouldn't be created at all, allowing invalid form submission that would fail backend validation.
| -- Need event handlers if: | ||
| -- 1. Has explicit min/max constraints, OR | ||
| -- 2. System presence (implies min=1 requirement) | ||
| minRequired /= Nothing || maxAllowed /= Nothing || formField.presence == System |
There was a problem hiding this comment.
Added || formField.presence == System to ensure event handlers are attached for System fields. This allows the validation element's value attribute to be updated as checkboxes are clicked.
Without event handlers, the validation element would always show value="0" even when checkboxes are selected, because Elm wouldn't track the selection changes. The hidden <input type="number" min="1" value="0"> would pass browser validation incorrectly.
| @echo "Generating cross-validation test data..." | ||
| node scripts/generate-cross-validation-tests.js | ||
| @echo "Formatting generated Elm test data..." | ||
| npx elm-format tests/GoElmCrossValidationTestData.elm --yes |
There was a problem hiding this comment.
Added automatic formatting after test generation to keep git history clean.
The Node.js generator doesn't produce properly formatted Elm code. Without this, every make test-json-compatibility would create formatting-only diffs.
Add PLAYWRIGHT_ARGS variable to control Playwright reporter output: - Local dev: defaults to --reporter=line for fast output - CI: override with PLAYWRIGHT_ARGS="" to use HTML reporter from config Update GitHub Actions workflow: - Upload playwright-report only on test failure - Add if-no-files-found: ignore to suppress warning when tests pass - Pass PLAYWRIGHT_ARGS="" to enable HTML reporter for debugging failures Fixes "No files were found with the provided path: playwright-report/" warning
This PR adds runtime validation for ChooseMultiple fields with System presence, completing the fix started in PR #54. It implements "Layer 2" defense (runtime validation) to complement PR #54's "Layer 1" (editor prevention).
Builds on: PR #54 & related to #4
Problem: System + ChooseMultiple fields with no explicit
minRequiredallowed 0 selections in CollectData mode, causing backend validation to fail despite appearing valid in the UI.Solution:
min="1"whenpresence == SystemandminRequired == NothingCommits
Setup & Formatting
Core Implementation
Documentation & Tooling