Tomas Jelinek has posted comments on this change.
Change subject: frontend: Allow console settings per user and pool.
......................................................................
Patch Set 3:
(9 comments)
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/presenter/popup/ConsolePopupPresenterWidget.java
Line 135: initModel(model);
Line 136: initView(model);
Line 137: initListeners(model);
Line 138:
Line 139: String vmName = (model.getVmConsoles() instanceof
VmConsolesImpl)
better would be to have a method VmConsoles something like getName() and the
specific children would have the specific implementation.
Line 140: ? model.getVmConsoles().getVm().getName()
Line 141: : model.getVmConsoles().getVm().getVmPoolName(); //
for pool dialogs display pool name
Line 142:
Line 143: getView().setVmName(vmName);
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/utils/ConsoleOptionsFrontendPersisterImpl.java
Line 62: ConsoleProtocol selectedProtocol =
consoles.getSelectedProcotol();
Line 63: ConsoleContext context = consoles.getConsoleContext();
Line 64:
Line 65: String id = isPool
Line 66: ? consoles.getVm().getVmPoolId().toString()
same as with get name
Line 67: : consoles.getVm().getId().toString();
Line 68: KeyMaker keyMaker = new KeyMaker(id, isPool, context);
Line 69:
Line 70: clientStorage.setLocalItem(keyMaker.make(SELECTED_PROTOCOL),
selectedProtocol.toString());
Line 83: String vmId = guidToStringNullSafe(consoles.getVm().getId());
Line 84: String poolId =
guidToStringNullSafe(consoles.getVm().getVmPoolId());
Line 85: ConsoleContext context = consoles.getConsoleContext();
Line 86:
Line 87: KeyMaker poolKeyMaker = new KeyMaker(poolId, true, context);
please add a check to if the poolId is null
Line 88: loadConsolesWithKeymaker(consoles, poolKeyMaker); // load pool
defaults
Line 89: if (consoles instanceof VmConsolesImpl) {
Line 90: KeyMaker vmKeyMaker = new KeyMaker(vmId, false, context);
Line 91: loadConsolesWithKeymaker(consoles, vmKeyMaker); // load vm
Line 222: clientStorage.setLocalItem(key, Boolean.toString(value));
Line 223: }
Line 224:
Line 225: class KeyMaker {
Line 226: private final String vmOrPoolId;
what about just "id"? ;)
Line 227:
Line 228: private final boolean isPool;
Line 229:
Line 230: private final ConsoleContext context;
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/ConsolesBase.java
Line 16: import org.ovirt.engine.ui.uicompat.ConstantsManager;
Line 17: import org.ovirt.engine.ui.uicompat.UIConstants;
Line 18: import org.ovirt.engine.ui.uicompat.UIMessages;
Line 19:
Line 20: public class ConsolesBase {
please make this calss abstract and make it implement the VmConsoles interface.
Line 21:
Line 22: private ConsoleProtocol selectedProtocol;
Line 23: protected final Map<ConsoleProtocol, ConsoleModel> consoleModels;
Line 24: protected static final Map<Class, ConsoleProtocol>
modelTypeMapping;
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalBasicListModel.java
Line 321
Line 322
Line 323
Line 324
Line 325
please don't forget to move also this line into the new code
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/userportal/UserPortalListModel.java
Line 1403: Collections.sort(all, new
NameableComparator());
Line 1404:
Line 1405: ArrayList<Model> items = new
ArrayList<Model>();
Line 1406: for (Object item : all) {
Line 1407: VmConsoles consoles = (item
instanceof VM)
please move this instanceof into comsoleModlesCache.getVmConsolesForEntity(...)
or similar
Line 1408: ?
consoleModelsCache.getVmConsolesForVmId(((VM) item).getId())
Line 1409: :
consoleModelsCache.getVmConsolesForPoolId(((VmPool) item).getVmPoolId());
Line 1410:
Line 1411: UserPortalItemModel model = new
UserPortalItemModel(item, consoles);
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/utils/ConnectAutomaticallyManager.java
Line 75: public EventChangeListener(AbstractUserPortalListModel model) {
Line 76: this.model = model;
Line 77: }
Line 78:
Line 79: public void register() {
you need to listen to "CanConnectAutomatically" property change event
Line 80: model.getItemsChangedEvent().addListener(this);
Line 81: }
Line 82:
Line 83: public void unregister() {
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/java/org/ovirt/engine/ui/userportal/widget/extended/vm/ConsoleEditButtonCell.java
Line 9
Line 10
Line 11
Line 12
Line 13
model.getVM() != null still should be there
--
To view, visit http://gerrit.ovirt.org/23007
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0102ad0f44e4fb826c561ac593a428b51622a377
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@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
