Lior Vernia has posted comments on this change.

Change subject: webadmin: fix the TODOs
......................................................................


Patch Set 7:

(3 comments)

I don't think you did all you had set out to do in this patch. Also a possible 
null pointer exception if I'm not mistaken.

....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/profiles/VnicProfileListModel.java
Line 66: 
Line 67:         SystemTreeItemModel treeSelectedItem =
Line 68:                 (SystemTreeItemModel) 
CommonModel.getInstance().getSystemTree().getSelectedItem();
Line 69:         SystemTreeItemModel treeSelectedDc =
Line 70:                 treeSelectedItem.getType() == 
SystemTreeItemType.DataCenter ? treeSelectedItem : null;
Aren't you also missing the case where the selected item is a network, and then 
you could search up the tree for its DC ancestor? There's a method especially 
for that in SystemTreeItemModel, if I recall correctly.
Line 71:         final VnicProfileModel profileModel =
Line 72:                 new NewVnicProfileModel(this, ((StoragePool) 
treeSelectedDc.getEntity()).getcompatibility_version());
Line 73:         setWindow(profileModel);
Line 74: 


Line 68:                 (SystemTreeItemModel) 
CommonModel.getInstance().getSystemTree().getSelectedItem();
Line 69:         SystemTreeItemModel treeSelectedDc =
Line 70:                 treeSelectedItem.getType() == 
SystemTreeItemType.DataCenter ? treeSelectedItem : null;
Line 71:         final VnicProfileModel profileModel =
Line 72:                 new NewVnicProfileModel(this, ((StoragePool) 
treeSelectedDc.getEntity()).getcompatibility_version());
It's possible for treeSelectedDc to be null, isn't it?
Line 73:         setWindow(profileModel);
Line 74: 
Line 75:         initNetworkList(profileModel);
Line 76:     }


Line 180:     @Override
Line 181:     protected void syncSearch() {
Line 182:         // TODO - fix
Line 183:         // SearchParameters tempVar = new 
SearchParameters(getSearchString(), SearchType.Profile);
Line 184:         // tempVar.setMaxCount(getSearchPageSize());
It doesn't seem fixed according to the comments you yourself had left...
Line 185:         // super.syncSearch(VdcQueryType.Search, tempVar);
Line 186: 
Line 187:         SystemTreeItemModel treeSelectedItem =
Line 188:                 (SystemTreeItemModel) 
CommonModel.getInstance().getSystemTree().getSelectedItem();


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia432883c76e16c52fc395f517e8f9bd8bedfb584
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@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

Reply via email to