Hello Shmuel Melamud,

I'd like you to do a code review.  Please visit

    https://gerrit.ovirt.org/40782

to review the following change.

Change subject: ui: Incorrect selection reset in EntityModelCellTable
......................................................................

ui: Incorrect selection reset in EntityModelCellTable

When ListModel is used together with EntityModelCellTable, change of
selection in the ListModel causes corresponding change of selection in
EntityModelCellTable. Selection change handler in EntityModelCellTable
sets selection in the ListModel to null, planning to assign the new
selection afterwards. This causes another loop of events. As the result,
selection in EntityModelCellTable is set to null before it is read by
the event handler.

To fix this, the event handler is changed to get the selection before
resetting the ListModel. Also, additional checks are added to avoid
resetting the ListModel when it is already set to the desired value.
This eliminates extra events and side effects of their handlers.

Change-Id: I8c6fd184c567aee993737112bcb45135c83dc1cb
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1220142
Signed-off-by: Shmuel Melamud <smela...@redhat.com>
---
M 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelCellTable.java
1 file changed, 29 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/82/40782/1

diff --git 
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelCellTable.java
 
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelCellTable.java
index a418331..8f8dacb 100644
--- 
a/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelCellTable.java
+++ 
b/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/EntityModelCellTable.java
@@ -3,6 +3,7 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import org.ovirt.engine.core.common.utils.ListUtils;
 import org.ovirt.engine.ui.common.CommonApplicationConstants;
 import org.ovirt.engine.ui.common.PopupTableResources;
 import org.ovirt.engine.ui.common.gin.AssetProvider;
@@ -223,34 +224,50 @@
                     return;
                 }
 
-                getListModel().setSelectedItems(null);
-                getListModel().setSelectedItem(null);
-                // Clear "IsSelected"
-                for (EntityModel entity : (List<EntityModel>) 
getListModel().getItems()) {
-                    entity.setIsSelected(false);
-                }
-
                 // Set "IsSelected"
                 SelectionModel<? super EntityModel> selectionModel = 
EntityModelCellTable.this.getSelectionModel();
                 if (selectionModel instanceof SingleSelectionModel) {
                     EntityModel selectedObject =
                             ((SingleSelectionModel<EntityModel>) 
selectionModel).getSelectedObject();
-                    if (selectedObject != null) {
-                        selectedObject.setIsSelected(true);
-                        getListModel().setSelectedItem(selectedObject);
+                    if (getListModel().getSelectedItem() != selectedObject) {
+                        resetListModelSelection();
+                        if (selectedObject != null) {
+                            selectedObject.setIsSelected(true);
+                            getListModel().setSelectedItem(selectedObject);
+                        }
                     }
                 } else if (selectionModel instanceof MultiSelectionModel) {
                     List<EntityModel> selectedItems = new 
ArrayList<EntityModel>();
                     for (EntityModel entity : 
((MultiSelectionModel<EntityModel>) selectionModel).getSelectedSet()) {
-                        entity.setIsSelected(true);
                         selectedItems.add(entity);
                     }
-                    getListModel().setSelectedItems(selectedItems);
+                    List<EntityModel> prevSelectedItems = 
getListModel().getSelectedItems();
+                    if (prevSelectedItems == null && selectedItems.size() != 0 
||
+                            prevSelectedItems != null && 
!ListUtils.listsEqual(prevSelectedItems, selectedItems)) {
+                        resetListModelSelection();
+                        getListModel().setSelectedItems(selectedItems);
+                        for (EntityModel entity : selectedItems) {
+                            entity.setIsSelected(true);
+                        }
+                    }
                 }
             }
         });
     }
 
+    private void resetListModelSelection() {
+        if (getListModel() == null || getListModel().getItems() == null) {
+            return;
+        }
+
+        getListModel().setSelectedItems(null);
+        getListModel().setSelectedItem(null);
+        // Clear "IsSelected"
+        for (EntityModel entity : (List<EntityModel>) 
getListModel().getItems()) {
+            entity.setIsSelected(false);
+        }
+    }
+
     private void addCheckBoxColumn(boolean hideCheckbox, boolean 
showSelectAllCheckbox) {
 
         if (!hideCheckbox) {


-- 
To view, visit https://gerrit.ovirt.org/40782
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8c6fd184c567aee993737112bcb45135c83dc1cb
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud <smela...@redhat.com>
Gerrit-Reviewer: Shmuel Melamud <smela...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to