Alona Kaplan has posted comments on this change.
Change subject: foreman integration - showing foreman hosts in new host dialog
......................................................................
Patch Set 10: (11 inline comments)
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostModel.java
Line 1316: SystemTreeItemModel selectedSystemTreeItem,
Line 1317: boolean newMode)
Line 1318: {
Line 1319: setHostId(vds.getId());
Line 1320: getRootPassword().setIsAvailable(newMode);
Don't like the 'newMode' parameter. Please override it it the NewHost and
EditHost models.
Line 1321: getOverrideIpTables().setIsAvailable(newMode);
Line 1322: setSpmPriorityValue(vds.getVdsSpmPriority());
Line 1323: setOriginalName(vds.getName());
Line 1324: getName().setEntity(vds.getName());
Line 1317: boolean newMode)
Line 1318: {
Line 1319: setHostId(vds.getId());
Line 1320: getRootPassword().setIsAvailable(newMode);
Line 1321: getOverrideIpTables().setIsAvailable(newMode);
same here.
Line 1322: setSpmPriorityValue(vds.getVdsSpmPriority());
Line 1323: setOriginalName(vds.getName());
Line 1324: getName().setEntity(vds.getName());
Line 1325: getHost().setEntity(vds.getHostName());
Line 1329: getConsoleAddressEnabled().setEntity(consoleAddressEnabled);
Line 1330: getConsoleAddress().setEntity(vds.getConsoleAddress());
Line 1331: getConsoleAddress().setIsChangable(consoleAddressEnabled);
Line 1332:
Line 1333: if ((!newMode && vds.getStatus() != VDSStatus.InstallFailed)
|| (newMode && getHost().getEntity() != null))
Same here. I don't like the 'newMode' parameter. You can extract this 'if'
block to a method and override it in the relevant children.
Line 1334: {
Line 1335: getHost().setIsChangable(false);
Line 1336: } else {
Line 1337: getHost().setIsChangable(true);
Line 1362:
Line 1363:
getPmSecondaryConcurrent().setEntity(vds.isPmSecondaryConcurrent());
Line 1364:
Line 1365:
Line 1366: if (dataCenters != null && vds.getStoragePoolId() != null)
Why did you add "vds.getStoragePoolId() != null" ?
If it is a way to check if you are in 'newMode' please override it in the
children.
Line 1367: {
Line 1368: getDataCenter().setItems(dataCenters);
Line 1369:
getDataCenter().setSelectedItem(Linq.firstOrDefault(dataCenters,
Line 1370: new
Linq.DataCenterPredicate(vds.getStoragePoolId())));
Line 1383: getCluster()
Line 1384: .setItems(new
ArrayList<VDSGroup>(Arrays.asList(new VDSGroup[] { tempVar })));
Line 1385: }
Line 1386: clusters = (ArrayList<VDSGroup>) getCluster().getItems();
Line 1387: if (clusters != null && vds.getVdsGroupId() != null) {
Same as the previous comment.
Line 1388:
getCluster().setSelectedItem(Linq.firstOrDefault(clusters,
Line 1389: new Linq.ClusterPredicate(vds.getVdsGroupId())));
Line 1390: }
Line 1391: if (getCluster().getSelectedItem() == null)
Line 1396: if (vds.getStatus() != VDSStatus.Maintenance &&
vds.getStatus() != VDSStatus.PendingApproval)
Line 1397: {
Line 1398: getDataCenter()
Line 1399: .setChangeProhibitionReason("Data Center can be
changed only when the Host is in Maintenance mode."); //$NON-NLS-1$
Line 1400: getDataCenter().setIsChangable(newMode);
Please avoid using 'newMode'
Line 1401: getCluster()
Line 1402: .setChangeProhibitionReason("Cluster can be
changed only when the Host is in Maintenance mode."); //$NON-NLS-1$
Line 1403: getCluster().setIsChangable(newMode);
Line 1404: }
Line 1399: .setChangeProhibitionReason("Data Center can be
changed only when the Host is in Maintenance mode."); //$NON-NLS-1$
Line 1400: getDataCenter().setIsChangable(newMode);
Line 1401: getCluster()
Line 1402: .setChangeProhibitionReason("Cluster can be
changed only when the Host is in Maintenance mode."); //$NON-NLS-1$
Line 1403: getCluster().setIsChangable(newMode);
Same here.
Line 1404: }
Line 1405: else if (selectedSystemTreeItem != null)
Line 1406: {
Line 1407: switch (selectedSystemTreeItem.getType())
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/NewHostModel.java
Line 18: public class NewHostModel extends HostModel {
Line 19:
Line 20: public NewHostModel() {
Line 21: super();
Line 22: setExternalHostName(new ListModel());
redundant- done in the HostModel
Line 23:
getExternalHostName().getSelectedItemChangedEvent().addListener(this);
Line 24:
getExternalHostName().setIsAvailable(ApplicationModeHelper.getUiMode() !=
ApplicationMode.GlusterOnly);
Line 25: setExternalHostProviderEnabled(new EntityModel());
Line 26: setProviders(new ListModel());
Line 21: super();
Line 22: setExternalHostName(new ListModel());
Line 23:
getExternalHostName().getSelectedItemChangedEvent().addListener(this);
Line 24:
getExternalHostName().setIsAvailable(ApplicationModeHelper.getUiMode() !=
ApplicationMode.GlusterOnly);
Line 25: setExternalHostProviderEnabled(new EntityModel());
same here.
Line 26: setProviders(new ListModel());
Line 27: getProviders().getSelectedItemChangedEvent().addListener(this);
Line 28:
getProviders().setIsAvailable(ApplicationModeHelper.getUiMode() !=
ApplicationMode.GlusterOnly);
Line 29:
Line 53: {
Line 54: Provider provider = (Provider)
getProviders().getSelectedItem();
Line 55: if (provider != null ) {
Line 56: AsyncQuery getHostsQuery = new AsyncQuery();
Line 57: getHostsQuery.setModel(this);
redundant
Line 58: getHostsQuery.asyncCallback = new INewAsyncCallback() {
Line 59: @Override
Line 60: public void onSuccess(Object model, Object result)
Line 61: {
....................................................
File
frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationConstants.java
Line 406: @DefaultStringValue("Show External Providers")
Line 407: String hostPopupEnableExternalHostProvider();
Line 408:
Line 409: @DefaultStringValue("External Hosts")
Line 410: String hostPopupExternalHostName();
Why do you need "External Host" and "External Hosts". I guess just one of the
is in use.
Line 411:
Line 412: @DefaultStringValue("Enable Power Management")
Line 413: String hostPopupPmEnabledLabel();
Line 414:
--
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: 10
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
