Created attachment 752004 [details] attached Screenshot exception.png Description of problem: Install ovirt-node-upstream, enter into "Security" page and resetting admin password with the same value in both "Password" and "Confirm Password" items. But Exception was shown after clicking "Save" button.(Seen exception.png) Version-Release number of selected component (if applicable): ovirt-node-iso-3.0.0-1.0.20130517.fc18.iso Steps to Reproduce: 1. Install ovirt-node-upstream, configure network with "static". 2. enter Security page, then resetting admin password with the same value. Actual results: Exception was shown after clicking "Save" button. Expected results: Resetting admin password with the same value should be successful. Additional info: ----------
Hey, I had two ideas related to this problem: 1. Introduce an InvalidChange exception which can be used during the on_change method, which gives the user control about what shall happen to the data. E.g. don't drop the invalid data, but keep it. 2. Use and fix NodePlugin.pending_changes(). Previously only invalid changes which were discoverred during the validation step were kept as invalid changes, now also invalid changes discovered during the on_change run are kept. That means they can be retrieved using NodePlugin.pending_changes(include_invalid=True). The following patch uses the second (2) approach to solve this bug. As it uses existing infrastructure and does not include another way to solve this. But we can discuss in general if (1) or (2) makes more sense. http://gerrit.ovirt.org/#/c/15079/
Ryan, Mike, any comments or hints on what I might have missed?
I don't think you missed anything, but I wanted to present an alternative. I've got a SameAs validator mostly working (up this afternoon) that should hide the ugly invalid change bits from the presentation layer and potentially from contributors who write plugins.
Please review: http://gerrit.ovirt.org/#/c/15079/ I introduced a ValidatedEntry widget as a Container which creates children which must be valid. For ease in the presentation layer, the fact that it's instantiating multiple widgets is hidden from plugins. Unfortunately, the current implementation requires adding an attribute to an existing class, as well as tweaking the flow of validity in plugin pages to check whether or not every attribute is valid before lighting up the save button. A change of that magnitude isn't strictly necessary, as it's certainly possible to have the plugin check for some bogus change value as invalid even if it's not in the list of validators. I didn't want to go the route of magic numbers, and it seemed like triggering widget validity after it passes the validators was logical. Feedback welcome.
This change has been merged.