Lior Vernia has posted comments on this change.
Change subject: webadmin: Add subnet left tab to new network dialog
......................................................................
Patch Set 1:
(6 comments)
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/NetworkModel.java
Line 445:
Line 446: getExternalProviders().validateSelectedItem(new IValidation[]
{ new NotEmptyValidation() });
Line 447:
Line 448: getSubnetName().validateEntity(new IValidation[] { new
AsciiNameValidation() });
Line 449: getSubnetCidr().setIsValid(true);
True is the default, no need to set it.
Line 450: getSubnetIpVersion().setIsValid(true);
Line 451: if (getSubnetName().getEntity() != null &&
!getSubnetName().getEntity().isEmpty()) {
Line 452: getSubnetCidr().validateEntity(new IValidation[] { new
NotEmptyValidation() });
Line 453: getSubnetIpVersion().validateSelectedItem(new
IValidation[] { new NotEmptyValidation() });
Line 451: if (getSubnetName().getEntity() != null &&
!getSubnetName().getEntity().isEmpty()) {
Line 452: getSubnetCidr().validateEntity(new IValidation[] { new
NotEmptyValidation() });
Line 453: getSubnetIpVersion().validateSelectedItem(new
IValidation[] { new NotEmptyValidation() });
Line 454: }
Line 455:
Need for double spacing?
Line 456:
Line 457: boolean profilesValid = true;
Line 458: Iterable<VnicProfileModel> profiles =
getProfiles().getItems();
Line 459: for (VnicProfileModel profileModel : profiles) {
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/NewNetworkModel.java
Line 182: }
Line 183:
Line 184:
Frontend.getInstance().runMultipleAction(VdcActionType.AttachNetworkToVdsGroup,
actionParameters1);
Line 185:
Line 186: if ((Boolean) getExport().getEntity() &&
!getSubnetName().getEntity().isEmpty()) {
Unless you initiate subnetName to be an empty string (which you can), you
should check for null as well.
Line 187: ExternalSubnet subnet = new ExternalSubnet();
Line 188: subnet.setName(getSubnetName().getEntity());
Line 189: subnet.setCidr(getSubnetCidr().getEntity());
Line 190:
subnet.setIpVersion(getSubnetIpVersion().getSelectedItem());
Line 183:
Line 184:
Frontend.getInstance().runMultipleAction(VdcActionType.AttachNetworkToVdsGroup,
actionParameters1);
Line 185:
Line 186: if ((Boolean) getExport().getEntity() &&
!getSubnetName().getEntity().isEmpty()) {
Line 187: ExternalSubnet subnet = new ExternalSubnet();
I would initiate subnet in the constructor to not have to worry about it here.
Line 188: subnet.setName(getSubnetName().getEntity());
Line 189: subnet.setCidr(getSubnetCidr().getEntity());
Line 190:
subnet.setIpVersion(getSubnetIpVersion().getSelectedItem());
Line 191:
Line 190:
subnet.setIpVersion(getSubnetIpVersion().getSelectedItem());
Line 191:
Line 192: actionParameters1 = new
ArrayList<VdcActionParametersBase>();
Line 193: actionParameters1.add(new
AddExternalSubnetParameters(subnet, networkId));
Line 194:
Frontend.getInstance().runMultipleAction(VdcActionType.AddSubnetToProvider,
actionParameters1);
There's no need to use runMultipleAction() here when you'll necessarily only
create one subnet; you could use runAction() instead.
Line 195: }
Line 196: }
Line 197:
Line 198: public ArrayList<NetworkClusterModel> getClustersToAttach()
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/datacenter/EditNetworkPopupView.java
Line 45: public void updateVisibility() {
Line 46: super.updateVisibility();
Line 47: attachPanel.setVisible(false);
Line 48: clusterTab.setVisible(false);
Line 49: toggleSubnetVisibility(false);
I don't think this does what you wanted to accomplish; if I'm not mistaken,
this is only called once (so unchecking and checking "export" would cause the
tab to show). At the moment this is okay because we haven't allowed changing
the "export" checkbox when editing a network, but in general you could override
toggleSubnetVisibility() and just set it to false.
Line 50: }
Line 51:
Line 52: @Override
Line 53: public EditNetworkModel flush() {
--
To view, visit http://gerrit.ovirt.org/22690
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5054c208f692a321d60532046ffa576770c4ec6b
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Mike Kolesnik <mkole...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
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
