Bug 987949

Summary: [oVirt] [provider] It's seemingly possible to add a provider with the same name
Product: [Retired] oVirt Reporter: Mike Kolesnik <mkolesni>
Component: ovirt-engine-webadminAssignee: Lior Vernia <lvernia>
Status: CLOSED CURRENTRELEASE QA Contact:
Severity: low Docs Contact:
Priority: medium    
Version: 3.3CC: acathrow, bazulay, ecohen, iheim, masayag, mgoldboi, mkolesni
Target Milestone: m1   
Target Release: 3.3.2   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard: network
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 1021905 (view as bug list) Environment:
Last Closed: 2013-12-19 14:24:38 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:
Bug Depends On:    
Bug Blocks: 1021905    

Description Mike Kolesnik 2013-07-24 12:56:58 UTC
Description of problem:
Can do action is not propagated to UI


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


How reproducible:
Always


Steps to Reproduce:
1. Go to external providers
2. Add some provider with name "test"
3. Add another provider with name "test"
  
Actual results:
There should be an error shown to the user that the name is unique


Expected results:
There is no error shown, the dialog closes


Additional info:
In log you can see:
2013-07-24 15:52:09,261 WARN  [org.ovirt.engine.core.bll.provider.AddProviderCommand] (ajp--127.0.0.1-8702-1) CanDoAction of action AddProvider failed. Reasons:VAR__ACTION__ADD,VAR__TYPE__PROVIDER,ACTION_TYPE_FAILED_NAME_ALREADY_USED

Comment 1 Lior Vernia 2013-10-21 06:28:51 UTC
Einav, is this something that should be fixed? As far as I know, all frontend validation nowadays only checks that the input is technically valid, not if names are already taken.

Comment 2 Einav Cohen 2013-10-21 16:26:49 UTC
(In reply to Lior Vernia from comment #1)
> Einav, is this something that should be fixed? As far as I know, all
> frontend validation nowadays only checks that the input is technically
> valid, not if names are already taken.

indeed - client validations are currently not checking name uniqueness - we are relying solely on server validation to do that.

the problem described in this bug is not that there is no client validation; the problem is that the server validation (namely, CanDoAction) results are not propagated properly to the UI (namely, no dialog with the CanDoAction details pops up, as should happen with any CanDoAction failure).

we had a bug that prevented any CanDoAction dialog from appearing after engine restart [http://gerrit.ovirt.org/#/c/18729/, Bug 968362], but it has been fix.
[the fix was merged (Sep 4) after this bug has been reported (Jul 24)].

@Mike - can you please test to see if the problem is still reproducible on latest master? If so, it means that the problem is not related to Bug 968362 and further investigation is required in order to find out why the CanDoAction failure dialog is not popping up as it should.

Thanks.

Comment 3 Lior Vernia 2013-10-22 06:42:23 UTC
Yes, upon more careful reading I see you are perfectly correct. I checked, and while there's an error popping up now, it does pop up only after the dialog closes, so that should still be fixed.

Comment 4 Einav Cohen 2013-10-22 14:00:06 UTC
(In reply to Lior Vernia from comment #3)
> Yes, upon more careful reading I see you are perfectly correct. I checked,
> and while there's an error popping up now, it does pop up only after the
> dialog closes, so that should still be fixed.

please note that fixing it might not be very easy: we have similar requests open already (see Bug 956562, for example), however note what I wrote in Bug 956562, Comment #2: While it makes sense to keep the dialog open for some CanDoAction failures (e.g. Name uniqueness), it probably makes sense to close the dialog for other CanDoAction failures. 
This will require us to 'mark' each CanDoAction failure as 'keep GUI dialog open upon this failure' = true/false, or something similar.

Comment 5 Lior Vernia 2013-10-24 20:19:58 UTC
Thanks for the thorough explanation Einav, I definitely see how it would be useful to have a way to classify errors to those two types. Thankfully, in this case the dialog is quite simple and it's an easy call to keep it open in case of failure.

Comment 6 Sandro Bonazzola 2013-12-19 14:24:38 UTC
oVirt 3.3.2 has been released resolving the problem described in this bug report.