This service will be undergoing maintenance at 00:00 UTC, 2017-10-23 It is expected to last about 30 minutes
Bug 966354 - Exception was shown when resetting admin password with the same value.
Exception was shown when resetting admin password with the same value.
Status: CLOSED NEXTRELEASE
Product: oVirt
Classification: Community
Component: ovirt-node (Show other bugs)
3.3
Unspecified Unspecified
urgent Severity urgent
: ---
: 3.3
Assigned To: Ryan Barry
node
:
Depends On:
Blocks: ovirt-node-3.0
  Show dependency treegraph
 
Reported: 2013-05-23 02:45 EDT by haiyang,dong
Modified: 2016-04-26 09:32 EDT (History)
14 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-08-13 09:57:33 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
attached Screenshot exception.png (12.40 KB, image/png)
2013-05-23 02:45 EDT, haiyang,dong
no flags Details


External Trackers
Tracker ID Priority Status Summary Last Updated
oVirt gerrit 15079 None None None Never

  None (edit)
Description haiyang,dong 2013-05-23 02:45:36 EDT
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:
----------
Comment 1 Fabian Deutsch 2013-05-27 05:07:31 EDT
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/
Comment 2 Fabian Deutsch 2013-05-30 06:26:07 EDT
Ryan, Mike,

any comments or hints on what I might have missed?
Comment 3 Ryan Barry 2013-05-30 10:05:34 EDT
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.
Comment 4 Ryan Barry 2013-06-07 17:58:00 EDT
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.
Comment 5 Ryan Barry 2013-08-06 11:36:32 EDT
This change has been merged.

Note You need to log in before you can comment on or make changes to this bug.