Lior Vernia has posted comments on this change.

Change subject: webadmin: call&add name validation when creating/editing 
Provider URL for Network Provider
......................................................................


Patch Set 4:

(2 comments)

http://gerrit.ovirt.org/#/c/31330/4/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/ProviderModel.java
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/ProviderModel.java:

Line 225:         getName().validateEntity(new IValidation[] { new 
NotEmptyValidation(), new AsciiNameValidation() });
Line 226:         getType().validateSelectedItem(new IValidation[] { new 
NotEmptyValidation() });
Line 227:         getNeutronAgentModel().validate();
Line 228: 
Line 229:         return validateConnectionSettings() &&
I would prefer for this to be called in a separate row, with the rest, and 
saved in a variable.

This will currently work okay, but only because it's the first among the 
conditions checked in this composite && statement.

Otherwise, it might not have been marked as invalid just because another field 
would have been marked as invalid first.

So, having functionality depend on ordering within a composite && statement 
looks not great to me.
Line 230:                 getName().getIsValid() &&
Line 231:                 getType().getIsValid() &&
Line 232:                 getNeutronAgentModel().getIsValid();
Line 233:     }


Line 314: 
Line 315:     private void onTest() {
Line 316:         if (!validateConnectionSettings()) {
Line 317:             return;
Line 318:         }
All changes under this line seem to not be needed?...
Line 319: 
Line 320:         flush();
Line 321:         startProgress(null);
Line 322:         
Frontend.getInstance().runAction(VdcActionType.TestProviderConnectivity,


-- 
To view, visit http://gerrit.ovirt.org/31330
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I0b9e9a6b66d800c973334629fa34b3b5443290df
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <mmu...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to