From d79d65c5f38787bfb2f773ffa4f6fcfeb491630a Mon Sep 17 00:00:00 2001
From: cuong-phan <39257573+cuong-phan@users.noreply.github.com>
Date: Thu, 16 May 2019 11:53:24 +0300
Subject: [PATCH] Fix duplicate grid column visibility change event (#11565)

* Fix duplicate grid column visibility change event

Toggle a column visibility through UI will fire 2 event. There should be only 1.

* Add Test
---
 server/src/main/java/com/vaadin/ui/Grid.java  |  3 +-
 ...GridEventSentOnColumnVisibilityChange.java | 64 +++++++++++++++++++
 ...EventSentOnColumnVisibilityChangeTest.java | 39 +++++++++++
 3 files changed, 105 insertions(+), 1 deletion(-)
 create mode 100644 uitest/src/main/java/com/vaadin/tests/components/grid/GridEventSentOnColumnVisibilityChange.java
 create mode 100644 uitest/src/test/java/com/vaadin/tests/components/grid/GridEventSentOnColumnVisibilityChangeTest.java

diff --git a/server/src/main/java/com/vaadin/ui/Grid.java b/server/src/main/java/com/vaadin/ui/Grid.java
index 3c9116fab3..d463832119 100644
--- a/server/src/main/java/com/vaadin/ui/Grid.java
+++ b/server/src/main/java/com/vaadin/ui/Grid.java
@@ -697,8 +697,9 @@ public class Grid<T> extends AbstractListing<T> implements HasComponents,
         @Override
         public void columnVisibilityChanged(String internalId, boolean hidden) {
             Column<T, ?> column = getColumnByInternalId(internalId);
+            column.checkColumnIsAttached();
             if (column.isHidden() != hidden) {
-                column.setHidden(hidden);
+                column.getState().hidden = hidden;
                 fireColumnVisibilityChangeEvent(column, hidden, true);
             }
         }
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridEventSentOnColumnVisibilityChange.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridEventSentOnColumnVisibilityChange.java
new file mode 100644
index 0000000000..dd4cf5e7e2
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridEventSentOnColumnVisibilityChange.java
@@ -0,0 +1,64 @@
+package com.vaadin.tests.components.grid;
+
+import java.util.Arrays;
+import java.util.List;
+
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
+import com.vaadin.ui.Grid;
+
+public class GridEventSentOnColumnVisibilityChange
+        extends AbstractTestUIWithLog {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        List<Person> people = Arrays.asList(
+                new Person("Nicolaus Copernicus", 1543),
+                new Person("Galileo Galilei", 1564),
+                new Person("Johannes Kepler", 1571));
+
+        Grid<Person> grid = new Grid<>();
+
+        grid.setItems(people);
+        grid.addColumn(Person::getName).setId("name").setCaption("Name")
+                .setHidable(true);
+        grid.addColumn(Person::getBirthYear).setCaption("Year of birth")
+                .setHidable(true);
+
+        grid.setSizeFull();
+
+        grid.addColumnVisibilityChangeListener(
+                event -> log("UserOriginated: " + event.isUserOriginated()));
+
+        addComponent(grid);
+    }
+
+    private class Person {
+        private final String name;
+        private final int birthYear;
+
+        public Person(String name, int birthYear) {
+            this.name = name;
+            this.birthYear = birthYear;
+        }
+
+        public String getName() {
+            return name;
+        }
+
+        public int getBirthYear() {
+            return birthYear;
+        }
+    }
+
+    @Override
+    public String getDescription() {
+        return "Every time when the user changes the visibility of the column,"
+                + " there should have only one event sent";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 11419;
+    }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridEventSentOnColumnVisibilityChangeTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridEventSentOnColumnVisibilityChangeTest.java
new file mode 100644
index 0000000000..c315513620
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridEventSentOnColumnVisibilityChangeTest.java
@@ -0,0 +1,39 @@
+package com.vaadin.tests.components.grid;
+
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.tests.tb3.SingleBrowserTest;
+
+public class GridEventSentOnColumnVisibilityChangeTest extends SingleBrowserTest {
+
+    @Test
+    public void changeVisibilityAssertLog() {
+        openTestURL();
+
+        GridElement grid = $(GridElement.class).first();
+        getSidebarOpenButton(grid).click();
+        // hide the first column
+        getSidebarPopup().findElements(By.tagName("td")).get(0).click();
+        
+        Assert.assertTrue("Log content should match",
+                "1. UserOriginated: true".equals(getLogRow(0)));
+    }
+
+    protected WebElement getSidebarOpenButton(GridElement grid) {
+        List<WebElement> elements = grid
+                .findElements(By.className("v-grid-sidebar-button"));
+        return elements.isEmpty() ? null : elements.get(0);
+    }
+
+    protected WebElement getSidebarPopup() {
+        List<WebElement> elements = findElements(
+                By.className("v-grid-sidebar-popup"));
+        return elements.isEmpty() ? null : elements.get(0);
+    }
+}
-- 
2.39.5