Vojtech Szocs has posted comments on this change.
Change subject: webadmin,userportal: Synchronize refresh.
......................................................................
Patch Set 2:
(3 comments)
Very good patchset, thanks for addressing my comments, see some of my other
comments inline.
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/RefreshActiveModel.java
Line 7: * and needs to inform models to refresh themselves.
Line 8: */
Line 9: @GenEvent
Line 10: public class RefreshActiveModel {
Line 11: Boolean doFastForward;
Why not use primitive boolean instead? (is null value acceptable here?)
....................................................
File
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/uicommon/model/TabModelProvider.java
Line 73: modelBoundWidgetChange();
Line 74: }
Line 75: }
Line 76: });
Line 77: getModel().unregisterHandlers();
Hm, at this point we don't have a reference to previous model -
onCommonModelChange is invoked after CommonModel causes model tree to
instantiate (WebAdmin) or after model providers instantiate their models
(UserPortal). So calling unregisterHandlers here is rather redundant, as
getModel points to current (new) model.
I suggest following for your consideration:
* in BaseApplicationInit#performLogin add this logic:
...
frontend.initLoggedInUser(loggedUser, loginPassword);
CleanupModelEvent.fire(eventBus); // this is new!
beforeUiCommonInitEvent(loginModel);
UiCommonInitEvent.fire(eventBus);
...
* CleanupModelEvent handled in TabModelProvider - call
getModel().unregisterHandlers()
This way, we could clean up old models right before new ones get created (as
part of beforeUiCommonInitEvent execution).
Line 78: getModel().setEventBus(getEventBus());
Line 79: }
Line 80:
Line 81: @SuppressWarnings("unchecked")
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/events/EventListModel.java
Line 266: }
Line 267: }
Line 268:
Line 269: @Override
Line 270: public void fireEvent(GwtEvent<?> event) {
This method could be pushed up the model hierarchy, i.e. into class which
declares the EventBus field & associated setter method.
Line 271: getEventBus().fireEvent(event);
Line 272: }
Line 273:
Line 274: private boolean entitiesChanged = true;
--
To view, visit http://gerrit.ovirt.org/21057
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa59712f8f74426b0ca6a132664167f42fb8d7b2
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@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
