Alona Kaplan has posted comments on this change. Change subject: webadmin: Extract external subnet model & widget ......................................................................
Patch Set 1: (4 comments) http://gerrit.ovirt.org/#/c/25358/1//COMMIT_MSG Commit Message: Line 5: CommitDate: 2014-03-04 10:44:53 +0200 Line 6: Line 7: webadmin: Extract external subnet model & widget Line 8: Line 9: Extracted the representatyion & functionality of external subnet to a typo Line 10: model & widget in order to facilitate code reuse and DRY. Line 11: Line 12: Change-Id: I2d9c34a1ceb8c92e8ff813301186a2f5c9f499b4 Line 13: Related-To: https://bugzilla.redhat.com/1064749 http://gerrit.ovirt.org/#/c/25358/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/NetworkModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/NetworkModel.java: Line 445: getExternalProviders().validateSelectedItem(new IValidation[] { new NotEmptyValidation() }); Line 446: Line 447: boolean subnetValid = true; Line 448: if ((Boolean) getExport().getEntity() Line 449: && getSubnetModel().getName().getEntity() != null 1. You can use- StringHelper.isNullOrEmpty() I know StringHelper is deprecated. If it bothers you, move the method to StringUtils. 2. Why isn't the name null or empty validation part of the subnetModel.validate? Why don't you validate the other fields if there is no name? Line 450: && !getSubnetModel().getName().getEntity().isEmpty()) { Line 451: subnetValid = getSubnetModel().validate(); Line 452: } Line 453: http://gerrit.ovirt.org/#/c/25358/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/NewNetworkModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/NewNetworkModel.java: Line 179: Line 180: Frontend.getInstance().runMultipleAction(VdcActionType.AttachNetworkToVdsGroup, actionParameters1); Line 181: Line 182: if ((Boolean) getExport().getEntity() Line 183: && getSubnetModel().getName().getEntity() != null The name is not mandatory. As I understood from you, the subnet should be added if there is only CIDR and IP version. Line 184: && !getSubnetModel().getName().getEntity().isEmpty()) { Line 185: getSubnetModel().setExternalNetwork(getNetwork().getProvidedBy()); Line 186: getSubnetModel().flush(); Line 187: http://gerrit.ovirt.org/#/c/25358/1/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/ExternalSubnetModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/providers/ExternalSubnetModel.java: Line 55: public ExternalSubnet getSubnet() { Line 56: return subnet; Line 57: } Line 58: Line 59: public void setSubnet(ExternalSubnet subnet) { Why do you need this method? Line 60: this.subnet = subnet; Line 61: } Line 62: Line 63: public ProviderNetwork getExternalNetwork() { -- To view, visit http://gerrit.ovirt.org/25358 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2d9c34a1ceb8c92e8ff813301186a2f5c9f499b4 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Mike Kolesnik <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Lior Vernia <[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
