Lior Vernia has posted comments on this change.
Change subject: webadmin: add VnicProfile as main tab
......................................................................
Patch Set 25: (17 inline comments)
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/profiles/VnicProfileListModel.java
Line 64: {
Line 65: return;
Line 66: }
Line 67:
Line 68: // TODO - fix
So, to actually do? :)
Line 69: // final VnicProfileModel profileModel = new
NewVnicProfileModel(this, getEntity().getCompatibilityVersion());
Line 70: final VnicProfileModel profileModel = new
NewVnicProfileModel(this, Version.v3_3);
Line 71: setWindow(profileModel);
Line 72:
Line 80: {
Line 81: return;
Line 82: }
Line 83:
Line 84: // TODO - fix
Same.
Line 85: // final VnicProfileModel profileModel = new
EditVnicProfileModel(this, getEntity().getCompatibilityVersion(),
Line 86: // getEntity());
Line 87: final VnicProfileModel profileModel = new
EditVnicProfileModel(this, Version.v3_3, profile);
Line 88: setWindow(profileModel);
Line 100: ConfirmationModel model = new RemoveVnicProfileModel(this,
getSelectedItems(), true);
Line 101: setConfirmWindow(model);
Line 102: }
Line 103:
Line 104: private void initNetworkList(final VnicProfileModel profileModel)
{
Perhaps move this logic to VnicProfileModel?
Line 105: profileModel.startProgress(null);
Line 106:
Line 107: SystemTreeItemModel treeSelectedItem =
Line 108: (SystemTreeItemModel)
CommonModel.getInstance().getSystemTree().getSelectedItem();
Line 133: (ArrayList<Network>)
((VdcQueryReturnValue) ReturnValue).getReturnValue();
Line 134:
Line 135: profileModel.getNetwork().setItems(networks);
Line 136:
Line 137: if (profileModel instanceof EditVnicProfileModel)
{
If this code is inside VnicProfileModel, you wouldn't have to do this, you
could call some protected method that's implemented one way in New and another
in Edit.
Line 138: Network currentNetwork =
Line 139:
findNetwork(profileModel.getProfile().getNetworkId(), networks);
Line 140:
profileModel.getNetwork().setSelectedItem(currentNetwork);
Line 141:
profileModel.getNetwork().setIsChangable(false);
Line 135: profileModel.getNetwork().setItems(networks);
Line 136:
Line 137: if (profileModel instanceof EditVnicProfileModel)
{
Line 138: Network currentNetwork =
Line 139:
findNetwork(profileModel.getProfile().getNetworkId(), networks);
This is strange. If you were in EditVnicProfieModel, then you would already
have the profile, and therefore the network ID, to begin with. You wouldn't
have to query for all the networks in the DC chosen in the tree.
So the split between New and Edit should happen earlier, and again in my
opinion probably in the corresponding models rather than by checking for
instanceof.
Line 140:
profileModel.getNetwork().setSelectedItem(currentNetwork);
Line 141:
profileModel.getNetwork().setIsChangable(false);
Line 142: } else {
Line 143:
profileModel.getNetwork().setSelectedItem(Linq.firstOrDefault(networks));
Line 149: Frontend.RunQuery(VdcQueryType.GetAllNetworks,
queryParams, _asyncQuery);
Line 150: }
Line 151: }
Line 152:
Line 153: private Network findNetwork(Guid networkId, List<Network>
networks) {
This method would have no use if the above changes were implemented...
Line 154: for (Network network : networks) {
Line 155: if (networkId.equals(network.getId())) {
Line 156: return network;
Line 157: }
Line 173: }
Line 174:
Line 175: @Override
Line 176: public boolean isSearchStringMatch(String searchString) {
Line 177: return
searchString.trim().toLowerCase().startsWith("network"); //$NON-NLS-1$
Surely this is not the right search string prefix? Does the search work alright?
Line 178: }
Line 179:
Line 180: @Override
Line 181: protected void syncSearch() {
Line 178: }
Line 179:
Line 180: @Override
Line 181: protected void syncSearch() {
Line 182: // TODO - fix
Same comment as before.
Line 183: // SearchParameters tempVar = new
SearchParameters(getSearchString(), SearchType.Profile);
Line 184: // tempVar.setMaxCount(getSearchPageSize());
Line 185: // super.syncSearch(VdcQueryType.Search, tempVar);
Line 186:
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/profiles/VnicProfileTemplateListModel.java
Line 43: getSearchCommand().execute();
Line 44: }
Line 45:
Line 46: @Override
Line 47: public void setItems(Iterable value) {
You can use SearchableListModel's setComparator() method in the constructor
instead, which also keeps the items sorted if somebody adds an item by doing
getItems().add().
Line 48: if (value != null) {
Line 49: List<VmTemplate> itemList = (List<VmTemplate>) value;
Line 50: Collections.sort(itemList, new Comparator<VmTemplate>() {
Line 51:
Line 67: }
Line 68: }
Line 69:
Line 70: @Override
Line 71: protected void syncSearch() {
Fix this method to use the SearchableListModel search mechanism?
Line 72: if (getEntity() == null)
Line 73: {
Line 74: return;
Line 75: }
Line 100: getSearchCommand().execute();
Line 101: }
Line 102: }
Line 103:
Line 104: private void updateActionAvailability() {
If this does nothing, there's no need for it to be here and for other methods
to call it. I'm actually for doing preparation work for the future, but if
you're against as you usually tell me, then this should go :)
Line 105: }
Line 106:
Line 107: @Override
Line 108: protected void onSelectedItemChanged() {
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/profiles/VnicProfileVmListModel.java
Line 43: getSearchCommand().execute();
Line 44: }
Line 45:
Line 46: @Override
Line 47: public void setItems(Iterable value) {
Same as with templates subtab, possible to use
SearchableListModel.setComparator() in the constructor instead.
Line 48: if (value != null) {
Line 49: List<VM> itemList = (List<VM>) value;
Line 50: Collections.sort(itemList, new Comparator<VM>() {
Line 51:
Line 67: }
Line 68: }
Line 69:
Line 70: @Override
Line 71: protected void syncSearch() {
Same as templates subtab, should probably use super.syncSearch().
Line 72: if (getEntity() == null)
Line 73: {
Line 74: return;
Line 75: }
Line 100: getSearchCommand().execute();
Line 101: }
Line 102: }
Line 103:
Line 104: private void updateActionAvailability() {
Same as templates subtab.
Line 105: }
Line 106:
Line 107: @Override
Line 108: protected void onSelectedItemChanged() {
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationConstants.java
Line 66:
Line 67: @DefaultStringValue("Networks")
Line 68: String networkMainTabLabel();
Line 69:
Line 70: @DefaultStringValue("Vnic Profiles")
Like comment in earlier patch, I think this should either be VNIC or vNIC,
depending on a consistent choice of style.
Line 71: String vnicProfilesMainTabLabel();
Line 72:
Line 73: @DefaultStringValue("Storage")
Line 74: String storageMainTabLabel();
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/gin/PresenterModule.java
Line 924: SubTabProviderNetworkView.class,
Line 925: SubTabProviderNetworkPresenter.ProxyDef.class);
Line 926:
Line 927: // Profile
Line 928: bindPresenter(VnicProfileSubTabPanelPresenter.class,
I think this might not be needed.
Line 929: VnicProfileSubTabPanelPresenter.ViewDef.class,
Line 930: VnicProfileSubTabPanelView.class,
Line 931: VnicProfileSubTabPanelPresenter.ProxyDef.class);
Line 932: bindPresenter(SubTabVnicProfilePermissionPresenter.class,
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/profile/VnicProfileSubTabPanelView.java
Line 4: import org.ovirt.engine.ui.common.widget.tab.AbstractTabPanel;
Line 5: import
org.ovirt.engine.ui.webadmin.section.main.presenter.tab.profile.VnicProfileSubTabPanelPresenter;
Line 6: import org.ovirt.engine.ui.webadmin.widget.tab.SimpleTabPanel;
Line 7:
Line 8: public class VnicProfileSubTabPanelView extends AbstractTabPanelView
implements VnicProfileSubTabPanelPresenter.ViewDef {
I think this isn't used anymore.
Line 9:
Line 10: private final SimpleTabPanel tabPanel = new SimpleTabPanel();
Line 11:
Line 12: public VnicProfileSubTabPanelView() {
--
To view, visit http://gerrit.ovirt.org/16998
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ied73fa4ecdb58c3224dc78f6641b2d56d55ec73b
Gerrit-PatchSet: 25
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
