Skip to content

fix: export rules added after DateInput's format rule - #131

Merged
dakorpar merged 1 commit into
masterfrom
fix/61-date-input-conditions
Aug 14, 2026
Merged

fix: export rules added after DateInput's format rule#131
dakorpar merged 1 commit into
masterfrom
fix/61-date-input-conditions

Conversation

@dakorpar

Copy link
Copy Markdown
Member

Closes #61.

The bug

addCondition() / addConditionOn() on DateInput and DateTimeInput never reached data-nette-rules, so client-side validation ignored them. Only setRequired() worked.

DateInput::__construct() registered its format check as a closure:

$this->addRule(fn (Control $input) => DateTimeFormat::validate($this->format, $input->getValue()), $this->invalidFormatMessage);

Helpers::exportRules() breaks — not continues — on the first rule whose canExport() is false, and Rule::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 because Rules::addRule() puts the required rule in a separate $required slot 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-rules as Contributte\FormsBootstrap\Inputs\DateInput::validateFormat. netteForms.js has no validator under that name: validateRule() returns null for unknown validators and validateControl() continues past a null, 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-rules attribute to every date input for a rule no client-side validator implements. The alternative — dropping the Nette rule and calling addError() from validate() — 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

Test Guards Verified by
testConditionsAreExported the issue itself — conditions reach data-nette-rules fails against pre-fix source
testValidationWithIncorrectFormat server-side rejection survives the swap fails when validateFormat is stubbed to return true
testEmptyOptionalValueDoesNotFailValidation an empty optional date doesn't trip the format rule passes

The 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-rules attribute.

make qa (PHPStan level 7 + codesniffer) and the full suite pass.

🤖 Generated with Claude Code

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

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.46%. Comparing base (87fa3b3) to head (e93fd13).
⚠️ Report is 1 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dakorpar
dakorpar merged commit 8c64550 into master Aug 14, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DateInput ignore addCondition and addConditionOn

1 participant