Details
-
Type:
Bug
-
Status:
Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: Release 1.5
-
Fix Version/s: Release 1.5.1, Release 1.6
-
Component/s: Validation
-
Labels:None
Description
In the rare case you have both required and ignore both set to true, ignore doesn't override required like it should.
The fix is simple:
ValidationMetadata.requiredOn is currently:
/** Returns true if the field is required when processing the specified event. */
public boolean requiredOn(String event) {
return this.required && (
(this.on == null) ||
(this.onIsPositive && this.on.contains(event)) ||
(!this.onIsPositive && !this.on.contains(event))
);
}
It should be:
/** Returns true if the field is required when processing the specified event. */
public boolean requiredOn(String event) {
return !this.ignore && this.required && (
(this.on == null) ||
(this.onIsPositive && this.on.contains(event)) ||
(!this.onIsPositive && !this.on.contains(event))
);
}
For reference, see chat log:
plightbo: tfenne: ignore is only ignored for setting things, but it doesn't ignore validation rules that come back from ValidationMetadata
[10:30am] tfenne: plightbo; ValidationMetadata /only/ hold the metadata
[10:31am] tfenne: the rules for interpreting are all in DefaultActionBeanPropertyBinder
[10:31am] plightbo: yup, but that code runs too late
[10:31am] tfenne: how so?
[10:31am] plightbo: validateRequiredFields() gets called first
[10:32am] plightbo: and it ends up calling checkSingleRequiredField() for a field that wasn't submitted and should be ignored
[10:32am] plightbo: but ends up adding an error, complaining the form didn't submit the field
[10:32am] plightbo: which, of course, i purposely didn't submit
[10:32am] tfenne: I can't say we would ever have expected {required=true, ignore=true} which I guess you must be doing?
[10:32am] plightbo: heh, yeah
[10:33am] tfenne: I see
[10:33am] plightbo: reason this happens is:
[10:33am] plightbo: i have this: http://rafb.net/p/weavcM63.html
[10:33am] plightbo: but, the underlying object also has those two fields as @NotNull using Hib Validator
[10:33am] tfenne: I see
[10:33am] plightbo: and then through the merge, you get required = true and ignore = true
[10:34am] tfenne: can you file a bug? seems like an easy fix for 1.5.1
[10:34am] plightbo: i could just make my merge code detect ignore and blank out the rest
[10:34am] tfenne: that'd work too
[10:34am] plightbo: but I think the fix is simple: you just make the requiredOn() call in ValidationMetadata check ignore
Fixed in build 956 (branches/1.5.x) and build 957 (trunk) with accompanying unit test.