Implement Vaadin-specific editor row commit/discard handling (#13334)

Change-Id: Iaaf5e4461adb1d4fd9b8f042c6001b8408d1a0e4

7 files changed, 214 insertions, 40 deletions

diff --git a/client/src/com/vaadin/client/ui/grid/GridConnector.java b/client/src/com/vaadin/client/ui/grid/GridConnector.java
index 9f09e00c4d..505b2870f5 100644
--- a/client/src/com/vaadin/client/ui/grid/GridConnector.java
+++ b/client/src/com/vaadin/client/ui/grid/GridConnector.java
@@ -150,6 +150,12 @@ public class GridConnector extends AbstractHasComponentsConnector implements
                 }
 
                 @Override
+                public void discard(int rowIndex) {
+                    serverInitiated = true;
+                    GridConnector.this.getWidget().getEditorRow().discard();
+                }
+
+                @Override
                 public void cancel(int rowIndex) {
                     serverInitiated = true;
                     GridConnector.this.getWidget().getEditorRow().cancel();
@@ -159,39 +165,43 @@ public class GridConnector extends AbstractHasComponentsConnector implements
                 public void confirmBind() {
                     endRequest();
                 }
+
+                @Override
+                public void confirmCommit() {
+                    endRequest();
+                }
             });
         }
 
         @Override
         public void bind(EditorRowRequest request) {
-            int index = request.getRowIndex();
+            if (!handleServerInitiated(request)) {
+                startRequest(request);
+                rpc.bind(request.getRowIndex());
+            }
+        }
 
-            if (serverInitiated) {
-                /*
-                 * EditorRow is calling us as a result of an RPC from the
-                 * server, so no need to do another roundtrip.
-                 */
-                serverInitiated = false;
-                request.invokeCallback();
-            } else {
-                /*
-                 * The clientside wanted to open the editor row, so inform the
-                 * server and proceed only when confirmation is received.
-                 */
+        @Override
+        public void commit(EditorRowRequest request) {
+            if (!handleServerInitiated(request)) {
+                startRequest(request);
+                rpc.commit(request.getRowIndex());
+            }
+        }
+
+        @Override
+        public void discard(EditorRowRequest request) {
+            if (!handleServerInitiated(request)) {
                 startRequest(request);
-                rpc.bind(index);
+                rpc.discard(request.getRowIndex());
             }
         }
 
         @Override
         public void cancel(EditorRowRequest request) {
-            /*
-             * Tell the server to cancel unless it was the server that told us
-             * to cancel. Cancels don't need a confirmation.
-             */
-            if (serverInitiated) {
-                serverInitiated = false;
-            } else {
+            if (!handleServerInitiated(request)) {
+                // No startRequest as we don't get (or need)
+                // a confirmation from the server
                 rpc.cancel(request.getRowIndex());
             }
         }
@@ -210,9 +220,30 @@ public class GridConnector extends AbstractHasComponentsConnector implements
             }
         }
 
-        private void startRequest(EditorRowRequest request) {
+        /**
+         * Used to handle the case where EditorRow calls us because it was
+         * invoked by the server via RPC and not by the client. In that case, we
+         * simply synchronously complete the request.
+         * 
+         * @param request
+         *            the request object
+         * @return true if the request was originally triggered by the server,
+         *         false otherwise
+         */
+        private boolean handleServerInitiated(EditorRowRequest request) {
             assert request != null;
             assert currentRequest == null;
+
+            if (serverInitiated) {
+                serverInitiated = false;
+                request.invokeCallback();
+                return true;
+            } else {
+                return false;
+            }
+        }
+
+        private void startRequest(EditorRowRequest request) {
             currentRequest = request;
         }
 
@@ -221,18 +252,6 @@ public class GridConnector extends AbstractHasComponentsConnector implements
             currentRequest.invokeCallback();
             currentRequest = null;
         }
-
-        @Override
-        public void commit(EditorRowRequest request) {
-            // TODO no-op until Vaadin comms implemented
-            request.invokeCallback();
-        }
-
-        @Override
-        public void discard(EditorRowRequest request) {
-            // TODO no-op until Vaadin comms implemented
-            request.invokeCallback();
-        }
     }
 
     /**
diff --git a/server/src/com/vaadin/ui/components/grid/EditorRow.java b/server/src/com/vaadin/ui/components/grid/EditorRow.java
index 2375627b89..af48d773db 100644
--- a/server/src/com/vaadin/ui/components/grid/EditorRow.java
+++ b/server/src/com/vaadin/ui/components/grid/EditorRow.java
@@ -25,6 +25,9 @@ import com.vaadin.data.fieldgroup.FieldGroup;
 import com.vaadin.data.fieldgroup.FieldGroup.BindException;
 import com.vaadin.data.fieldgroup.FieldGroup.CommitException;
 import com.vaadin.data.fieldgroup.FieldGroupFieldFactory;
+import com.vaadin.server.ClientConnector;
+import com.vaadin.server.ErrorEvent;
+import com.vaadin.server.ErrorHandler;
 import com.vaadin.ui.Field;
 
 /**
@@ -38,6 +41,9 @@ public class EditorRow implements Serializable {
     private Grid grid;
 
     private FieldGroup fieldGroup = new FieldGroup();
+
+    private ErrorHandler errorHandler;
+
     private Object editedItemId = null;
 
     private HashSet<Object> uneditableProperties = new HashSet<Object>();
@@ -112,6 +118,34 @@ public class EditorRow implements Serializable {
     }
 
     /**
+     * Returns the error handler of this editor row.
+     * 
+     * @return the error handler or null if there is no dedicated error handler
+     * 
+     * @see #setErrorHandler(ErrorHandler)
+     * @see ClientConnector#getErrorHandler()
+     */
+    public ErrorHandler getErrorHandler() {
+        return errorHandler;
+    }
+
+    /**
+     * Sets the error handler for this editor row. The error handler is invoked
+     * for exceptions thrown while processing client requests; specifically when
+     * {@link #commit()} triggered by the client throws a CommitException. If
+     * the error handler is not set, one is looked up via Grid.
+     *
+     * @param errorHandler
+     *            the error handler to use
+     * 
+     * @see ClientConnector#setErrorHandler(ErrorHandler)
+     * @see ErrorEvent#findErrorHandler(ClientConnector)
+     */
+    public void setErrorHandler(ErrorHandler errorHandler) {
+        this.errorHandler = errorHandler;
+    }
+
+    /**
      * Builds a field using the given caption and binds it to the given property
      * id using the field binder. Ensures the new field is of the given type.
      * <p>
diff --git a/server/src/com/vaadin/ui/components/grid/Grid.java b/server/src/com/vaadin/ui/components/grid/Grid.java
index 4285b926c9..d4f9c27e6a 100644
--- a/server/src/com/vaadin/ui/components/grid/Grid.java
+++ b/server/src/com/vaadin/ui/components/grid/Grid.java
@@ -38,6 +38,7 @@ import com.vaadin.data.Container.Sortable;
 import com.vaadin.data.RpcDataProviderExtension;
 import com.vaadin.data.RpcDataProviderExtension.DataProviderKeyMapper;
 import com.vaadin.data.util.IndexedContainer;
+import com.vaadin.server.ErrorHandler;
 import com.vaadin.server.KeyMapper;
 import com.vaadin.shared.ui.grid.EditorRowClientRpc;
 import com.vaadin.shared.ui.grid.EditorRowServerRpc;
@@ -417,14 +418,51 @@ public class Grid extends AbstractComponent implements SelectionChangeNotifier,
 
             @Override
             public void bind(int rowIndex) {
-                getEditorRow().internalEditItem(
-                        datasource.getIdByIndex(rowIndex));
-                getEditorRowRpc().confirmBind();
+                try {
+                    getEditorRow().internalEditItem(
+                            datasource.getIdByIndex(rowIndex));
+                    getEditorRowRpc().confirmBind();
+                } catch (Exception e) {
+                    handleError(e);
+                }
             }
 
             @Override
             public void cancel(int rowIndex) {
-                getEditorRow().internalCancel();
+                try {
+                    // For future proofing even though cannot currently fail
+                    getEditorRow().internalCancel();
+                } catch (Exception e) {
+                    handleError(e);
+                }
+            }
+
+            @Override
+            public void commit(int rowIndex) {
+                try {
+                    getEditorRow().commit();
+                    getEditorRowRpc().confirmCommit();
+                } catch (Exception e) {
+                    handleError(e);
+                }
+            }
+
+            @Override
+            public void discard(int rowIndex) {
+                try {
+                    getEditorRow().discard();
+                } catch (Exception e) {
+                    handleError(e);
+                }
+            }
+
+            private void handleError(Exception e) {
+                ErrorHandler handler = getEditorRow().getErrorHandler();
+                if (handler == null) {
+                    handler = com.vaadin.server.ErrorEvent
+                            .findErrorHandler(Grid.this);
+                }
+                handler.error(new ConnectorErrorEvent(Grid.this, e));
             }
         });
     }
diff --git a/shared/src/com/vaadin/shared/ui/grid/EditorRowClientRpc.java b/shared/src/com/vaadin/shared/ui/grid/EditorRowClientRpc.java
index 1c99a5035d..d6822cfccc 100644
--- a/shared/src/com/vaadin/shared/ui/grid/EditorRowClientRpc.java
+++ b/shared/src/com/vaadin/shared/ui/grid/EditorRowClientRpc.java
@@ -34,6 +34,14 @@ public interface EditorRowClientRpc extends ClientRpc {
     void bind(int rowIndex);
 
     /**
+     * Tells the client to discard unsaved changes in the editor row.
+     * 
+     * @param rowIndex
+     *            the index of the edited row
+     */
+    void discard(int rowIndex);
+
+    /**
      * Tells the client to cancel editing and hide the editor row.
      * 
      * @param rowIndex
@@ -46,4 +54,10 @@ public interface EditorRowClientRpc extends ClientRpc {
      * by the client.
      */
     void confirmBind();
+
+    /**
+     * Confirms a pending {@link EditorRowServerRpc#commit(int) commit request}
+     * sent by the client.
+     */
+    void confirmCommit();
 }
diff --git a/shared/src/com/vaadin/shared/ui/grid/EditorRowServerRpc.java b/shared/src/com/vaadin/shared/ui/grid/EditorRowServerRpc.java
index 45705ca924..25bafe2da1 100644
--- a/shared/src/com/vaadin/shared/ui/grid/EditorRowServerRpc.java
+++ b/shared/src/com/vaadin/shared/ui/grid/EditorRowServerRpc.java
@@ -37,6 +37,25 @@ public interface EditorRowServerRpc extends ServerRpc {
     void bind(int rowIndex);
 
     /**
+     * Asks the server to commit unsaved changes in the editor row to the data
+     * source. When a commit request is sent, it must be acknowledged with a
+     * {@link EditorRowClientRpc#confirmCommit() confirm call}.
+     * 
+     * @param rowIndex
+     *            the index of the edited row
+     */
+    void commit(int rowIndex);
+
+    /**
+     * Asks the server to replace any unsaved changes with values from the data
+     * source.
+     * 
+     * @param rowIndex
+     *            the index of the edited row
+     */
+    void discard(int rowIndex);
+
+    /**
      * Tells the server to cancel editing. When sending a cancel request, the
      * client does not need to wait for confirmation by the server before hiding
      * the editor row.
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
index dc5b48107e..afa51644eb 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/GridBasicFeatures.java
@@ -28,6 +28,7 @@ import java.util.Random;
 import com.vaadin.data.Container.Filter;
 import com.vaadin.data.Item;
 import com.vaadin.data.Property;
+import com.vaadin.data.fieldgroup.FieldGroup.CommitException;
 import com.vaadin.data.util.IndexedContainer;
 import com.vaadin.shared.ui.grid.GridStaticCellType;
 import com.vaadin.shared.ui.grid.HeightMode;
@@ -718,7 +719,23 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
                         c.getEditorRow().editItem(100);
                     }
                 }, null);
-
+        createClickAction("Commit", "Editor row", new Command<Grid, String>() {
+            @Override
+            public void execute(Grid c, String value, Object data) {
+                try {
+                    c.getEditorRow().commit();
+                } catch (CommitException e) {
+                    // TODO Auto-generated catch block
+                    e.printStackTrace();
+                }
+            }
+        }, null);
+        createClickAction("Discard", "Editor row", new Command<Grid, String>() {
+            @Override
+            public void execute(Grid c, String value, Object data) {
+                c.getEditorRow().discard();
+            }
+        }, null);
         createClickAction("Cancel edit", "Editor row",
                 new Command<Grid, String>() {
                     @Override
@@ -726,7 +743,6 @@ public class GridBasicFeatures extends AbstractComponentTest<Grid> {
                         c.getEditorRow().cancel();
                     }
                 }, null);
-
     }
 
     @SuppressWarnings("boxing")
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridEditorRowTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridEditorRowTest.java
index 7ca3894060..8fd1decc64 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridEditorRowTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridEditorRowTest.java
@@ -116,4 +116,38 @@ public class GridEditorRowTest extends GridBasicFeaturesTest {
         assertEquals("(100, 2)", widgets.get(2).getAttribute("value"));
         assertEquals("<b>100</b>", widgets.get(9).getAttribute("value"));
     }
+
+    @Test
+    public void testCommit() {
+        selectMenuPath("Component", "Editor row", "Edit item 100");
+
+        List<WebElement> widgets = getEditorRow().findElements(
+                By.className("v-textfield"));
+
+        widgets.get(0).click();
+
+        widgets.get(0).sendKeys(" changed");
+
+        WebElement saveButton = getEditorRow().findElement(
+                By.className("v-editor-row-save"));
+
+        saveButton.click();
+
+        assertEquals("(100, 0) changed", getGridElement().getCell(100, 0)
+                .getText());
+    }
+
+    @Test
+    public void testDiscard() {
+        selectMenuPath("Component", "Editor row", "Edit item 100");
+
+        List<WebElement> widgets = getEditorRow().findElements(
+                By.className("v-textfield"));
+
+        widgets.get(0).sendKeys(" changed");
+
+        selectMenuPath("Component", "Editor row", "Discard");
+
+        assertEquals("(100, 0)", getGridElement().getCell(100, 0).getText());
+    }
 }