Bug 1029934

Summary: No error message displayed when trying to add an already existing (but unattached) SD in a DC
Product: Red Hat Enterprise Virtualization Manager Reporter: Luca Villa <luvilla>
Component: ovirt-engine-webadmin-portalAssignee: Daniel Erez <derez>
Status: CLOSED ERRATA QA Contact: Carlos Mestre González <cmestreg>
Severity: medium Docs Contact:
Priority: medium    
Version: 3.2.0CC: acanan, amureini, ecohen, gchakkar, iheim, pablo.iranzo, rbalakri, Rhev-m-bugs, scohen, tnisan, yeylon
Target Milestone: ---Keywords: Reopened
Target Release: 3.5.0Flags: scohen: needinfo+
amureini: Triaged+
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: storage
Fixed In Version: ovirt-3.5.0_rc1.1 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-02-11 17:56:02 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Storage RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1058022    
Bug Blocks: 1142923, 1156165    

Description Luca Villa 2013-11-13 14:51:03 UTC
Description of problem:

The RHEV GUI behaviour described below can be misleading to the user and should probably be improved/fixed to try improving the user experience.

When you select a new DC in the left pane and you add the first SD and then the second one before the first is activated, it fails with an error message because the master SD is inactive.

If you then try again (after the master was activated) to add the second SD the UI fails silently (apart from the frame of the name box becoming yellow). The reason is that during the first attempt the new SD was actually created but not attached to the DC. But to figure it out the user should change context and explore the available SDs after selecting "System" in the left pane.

Version-Release number of selected component (if applicable):

rhevm-webadmin-portal-3.2.2-0.41.el6ev.noarch

How reproducible:
Always

Steps to Reproduce:
1. add SD "Data0" as first SD on default DC.
2. try adding a second storage "Data1" to the DC immediately while "Data0" is still inactive. 
3. RHEV refuses to add "Data1" because the master storage ("Data0") was inactive at the time. 
4. activate "Data0"
5. Try adding Data1 again.

Actual results:
Data1 is not added with no evident reason and without any error message being displayed to the user apart from the SD name box frame becoming yellow which is far from explanatory.

Expected results:
An error message should point the user to the fact that he's trying to add an already existent SD, or maybe ask him if he want to attach the existing SD instead.

Additional info:

Comment 2 Daniel Erez 2013-11-14 16:56:03 UTC
The yellow border is an indication for invalid input and should be accompanied with a tool-tip for error explanation (E.g. "Name must be unique.").
In case of an identical name validation, few more dialogs behave the same (such as New/Edit VM). Other dialogs skip this validation and show an error message returned from the server.

@Einav - UX-wise, do you think we should remove the client validation and rely on canDo messages?

Comment 3 Einav Cohen 2013-11-14 19:04:00 UTC
(In reply to Daniel Erez from comment #2)
> The yellow border is an indication for invalid input and should be
> accompanied with a tool-tip for error explanation (E.g. "Name must be
> unique.").
> In case of an identical name validation, few more dialogs behave the same
> (such as New/Edit VM). Other dialogs skip this validation and show an error
> message returned from the server.
> 
> @Einav - UX-wise, do you think we should remove the client validation and
> rely on canDo messages?

hi Derez, actually - no: I think that the "client" validation [1] is important in this case; I think that situations in which some validations on the 'Name' field (or any other field, for that matter) are done on the client (e.g. not empty / max length / legal characters), which result in the tool-tip-accompanied-yellow-bordered field upon failure, and some other validations (e.g. Name uniqueness) are done on the server, which result in a CanDoAction pop-up, possibly closing the entire dialog, etc. upon failure, is graphically/behaviorally inconsistent, hence confusing and bad ux in general. 

[1] it is not really a *client* validation, of course - we are calling the "isXXXNameUnique" backend query in order to determine that; 
we are referring here to "client validations" as checks (which may or may not involve calling to backend queries) performed before actually invoking the relevant backend action, which typically result in tool-tip-accompanied-yellow-bordered field upon failure.

Comment 4 Daniel Erez 2013-11-14 20:56:59 UTC
(In reply to Einav Cohen from comment #3)
> (In reply to Daniel Erez from comment #2)
> > The yellow border is an indication for invalid input and should be
> > accompanied with a tool-tip for error explanation (E.g. "Name must be
> > unique.").
> > In case of an identical name validation, few more dialogs behave the same
> > (such as New/Edit VM). Other dialogs skip this validation and show an error
> > message returned from the server.
> > 
> > @Einav - UX-wise, do you think we should remove the client validation and
> > rely on canDo messages?
> 
> hi Derez, actually - no: I think that the "client" validation [1] is
> important in this case; I think that situations in which some validations on
> the 'Name' field (or any other field, for that matter) are done on the
> client (e.g. not empty / max length / legal characters), which result in the
> tool-tip-accompanied-yellow-bordered field upon failure, and some other
> validations (e.g. Name uniqueness) are done on the server, which result in a
> CanDoAction pop-up, possibly closing the entire dialog, etc. upon failure,
> is graphically/behaviorally inconsistent, hence confusing and bad ux in
> general. 
> 
> [1] it is not really a *client* validation, of course - we are calling the
> "isXXXNameUnique" backend query in order to determine that; 
> we are referring here to "client validations" as checks (which may or may
> not involve calling to backend queries) performed before actually invoking
> the relevant backend action, which typically result in
> tool-tip-accompanied-yellow-bordered field upon failure.

So, IIUC, in your view, this is not a bug?

Comment 5 Einav Cohen 2013-11-14 21:22:05 UTC
(In reply to Daniel Erez from comment #4)
> (In reply to Einav Cohen from comment #3)
> > (In reply to Daniel Erez from comment #2)
> > > The yellow border is an indication for invalid input and should be
> > > accompanied with a tool-tip for error explanation (E.g. "Name must be
> > > unique.").
> > > In case of an identical name validation, few more dialogs behave the same
> > > (such as New/Edit VM). Other dialogs skip this validation and show an error
> > > message returned from the server.
> > > 
> > > @Einav - UX-wise, do you think we should remove the client validation and
> > > rely on canDo messages?
> > 
> > hi Derez, actually - no: I think that the "client" validation [1] is
> > important in this case; I think that situations in which some validations on
> > the 'Name' field (or any other field, for that matter) are done on the
> > client (e.g. not empty / max length / legal characters), which result in the
> > tool-tip-accompanied-yellow-bordered field upon failure, and some other
> > validations (e.g. Name uniqueness) are done on the server, which result in a
> > CanDoAction pop-up, possibly closing the entire dialog, etc. upon failure,
> > is graphically/behaviorally inconsistent, hence confusing and bad ux in
> > general. 
> > 
> > [1] it is not really a *client* validation, of course - we are calling the
> > "isXXXNameUnique" backend query in order to determine that; 
> > we are referring here to "client validations" as checks (which may or may
> > not involve calling to backend queries) performed before actually invoking
> > the relevant backend action, which typically result in
> > tool-tip-accompanied-yellow-bordered field upon failure.
> 
> So, IIUC, in your view, this is not a bug?

indeed, feel free to close this bug.

BTW, @Luca: I am not sure what your concern was: the fact that the notification was not clear enough, or the fact that the "name is not unique" notification was not graphically identical to the "master domain is inactive" notification.

- We have a general RFE for improving the client validation notifications [bug 877237] - so once implemented, the "Name must be unique." notification should be much clearer. 

- It is OK for both of these notifications to be different, since they occur due to fairly different circumstances: The "Name must be unique" failure is something that the user can easily "fix", therefore we display the client validation notification and keeping the dialog open. The "master domain is inactive" notification is not something related to "illegal"/"invalid" user-defined input, rather a system state on which the user has no control over.

Comment 6 Daniel Erez 2013-11-17 07:31:44 UTC
As discussed above, the current behavior is by design. The validation notifications infrastructure will be improved as part of bug 877237.

*** This bug has been marked as a duplicate of bug 877237 ***

Comment 9 Ayal Baron 2014-01-29 13:54:47 UTC
We're actively working on getting rid of the master domain which would make this entire flow not relevant.

*** This bug has been marked as a duplicate of bug 757291 ***

Comment 10 Luca Villa 2014-01-30 16:28:21 UTC
(In reply to Ayal Baron from comment #9)
> We're actively working on getting rid of the master domain which would make
> this entire flow not relevant.
> 
> *** This bug has been marked as a duplicate of bug 757291 ***

Ayal, while I agree in principle with your motivation I have to disagree because of what it implies to the customer.
We have closed this which is about a bug affecting the current product in favour of a RFE which is currently not targeted for any specific release.
I see only 2 possible ways here: either we fix this in the current product or  we provide the customer with a roadmap of when this could be fixed.
Any thought?
Thanks, Luca

Comment 15 Carlos Mestre González 2014-08-12 09:50:38 UTC
Please, next time provide what the solution is suppose to be so it can be discussed in advance in QA (in this case about the appropiated message).

1. There's a storage domain not attached with name domain_name
2. When trying to add the same storage domain with the same same
  -> the name field is highlitted since the name is duplicated

3. When trying to add the same storage domain with different name:
  -> Message like this is shown:
"Create operation failed. Domain domain_name already exists in the system."

For the 3rd point this doesn't seem a clear message, since is complaining about the name, not the actual storage (address:path).

The message should be something in the lines of the rest api response ([Cannot add Storage Connection. Storage connection already exists]), so something like:

"Create operation failed. Storage connection address:path already exists for domain domain_name"

Comment 16 Allon Mureinik 2014-08-12 10:42:34 UTC
The error message that should return is "ACTION_TYPE_FAILED_STORAGE_CONNECTION_BELONGS_TO_SEVERAL_STORAGE_DOMAINS=Cannot ${action} ${type}. Storage connection parameters are used by the following storage domains : ${domainNames}."

Comment 17 Daniel Erez 2014-08-17 09:55:26 UTC
As requested, clarified the error message in the webadmin:
"Create operation failed. Storage connection is already used by the following storage domain: {storage_name}."

Comment 18 Carlos Mestre González 2014-10-22 15:17:30 UTC
verified in vt5.

Comment 20 errata-xmlrpc 2015-02-11 17:56:02 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://rhn.redhat.com/errata/RHSA-2015-0158.html