Alona Kaplan has posted comments on this change.
Change subject: foreman integration - showing foreman hosts in new host dialog
......................................................................
Patch Set 14: (7 inline comments)
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/EditHostModel.java
Line 19: }
Line 20:
Line 21: @Override
Line 22: protected void updateModelDataCenterFromVds(ArrayList<StoragePool>
dataCenters, VDS vds) {
Line 23: if (dataCenters != null && vds.getStoragePoolId() != null)
Why do you need to check- vds.getStoragePoolId() != null ?
Line 24: {
Line 25: getDataCenter().setItems(dataCenters);
Line 26:
getDataCenter().setSelectedItem(Linq.firstOrDefault(dataCenters,
Line 27: new
Linq.DataCenterPredicate(vds.getStoragePoolId())));
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostModel.java
Line 1419: getCluster()
Line 1420: .setItems(new
ArrayList<VDSGroup>(Arrays.asList(new VDSGroup[] { tempVar })));
Line 1421: }
Line 1422: clusters = (ArrayList<VDSGroup>) getCluster().getItems();
Line 1423: if (clusters != null && vds.getVdsGroupId() != null) {
Instead of checking vds.getVdsGroupId() != null you should extract the if
block to Add/EditHostModel.
Line 1424:
getCluster().setSelectedItem(Linq.firstOrDefault(clusters,
Line 1425: new Linq.ClusterPredicate(vds.getVdsGroupId())));
Line 1426: }
Line 1427: if (getCluster().getSelectedItem() == null)
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/NewHostModel.java
Line 54: }
Line 55:
Line 56: private void providers_SelectedItemChanged() {
Line 57: Provider provider = (Provider)
getProviders().getSelectedItem();
Line 58: if (provider != null ) {
Can shorten to-
getProviderSearchFilter().setIsChangable(provider!=null);
getProviderSearchFilterLabel().setIsChangable(provider!=null);
getUpdateHostsCommand().setIsExecutionAllowed(provider!=null);
Line 59: getProviderSearchFilter().setIsChangable(true);
Line 60: getProviderSearchFilterLabel().setIsChangable(true);
Line 61: getUpdateHostsCommand().setIsExecutionAllowed(true);
Line 62: } else {
Line 80: ListModel hostNameListModel =
getExternalHostName();
Line 81: hosts.add(0, null);
Line 82: hostNameListModel.setItems(hosts);
Line 83: hostNameListModel.setIsChangable(true);
Line 84: getProviderSearchFilter().setIsChangable(true);
You use this block multiple time. Maybe extract it to a method-
enableSearchHost(boolean enable)
Line 85:
getProviderSearchFilterLabel().setIsChangable(true);
Line 86:
getUpdateHostsCommand().setIsExecutionAllowed(true);
Line 87: }
Line 88: };
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/HostPopupView.java
Line 301: @Path(value = "consoleAddressEnabled.entity")
Line 302: EntityModelCheckBoxEditor consoleAddressEnabled;
Line 303:
Line 304: @UiField(provided = true)
Line 305: InfoIcon providerSearchInfoIcon;
In NewHostPopupView- you should setVisible(false) this icon in case of gluster
only mode (as you have done in the model for all the other items related to
providers).
Line 306:
Line 307: @UiField
Line 308: FlowPanel externalProviderPanel;
Line 309:
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/HostPopupView.ui.xml
Line 128: width: 197px;
Line 129: display: inline-block;
Line 130: padding-left: 0px;
Line 131: }
Line 132:
Please remove trailing whitespace.
Line 133: .searchPanel {
Line 134: display: inline-block;
Line 135: float: right;
Line 136: line-height: 30px;
Line 171: <g:FlowPanel width="100%">
Line 172: <g:FlowPanel
addStyleNames="{style.filterIcon}">
Line 173: <e:EntityModelTextBoxEditor
ui:field="providerSearchFilterLabel"/>
Line 174: <d:InfoIcon
ui:field="providerSearchInfoIcon" addStyleNames="{style.filterIcon}"/>
Line 175: </g:FlowPanel>
same here.
Line 176: <g:FlowPanel
addStyleNames="{style.searchPanel}">
Line 177: <g:HorizontalPanel
verticalAlignment="middle">
Line 178:
<e:EntityModelTextBoxEditor ui:field="providerSearchFilterEditor"/>
Line 179: <g:Image
ui:field="updateHostsButton"/>
--
To view, visit http://gerrit.ovirt.org/14582
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I30ea180e477a8f0714c4dc97ec55f29be515723a
Gerrit-PatchSet: 14
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Alona Kaplan <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
