Fix Grid editor hanging on exception in commit (#15536)

This patch adds a minimal editor subpart support.

Change-Id: I36a81cb432f71821715cb60338a07a289bdae18d

diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index 8c1b833acc9c82257af2546071aa885366bd5bb8..1bca9e84aef0eb6d217d4d1ef644c5e488b981cf 100644 (file)
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -121,6 +121,7 @@ import com.vaadin.client.widget.grid.sort.SortEvent;
 import com.vaadin.client.widget.grid.sort.SortHandler;
 import com.vaadin.client.widget.grid.sort.SortOrder;
 import com.vaadin.client.widgets.Escalator.AbstractRowContainer;
+import com.vaadin.client.widgets.Grid.Editor.State;
 import com.vaadin.shared.data.sort.SortDirection;
 import com.vaadin.shared.ui.grid.GridConstants;
 import com.vaadin.shared.ui.grid.GridStaticCellType;
@@ -4766,6 +4767,20 @@ public class Grid<T> extends ResizeComposite implements
             container = escalator.getBody();
         } else if (type.equalsIgnoreCase("footer")) {
             container = escalator.getFooter();
+        } else if (type.equalsIgnoreCase("editor")) {
+            if (editor.getState() != State.ACTIVE) {
+                // Editor is not there.
+                return null;
+            }
+
+            if (indices.length == 0) {
+                return DOM.asOld(editor.editorOverlay);
+            } else if (indices.length == 1 && indices[0] < columns.size()) {
+                escalator.scrollToColumn(indices[0], ScrollDestination.ANY, 0);
+                return editor.getWidget(columns.get(indices[0])).getElement();
+            } else {
+                return null;
+            }
         }
 
         if (null != container) {
@@ -4851,6 +4866,22 @@ public class Grid<T> extends ResizeComposite implements
                         + (containerRow ? "]" : "][" + cell.getColumn() + "]");
             }
         }
+
+        // Check if subelement is part of editor.
+        if (editor.getState() == State.ACTIVE) {
+            if (editor.editorOverlay.isOrHasChild(subElement)) {
+                int i = 0;
+                for (Column<?, T> column : columns) {
+                    if (editor.getWidget(column).getElement()
+                            .isOrHasChild(subElement)) {
+                        return "editor[" + i + "]";
+                    }
+                    ++i;
+                }
+                return "editor";
+            }
+        }
+
         return null;
     }
 

diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index d9f1b266a282e019bcb953a3b3fd4b88676d1504..fa0ec6fb8d601a147de0ee81fbd47a434508c03a 100644 (file)
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -2719,10 +2719,10 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
                 try {
                     Object id = getContainerDataSource().getIdByIndex(rowIndex);
                     doEditItem(id);
-                    getEditorRpc().confirmBind();
                 } catch (Exception e) {
                     handleError(e);
                 }
+                getEditorRpc().confirmBind();
             }
 
             @Override
@@ -2739,10 +2739,10 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
             public void save(int rowIndex) {
                 try {
                     saveEditor();
-                    getEditorRpc().confirmSave();
                 } catch (Exception e) {
                     handleError(e);
                 }
+                getEditorRpc().confirmSave();
             }
 
             private void handleError(Exception e) {

diff --git a/uitest/src/com/vaadin/testbench/elements/GridElement.java b/uitest/src/com/vaadin/testbench/elements/GridElement.java
index 254acbfa2a471292e2dc6ae15f5e6aa4bf7dc368..0c94c1dd88c308bcbacecc1b490c2c534753f285 100644 (file)
--- a/uitest/src/com/vaadin/testbench/elements/GridElement.java
+++ b/uitest/src/com/vaadin/testbench/elements/GridElement.java
@@ -62,6 +62,51 @@ public class GridElement extends AbstractComponentElement {
         }
     }
 
+    public static class GridEditorElement extends AbstractElement {
+
+        private GridElement grid;
+
+        private GridEditorElement setGrid(GridElement grid) {
+            this.grid = grid;
+            return this;
+        }
+
+        /**
+         * Gets the editor field for column in given index.
+         * 
+         * @param colIndex
+         *            column index
+         * @return the editor field for given location
+         */
+        public TestBenchElement getField(int colIndex) {
+            return grid.getSubPart("#editor[" + colIndex + "]");
+        }
+
+        /**
+         * Saves the fields of this editor.
+         * <p>
+         * <em>Note:</em> that this closes the editor making this element
+         * useless.
+         */
+        public void save() {
+            getField(0);
+            List<WebElement> buttons = findElements(By.xpath("./button"));
+            buttons.get(0).click();
+        }
+
+        /**
+         * Cancels this editor.
+         * <p>
+         * <em>Note:</em> that this closes the editor making this element
+         * useless.
+         */
+        public void cancel() {
+            getField(0);
+            List<WebElement> buttons = findElements(By.xpath("./button"));
+            buttons.get(1).click();
+        }
+    }
+
     /**
      * Scrolls Grid element so that wanted row is displayed
      * 
@@ -262,6 +307,11 @@ public class GridElement extends AbstractComponentElement {
         return rootElements.get(2);
     }
 
+    public GridEditorElement getEditor() {
+        return getSubPart("#editor").wrap(GridEditorElement.class)
+                .setGrid(this);
+    }
+
     /**
      * Helper function to get Grid subparts wrapped correctly
      * 
@@ -272,5 +322,4 @@ public class GridElement extends AbstractComponentElement {
     private TestBenchElement getSubPart(String subPartSelector) {
         return (TestBenchElement) findElement(By.vaadin(subPartSelector));
     }
-
 }

diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java
index faa7744ff8f429231eda93c3bf51027ab3230fad..35b2fc24fed2a7e32c48572624ba82b7203328da 100644 (file)
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java
@@ -16,6 +16,7 @@
 package com.vaadin.tests.components.grid.basicfeatures.server;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -30,6 +31,8 @@ import org.openqa.selenium.NoSuchElementException;
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.interactions.Actions;
 
+import com.vaadin.testbench.elements.GridElement.GridEditorElement;
+import com.vaadin.testbench.elements.NotificationElement;
 import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeatures;
 import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeaturesTest;
 
@@ -37,6 +40,7 @@ public class GridEditorTest extends GridBasicFeaturesTest {
 
     @Before
     public void setUp() {
+        setDebug(true);
         openTestURL();
         selectMenuPath("Component", "Editor", "Enabled");
     }
@@ -165,4 +169,21 @@ public class GridEditorTest extends GridBasicFeaturesTest {
         return getEditor().findElements(By.className("v-textfield"));
 
     }
+
+    @Test
+    public void testInvalidEdition() {
+        selectMenuPath("Component", "Editor", "Edit item 5");
+        assertFalse(logContainsText("Exception occured, java.lang.IllegalStateException"));
+        GridEditorElement editor = getGridElement().getEditor();
+        WebElement intField = editor.getField(7);
+        intField.clear();
+        intField.sendKeys("banana phone");
+        editor.save();
+        assertTrue(
+                "No exception on invalid value.",
+                logContainsText("Exception occured, com.vaadin.data.fieldgroup.FieldGroup$CommitExceptionCommit failed"));
+        selectMenuPath("Component", "Editor", "Edit item 100");
+        assertFalse("Exception should not exist",
+                isElementPresent(NotificationElement.class));
+    }
 }