[MAGNOLIA-3826] Dialog validation is ignored when a ValidatingSaveHandler is attached to the dialog definition Created: 08/Sep/11  Updated: 10/Jan/12  Resolved: 30/Dec/11

Status: Closed
Project: Magnolia
Component/s: admininterface, templating components
Affects Version/s: 4.4.2
Fix Version/s: 4.4.6, 4.5

Type: Bug Priority: Neutral
Reporter: Rory Gibson Assignee: Milan Divilek
Resolution: Fixed Votes: 1
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Template:
Patch included:
Yes
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Bug DoR:
[ ]* Steps to reproduce, expected, and actual results filled
[ ]* Affected version filled
Date of First Response:

 Description   

We've added a custom ValidatingSaveHandler to the Page Info dialog for our pages.
Adding this has caused the original mandatory fields on the dialog, which were working beforehand, to become optional (despite still being marked with a *).

After some debugging, we've isolated the fault as follows:

The validate() method in DialogMVCHandler contains the following:

boolean passed = this.getDialog().validate();

SaveHandler saveHandler = this.getSaveHandler();
if (saveHandler instanceof ValidatingSaveHandler) {
     passed = ((ValidatingSaveHandler) saveHandler).validate();
}

As you can see, this ignores the results of dialog validation if a ValidatingSaveHandler is present.

We suggest changing this to:

boolean passed = this.getDialog().validate();

SaveHandler saveHandler = this.getSaveHandler();
if (saveHandler instanceof ValidatingSaveHandler) {
     passed = passed && ((ValidatingSaveHandler) saveHandler).validate();
}

We've worked around the problem at the momentby explicitly calling this.getDialog().validate() from our ValidatingSaveHandler, but this isn't really satisfactory.



 Comments   
Comment by Danilo Ghirardelli [ 08/Sep/11 ]

I had the same problem and adopted the same solution. I would probably suggest to change the code like this, to avoid another round of validation if the basic one has already failed:

boolean passed = this.getDialog().validate();

SaveHandler saveHandler = this.getSaveHandler();
if ((passed) && (saveHandler instanceof ValidatingSaveHandler)) {
     passed = ((ValidatingSaveHandler) saveHandler).validate();
}
Comment by Jan Haderka [ 23/Dec/11 ]

Junit test?

Generated at Mon Feb 12 03:50:03 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.