fix: export rules added after DateInput's format rule - #131
Merged
Conversation
DateInput registered its format check as a closure. Nette's Helpers::exportRules() breaks out of the loop on the first rule whose canExport() is false, and Rule::canExport() accepts only strings and static callables — never closures. The format rule is added in the constructor, so it sat in front of everything else and every addRule()/addCondition()/addConditionOn() registered afterwards was silently dropped from data-nette-rules. setRequired() kept working because Rules::addRule() stores the required rule in a separate slot instead of the ordered list, so it never sat behind the closure. Turning the check into a public static method makes the rule exportable and stops it truncating the rest. Server-side validation is unchanged. Client-side the rule now appears in data-nette-rules under a name netteForms.js has no validator for; validateRule() returns null for unknown validators and validateControl() skips it, so the browser moves on to the user's conditions. Closes #61 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #131 +/- ##
============================================
+ Coverage 97.05% 97.46% +0.40%
Complexity 309 309
============================================
Files 25 25
Lines 986 987 +1
============================================
+ Hits 957 962 +5
+ Misses 29 25 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #61.
The bug
addCondition()/addConditionOn()onDateInputandDateTimeInputnever reacheddata-nette-rules, so client-side validation ignored them. OnlysetRequired()worked.DateInput::__construct()registered its format check as a closure:Helpers::exportRules()breaks — notcontinues — on the first rule whosecanExport()is false, andRule::canExport()accepts only strings and static callables, never closures. Since the format rule is added in the constructor it sits in front of everything, so every rule and condition registered afterwards was silently dropped from the exported payload.setRequired()was unaffected becauseRules::addRule()puts the required rule in a separate$requiredslot rather than the ordered list, so it never sat behind the closure — which is exactly the symptom reported in the issue.The fix
The format check becomes a public static method, making the rule exportable so it no longer truncates what follows.
Server-side validation is unchanged — same format, same value, same message.
Client-side, the rule now appears in
data-nette-rulesasContributte\FormsBootstrap\Inputs\DateInput::validateFormat.netteForms.jshas no validator under that name:validateRule()returnsnullfor unknown validators andvalidateControl()continues past anull, so the browser skips it and goes on to evaluate the user's conditions.Trade-off worth a reviewer's eye
This adds a
data-nette-rulesattribute to every date input for a rule no client-side validator implements. The alternative — dropping the Nette rule and callingaddError()fromvalidate()— keeps the markup clean but means re-implementing Nette's empty-optional skipping and condition-branch nesting by hand. I went with the smaller change; happy to switch if you'd rather not emit the attribute.Tests
testConditionsAreExporteddata-nette-rulestestValidationWithIncorrectFormatvalidateFormatis stubbed toreturn truetestEmptyOptionalValueDoesNotFailValidationThe middle one closed a real gap: the two pre-existing validation tests only feed valid dates and assert
getValue(), so a broken validator would not have turned the suite red. Both new guards were mutation-checked rather than just observed green.Four existing assertions were updated for the now-present
data-nette-rulesattribute.make qa(PHPStan level 7 + codesniffer) and the full suite pass.🤖 Generated with Claude Code