Alona Kaplan has posted comments on this change.

Change subject: webadmin: Add network QoS to Vnic Profile UI
......................................................................


Patch Set 11:

(4 comments)

....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/EditNetworkModel.java
Line 88:                             getSelectedDc().getcompatibility_version(),
Line 89:                             profileView, null, false);
Line 90: 
Line 91:                     if (profileView.getNetworkQosName() != null && 
!profileView.getNetworkQosName().equals("")) { //$NON-NLS-1$
Line 92:                         NetworkQoS networkQoS = new NetworkQoS();
The qos list and selected item are initialized in the VnicProfileModel's ctor. 
Why do you need extra initializtion?
Line 93:                         
networkQoS.setName(profileView.getNetworkQosName());
Line 94:                         
editModel.getNetworkQoS().setSelectedItem(networkQoS);
Line 95:                     }
Line 96:                     editModel.getNetworkQoS().setIsChangable(false);


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/datacenters/NewNetworkModel.java
Line 106:         List<VnicProfileModel> profiles = new 
LinkedList<VnicProfileModel>();
Line 107: 
Line 108:         NewVnicProfileModel newModel =
Line 109:                 new NewVnicProfileModel(getSourceListModel(), 
getSelectedDc().getcompatibility_version(), false);
Line 110:         newModel.initNetworkQoSList(getSelectedDc().getId(), null);
Same here. The initialization should be done in NewVnicProfileModel.
Line 111:         profiles.add(newModel);
Line 112:         getProfiles().setItems(profiles);
Line 113:     }
Line 114: 


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/profiles/VnicProfileListModel.java
Line 71: 
Line 72:         initNetworkList(profileModel);
Line 73: 
Line 74:         if (treeSelectedDc != null) {
Line 75:             profileModel.initNetworkQoSList(treeSelectedDc.getId(), 
null);
same here. The qos list should be initialized inside the relevant profile model.
There is no reason to do it outside the model. All the places that use the 
model have the same logic for getting the qos list.
Line 76:         }
Line 77:     }
Line 78: 
Line 79:     public void edit() {


....................................................
File 
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/vnicProfile/VnicProfilesEditor.java
Line 105:                             
existingProfileModel.getDcCompatibilityVersion());
Line 106: 
Line 107:                     NetworkQoS networkQoS = 
(NetworkQoS)((VnicProfileModel) value).getNetworkQoS().getSelectedItem();
Line 108:                     if (networkQoS != null) {
Line 109:                         
newVnicProfileModel.initNetworkQoSList(networkQoS.getStoragePoolId(), 
networkQoS.getId());
same here.
Line 110:                     }
Line 111:                     models.add(models.indexOf(existingProfileModel) + 
1, newVnicProfileModel);
Line 112: 
Line 113:                     setAcceptableValues(models);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I94ac502681b23e6e2dd138cd5e4de7c6ec482daa
Gerrit-PatchSet: 11
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: ofri masad <oma...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-Reviewer: ofri masad <oma...@redhat.com>
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to