Bug 1123620

Summary: [RFE] dialog validations: automatically mark a side-section as invalid if it contains at least one invalid field
Product: [oVirt] ovirt-engine Reporter: Lior Vernia <lvernia>
Component: RFEsAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED WONTFIX QA Contact: Pavel Stehlik <pstehlik>
Severity: medium Docs Contact:
Priority: unspecified    
Version: ---CC: bazulay, bugs, ecohen, gklein, iheim, lsurette, mgoldboi, rbalakri, yeylon
Target Milestone: ---Keywords: CodeChange, FutureFeature
Target Release: ---Flags: ylavi: ovirt-future?
ylavi: planning_ack?
ylavi: devel_ack?
ylavi: testing_ack?
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: ux
Fixed In Version: Doc Type: Enhancement
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-11-18 23:16:32 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Lior Vernia 2014-07-27 10:21:37 UTC
Description of problem:

In composite dialogs which comprise multiple left tabs, it is customary to mark the tab(s) as invalid where and GUI widgets had failed validation. This is currently done in a manual fashion, leading to sizeable boiler-plate code wherever this logic is implemented.

This can and should be automated, as the logic is quite simple - if any widget under the hierarchy of a tab is invalid then mark the tab is invalid - and the required information (which widgets belong to which tab) can be deduced from the DOM layout of the dialog.



Outline of possible solution:

Currently our validation scheme works as such: it is triggered on the model side, as part of the dialog approval flow. Each member of the model is [in]validated, which potentially triggers an event on the view side to mark a widget as [in]valid. This can only happen if the widget implements HasValidation, and the vast majority of our widgets implement that via inheritance from AbstractValidatedWidget.

To build on this scheme (rather than change flows), I suggest to modify the treatment on the view side. As part of AbstractValidatedWidget.markAs[In]Valid(), we may also traverse up the DOM element parent tree and search for tabs - or more generally, for widgets that would implement an interface such as HasCompositeValidation (which could extend HasValidation).

DialogTab would be the first widget to implement this interface, which would expose at the very least a method markDescendentAsValid(boolean valid, Widget descendent). DialogTab would keep a HashSet that would remember its descendents that had been marked as invalid, whenever markDescendentAsValid() is called it would update this set. The tab itself will be rendered valid if and only if the set is empty.

While conceptually non-trivial, this solution should take little coding and would work well with our current validation scheme. A missing detail is how to obtain widgets back from DOM elements - Google does suggest a couple of fishy solutions, but at worst we could keep a map of this ourselves.

* A different alternative would be to change our validation scheme to be triggered automatically as part of the dialog lifecycle methods rather than manually from the model side, and then traverse the DOM tree from root to leaf. Traversal in post-order will enable [in]validating tabs (as an example) without having to keep a set to "remember" which widgets had been marked as invalid. This would save even more boiler-plate code (with respect to triggering validation on the model side) but will require wider changes.

Comment 1 Yaniv Lavi 2015-04-07 07:06:39 UTC
Is this a dup of BZ #1057386 ?

Comment 2 Lior Vernia 2015-04-07 08:07:23 UTC
No.

Comment 3 Einav Cohen 2015-04-07 15:16:14 UTC
(In reply to Lior Vernia from comment #2)
> No.

correct :)

Comment 5 Einav Cohen 2015-11-18 23:16:32 UTC
this is a code-change (no impact on user - only developer) not worth investing in at this point IMO -> closing.