Lior Vernia has posted comments on this change. Change subject: webadmin: Add managemenet network field to new/edit cluster dialog ......................................................................
Patch Set 34: (4 comments) http://gerrit.ovirt.org/#/c/37141/34/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterListModel.java: Line 731: Line 732: final ManagementNetworkOnClusterOperationParameters clusterOperationParameters; Line 733: final VdcActionType actionType; Line 734: final Network managementNetwork = model.getManagementNetwork().getSelectedItem(); Line 735: final Guid managementNetworkId = managementNetwork == null ? null : managementNetwork.getId(); I'd expect managementNetwork == null to be impossible once you add the validation in ClusterModel. Line 736: if (model.getIsNew()) { Line 737: actionType = VdcActionType.AddVdsGroup; Line 738: clusterOperationParameters = new ManagementNetworkOnClusterOperationParameters(cluster, managementNetworkId); Line 739: } else { Line 740: actionType = VdcActionType.UpdateVdsGroup; Line 741: if (model.isClusterDetached()) { Line 742: clusterOperationParameters = new ManagementNetworkOnClusterOperationParameters(cluster, managementNetworkId); Line 743: } else { Line 744: clusterOperationParameters = new ManagementNetworkOnClusterOperationParameters(cluster); I don't know how you chose to implement the backend command, but maybe it'd be worthwhile to implement it so that if it can always be passed the ID, and only do something (update network if it's detached, fail otherwise) if the ID is the same as its previous management network ID? Will make this block much more elegant, you'll be able to keep the previous form (of always sending the same parameters and just changing the VdcActionType). Line 745: } Line 746: } Line 747: Frontend.getInstance().runAction( Line 748: actionType, http://gerrit.ovirt.org/#/c/37141/34/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/clusters/ClusterModel.java: Line 1874: public boolean validate(boolean validateCpu) Line 1875: { Line 1876: return validate(true, validateCpu, true); Line 1877: } Line 1878: How about adding a non-empty validation to getManagementNetworks().validateSelectedEntity()? Now that the detached cluster scenario is taken care of, there's no reason this should ever remain empty. Line 1879: public boolean validate(boolean validateStoragePool, boolean validateCpu, boolean validateCustomProperties) Line 1880: { Line 1881: getName().validateEntity(new IValidation[] { Line 1882: new NotEmptyValidation(), Line 1950: && getRngHwrngSourceRequired().getIsValid() Line 1951: && getGlusterHostPassword().getIsValid() Line 1952: && (getIsImportGlusterConfiguration().getEntity() ? (getGlusterHostAddress().getIsValid() Line 1953: && getGlusterHostPassword().getIsValid() Line 1954: && getSerialNumberPolicy().getCustomSerialNumber().getIsValid() Make sure to also add && getManagementNetwork().getIsValid() somewhere here. Line 1955: && isFingerprintVerified()) : true); Line 1956: setValidTab(TabName.GENERAL_TAB, generalTabValid); Line 1957: ValidationCompleteEvent.fire(getEventBus(), this); Line 1958: return generalTabValid && getCustomPropertySheet().getIsValid() && getSpiceProxy().getIsValid(); -- To view, visit http://gerrit.ovirt.org/37141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I055babd6037f127235c349499f1545396e38333f Gerrit-PatchSet: 34 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yevgeny Zaspitsky <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Yevgeny Zaspitsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
