aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAnna Koskinen <Ansku@users.noreply.github.com>2019-11-06 10:28:47 +0200
committerGitHub <noreply@github.com>2019-11-06 10:28:47 +0200
commit810b5b0aed16c2ea56ee55d6361d35ba984305d7 (patch)
tree845dbc148044dc7d638a9066287f540438b65337
parenteef77afb17ee72a2e3185c6f866f17b98b2f633e (diff)
downloadvaadin-framework-810b5b0aed16c2ea56ee55d6361d35ba984305d7.tar.gz
vaadin-framework-810b5b0aed16c2ea56ee55d6361d35ba984305d7.zip
Check actual Grid selection instead of relying on allSelected flag. (#11787) (#11790)
The checkbox for selecting all rows only selects all the rows that have not been filtered out. Changing the filtering does not change the selection or the checkbox state so assuming that all rows are selected simply because the checkbox has been checked cannot work. Fixes #11479
-rw-r--r--client/src/main/java/com/vaadin/client/connectors/grid/MultiSelectionModelConnector.java2
-rw-r--r--server/src/main/java/com/vaadin/ui/components/grid/MultiSelectionModelImpl.java27
-rw-r--r--uitest/src/main/java/com/vaadin/tests/components/grid/GridSelectAllFiltering.java71
-rw-r--r--uitest/src/test/java/com/vaadin/tests/components/grid/GridSelectAllFilteringTest.java62
4 files changed, 153 insertions, 9 deletions
diff --git a/client/src/main/java/com/vaadin/client/connectors/grid/MultiSelectionModelConnector.java b/client/src/main/java/com/vaadin/client/connectors/grid/MultiSelectionModelConnector.java
index 7bbc66d271..de003e6daf 100644
--- a/client/src/main/java/com/vaadin/client/connectors/grid/MultiSelectionModelConnector.java
+++ b/client/src/main/java/com/vaadin/client/connectors/grid/MultiSelectionModelConnector.java
@@ -250,7 +250,7 @@ public class MultiSelectionModelConnector
@Override
protected boolean isSelected(JsonObject item) {
- return getState().allSelected || super.isSelected(item);
+ return super.isSelected(item);
}
/**
diff --git a/server/src/main/java/com/vaadin/ui/components/grid/MultiSelectionModelImpl.java b/server/src/main/java/com/vaadin/ui/components/grid/MultiSelectionModelImpl.java
index f51744c96e..0ef1d1a5f7 100644
--- a/server/src/main/java/com/vaadin/ui/components/grid/MultiSelectionModelImpl.java
+++ b/server/src/main/java/com/vaadin/ui/components/grid/MultiSelectionModelImpl.java
@@ -15,7 +15,24 @@
*/
package com.vaadin.ui.components.grid;
-import com.vaadin.data.provider.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import com.vaadin.data.provider.DataCommunicator;
+import com.vaadin.data.provider.DataProvider;
+import com.vaadin.data.provider.HierarchicalDataProvider;
+import com.vaadin.data.provider.HierarchicalQuery;
+import com.vaadin.data.provider.Query;
import com.vaadin.event.selection.MultiSelectionEvent;
import com.vaadin.event.selection.MultiSelectionListener;
import com.vaadin.shared.Registration;
@@ -23,11 +40,6 @@ import com.vaadin.shared.data.selection.GridMultiSelectServerRpc;
import com.vaadin.shared.ui.grid.MultiSelectionModelState;
import com.vaadin.ui.MultiSelect;
-import java.util.*;
-import java.util.function.Consumer;
-import java.util.stream.Collectors;
-import java.util.stream.Stream;
-
/**
* Multiselection model for grid.
* <p>
@@ -134,8 +146,7 @@ public class MultiSelectionModelImpl<T> extends AbstractSelectionModel<T>
@Override
public boolean isSelected(T item) {
- return isAllSelected()
- || selectionContainsId(getGrid().getDataProvider().getId(item));
+ return selectionContainsId(getGrid().getDataProvider().getId(item));
}
/**
diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridSelectAllFiltering.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSelectAllFiltering.java
new file mode 100644
index 0000000000..3c85e9c435
--- /dev/null
+++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSelectAllFiltering.java
@@ -0,0 +1,71 @@
+package com.vaadin.tests.components.grid;
+
+import java.util.Iterator;
+import java.util.Set;
+
+import com.vaadin.data.provider.DataProvider;
+import com.vaadin.data.provider.ListDataProvider;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.data.bean.Person;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Grid;
+import com.vaadin.ui.Grid.SelectionMode;
+import com.vaadin.ui.renderers.NumberRenderer;
+
+public class GridSelectAllFiltering extends SimpleGridUI {
+ private String filterText = "Johannes";
+
+ @Override
+ protected void setup(VaadinRequest request) {
+ Grid<Person> grid = new Grid<>();
+ grid.setSelectionMode(SelectionMode.MULTI);
+ grid.setHeightByRows(3);
+ grid.addColumn(Person::getFirstName);
+ grid.addColumn(Person::getAge, new NumberRenderer());
+
+ ListDataProvider<Person> dataProvider = DataProvider
+ .ofCollection(createPersons());
+ dataProvider.setFilter(person -> {
+ if (person.getFirstName().contains(filterText)) {
+ return false;
+ }
+ return true;
+ });
+ grid.setDataProvider(dataProvider);
+
+ Button toggleButton = new Button("Toggle filter", e -> {
+ if ("Johannes".equals(filterText)) {
+ filterText = "Galileo";
+ } else {
+ filterText = "Johannes";
+ }
+ dataProvider.refreshAll();
+ });
+ toggleButton.setId("toggle");
+
+ Button checkButton = new Button("Check selection", e -> {
+ Set<Person> selected = grid.getSelectedItems();
+ Iterator<Person> i = selected.iterator();
+ log("selected " + selected.size()
+ + (i.hasNext() ? ": " + i.next().getFirstName() : "")
+ + (i.hasNext() ? ", " + i.next().getFirstName() : "")
+ + (i.hasNext() ? ", " + i.next().getFirstName() : "")
+ + (i.hasNext() ? "... " : ""));
+ });
+ checkButton.setId("check");
+
+ addComponents(grid, toggleButton, checkButton);
+ }
+
+ @Override
+ protected Integer getTicketNumber() {
+ return 11479;
+ }
+
+ @Override
+ protected String getTestDescription() {
+ return "Selecting all does not select items that have been "
+ + "filtered out, they should not be shown selected "
+ + "after the filter changes.";
+ }
+}
diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridSelectAllFilteringTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridSelectAllFilteringTest.java
new file mode 100644
index 0000000000..37c7d6281c
--- /dev/null
+++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridSelectAllFilteringTest.java
@@ -0,0 +1,62 @@
+package com.vaadin.tests.components.grid;
+
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+import org.openqa.selenium.WebElement;
+
+import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.ButtonElement;
+import com.vaadin.testbench.elements.GridElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class GridSelectAllFilteringTest extends MultiBrowserTest {
+
+ @Test
+ public void checkSelectAll() {
+ openTestURL();
+
+ GridElement grid = $(GridElement.class).first();
+ ButtonElement toggleButton = $(ButtonElement.class).id("toggle");
+ ButtonElement checkButton = $(ButtonElement.class).id("check");
+
+ // ensure no initial selection
+ checkButton.click();
+ assertEquals("Unexpected log entry,", "1. selected 0", getLogRow(0));
+ assertEquals("Unexpected amount of visually selected rows,", 0,
+ grid.findElements(By.className("v-grid-row-selected")).size());
+
+ // select all
+ WebElement selectAllCheckbox = grid
+ .findElement(By.className("v-grid-select-all-checkbox"));
+ selectAllCheckbox.click();
+
+ // ensure only the two visible rows get selected
+ checkButton.click();
+ assertEquals("Unexpected log entry,",
+ "2. selected 2: Nicolaus Copernicus, Galileo Galilei",
+ getLogRow(0));
+ assertEquals("Unexpected amount of visually selected rows,", 2,
+ grid.findElements(By.className("v-grid-row-selected")).size());
+
+ // toggle filter
+ toggleButton.click();
+
+ // ensure selection did not change but only one selected row is visible
+ checkButton.click();
+ assertEquals("Unexpected log entry,",
+ "3. selected 2: Nicolaus Copernicus, Galileo Galilei",
+ getLogRow(0));
+ assertEquals("Unexpected amount of visually selected rows,", 1,
+ grid.findElements(By.className("v-grid-row-selected")).size());
+
+ // remove all selections
+ selectAllCheckbox.click();
+
+ // ensure all selections got removed whether they were visible or not
+ checkButton.click();
+ assertEquals("Unexpected log entry,", "4. selected 0", getLogRow(0));
+ assertEquals("Unexpected amount of visually selected rows,", 0,
+ grid.findElements(By.className("v-grid-row-selected")).size());
+ }
+}