Show editor save error (#16602)

Change-Id: I2727a9fabef4291798e97495c2df86b077387cbb

9 files changed, 109 insertions, 35 deletions

diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java
index 60a730c80e..f8aa044a8d 100644
--- a/client/src/com/vaadin/client/connectors/GridConnector.java
+++ b/client/src/com/vaadin/client/connectors/GridConnector.java
@@ -228,13 +228,13 @@ public class GridConnector extends AbstractHasComponentsConnector implements
 
                 @Override
                 public void confirmBind(final boolean bindSucceeded) {
-                    endRequest(bindSucceeded, null);
+                    endRequest(bindSucceeded, null, null);
                 }
 
                 @Override
                 public void confirmSave(boolean saveSucceeded,
-                        List<String> errorColumnsIds) {
-                    endRequest(saveSucceeded, errorColumnsIds);
+                        String errorMessage, List<String> errorColumnsIds) {
+                    endRequest(saveSucceeded, errorMessage, errorColumnsIds);
                 }
             });
         }
@@ -303,7 +303,8 @@ public class GridConnector extends AbstractHasComponentsConnector implements
             currentRequest = request;
         }
 
-        private void endRequest(boolean succeeded, List<String> errorColumnsIds) {
+        private void endRequest(boolean succeeded, String errorMessage,
+                List<String> errorColumnsIds) {
             assert currentRequest != null : "Current request was null";
             /*
              * Clear current request first to ensure the state is valid if
@@ -324,7 +325,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements
                     errorColumns = null;
                 }
 
-                request.failure(errorColumns);
+                request.failure(errorMessage, errorColumns);
             }
         }
     }
diff --git a/client/src/com/vaadin/client/widget/grid/EditorHandler.java b/client/src/com/vaadin/client/widget/grid/EditorHandler.java
index 1d152c708c..970130737c 100644
--- a/client/src/com/vaadin/client/widget/grid/EditorHandler.java
+++ b/client/src/com/vaadin/client/widget/grid/EditorHandler.java
@@ -86,12 +86,16 @@ public interface EditorHandler<T> {
          * Informs Grid that an error occurred while trying to process the
          * request.
          * 
+         * @param errorMessage
+         *            and error message to show to the user, or
+         *            <code>null</code> to not show any message.
          * @param errorColumns
          *            a collection of columns for which an error indicator
          *            should be shown, or <code>null</code> if no columns should
          *            be marked as erroneous.
          */
-        public void failure(Collection<Grid.Column<?, T>> errorColumns);
+        public void failure(String errorMessage,
+                Collection<Grid.Column<?, T>> errorColumns);
 
         /**
          * Checks whether the request is completed or not.
@@ -107,8 +111,8 @@ public interface EditorHandler<T> {
      * <p>
      * The implementation <em>must</em> call either
      * {@link EditorRequest#success()} or
-     * {@link EditorRequest#failure(Collection)} to signal a successful or a
-     * failed (respectively) bind action.
+     * {@link EditorRequest#failure(String, Collection)} to signal a successful
+     * or a failed (respectively) bind action.
      * 
      * @param request
      *            the data binding request
@@ -123,9 +127,10 @@ public interface EditorHandler<T> {
      * <p>
      * In contrast to {@link #bind(EditorRequest)} and
      * {@link #save(EditorRequest)}, any calls to
-     * {@link EditorRequest#success()} or {@link EditorRequest#fail()} have no
-     * effect on the outcome of the cancel action. The editor is already closed
-     * when this method is called.
+     * {@link EditorRequest#success()} or
+     * {@link EditorRequest#failure(String, Collection)} have no effect on the
+     * outcome of the cancel action. The editor is already closed when this
+     * method is called.
      * 
      * @param request
      *            the cancel request
diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java
index 0f3ffc696a..9a6a6d6029 100644
--- a/client/src/com/vaadin/client/widgets/Grid.java
+++ b/client/src/com/vaadin/client/widgets/Grid.java
@@ -998,13 +998,16 @@ public class Grid<T> extends ResizeComposite implements
             return w;
         }
 
-        private void complete(Collection<Column<?, T>> errorColumns) {
+        private void complete(String errorMessage,
+                Collection<Column<?, T>> errorColumns) {
             if (completed) {
                 throw new IllegalStateException(
                         "An EditorRequest must be completed exactly once");
             }
             completed = true;
 
+            grid.getEditor().setErrorMessage(errorMessage);
+
             grid.getEditor().clearEditorColumnErrors();
             if (errorColumns != null) {
                 for (Column<?, T> column : errorColumns) {
@@ -1015,15 +1018,16 @@ public class Grid<T> extends ResizeComposite implements
 
         @Override
         public void success() {
-            complete(null);
+            complete(null, null);
             if (callback != null) {
                 callback.onSuccess(this);
             }
         }
 
         @Override
-        public void failure(Collection<Grid.Column<?, T>> errorColumns) {
-            complete(errorColumns);
+        public void failure(String errorMessage,
+                Collection<Grid.Column<?, T>> errorColumns) {
+            complete(errorMessage, errorColumns);
             if (callback != null) {
                 callback.onError(this);
             }
@@ -1179,6 +1183,17 @@ public class Grid<T> extends ResizeComposite implements
             });
         }
 
+        public void setErrorMessage(String errorMessage) {
+            if (errorMessage == null) {
+                message.removeFromParent();
+            } else {
+                message.setInnerText(errorMessage);
+                if (message.getParentElement() == null) {
+                    messageWrapper.appendChild(message);
+                }
+            }
+        }
+
         public int getRow() {
             return rowIndex;
         }
diff --git a/server/src/com/vaadin/ui/Grid.java b/server/src/com/vaadin/ui/Grid.java
index ef97f9b336..150f672835 100644
--- a/server/src/com/vaadin/ui/Grid.java
+++ b/server/src/com/vaadin/ui/Grid.java
@@ -90,7 +90,6 @@ import com.vaadin.shared.ui.grid.GridStaticSectionState.RowState;
 import com.vaadin.shared.ui.grid.HeightMode;
 import com.vaadin.shared.ui.grid.ScrollDestination;
 import com.vaadin.shared.util.SharedUtil;
-import com.vaadin.ui.Notification.Type;
 import com.vaadin.ui.renderer.Renderer;
 import com.vaadin.ui.renderer.TextRenderer;
 import com.vaadin.util.ReflectTools;
@@ -262,9 +261,12 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
 
         private Set<Column> errorColumns = new HashSet<Column>();
 
+        private String userErrorMessage;
+
         public CommitErrorEvent(Grid grid, CommitException cause) {
             super(grid);
             this.cause = cause;
+            userErrorMessage = cause.getLocalizedMessage();
         }
 
         /**
@@ -310,6 +312,25 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
             return Collections.unmodifiableCollection(errorColumns);
         }
 
+        /**
+         * Gets the error message to show to the user.
+         * 
+         * @return error message to show
+         */
+        public String getUserErrorMessage() {
+            return userErrorMessage;
+        }
+
+        /**
+         * Sets the error message to show to the user.
+         * 
+         * @param userErrorMessage
+         *            the user error message to set
+         */
+        public void setUserErrorMessage(String userErrorMessage) {
+            this.userErrorMessage = userErrorMessage;
+        }
+
     }
 
     /**
@@ -349,12 +370,8 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
                         .getHeaderCaption();
                 String message = invalidFields.get(firstErrorField)
                         .getLocalizedMessage();
-                /*
-                 * TODO This should be shown in the editor component once there
-                 * is a place for that. Optionally, all errors should be shown
-                 */
-                Notification.show(caption + ": " + message, Type.ERROR_MESSAGE);
 
+                event.setUserErrorMessage(caption + ": " + message);
             } else {
                 com.vaadin.server.ErrorEvent.findErrorHandler(Grid.this).error(
                         new ConnectorErrorEvent(Grid.this, event.getCause()));
@@ -2536,10 +2553,10 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
          *            {@code true} if this column should be editable,
          *            {@code false} otherwise
          * @return this column
-         *
+         * 
          * @throws IllegalStateException
          *             if the editor is currently active
-         *
+         * 
          * @see Grid#editItem(Object)
          * @see Grid#isEditorActive()
          */
@@ -3056,6 +3073,7 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
             @Override
             public void save(int rowIndex) {
                 List<String> errorColumnIds = null;
+                String errorMessage = null;
                 boolean success = false;
                 try {
                     saveEditor();
@@ -3066,6 +3084,8 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
                                 Grid.this, e);
                         getEditorErrorHandler().commitError(event);
 
+                        errorMessage = event.getUserErrorMessage();
+
                         errorColumnIds = new ArrayList<String>();
                         for (Column column : event.getErrorColumns()) {
                             errorColumnIds.add(column.state.id);
@@ -3078,7 +3098,8 @@ public class Grid extends AbstractComponent implements SelectionNotifier,
                 } catch (Exception e) {
                     handleError(e);
                 }
-                getEditorRpc().confirmSave(success, errorColumnIds);
+                getEditorRpc().confirmSave(success, errorMessage,
+                        errorColumnIds);
             }
 
             private void handleError(Exception e) {
diff --git a/shared/src/com/vaadin/shared/ui/grid/EditorClientRpc.java b/shared/src/com/vaadin/shared/ui/grid/EditorClientRpc.java
index 6fb0b7a069..61066f28c2 100644
--- a/shared/src/com/vaadin/shared/ui/grid/EditorClientRpc.java
+++ b/shared/src/com/vaadin/shared/ui/grid/EditorClientRpc.java
@@ -58,9 +58,12 @@ public interface EditorClientRpc extends ClientRpc {
      * 
      * @param saveSucceeded
      *            <code>true</code> iff the save action was successful
+     * @param errorMessage
+     *            the error message to show the user
      * @param errorColumnsIds
      *            a list of column keys that should get error markers, or
      *            <code>null</code> if there should be no error markers
      */
-    void confirmSave(boolean saveSucceeded, List<String> errorColumnsIds);
+    void confirmSave(boolean saveSucceeded, String errorMessage,
+            List<String> errorColumnsIds);
 }
diff --git a/uitest/src/com/vaadin/testbench/elements/GridElement.java b/uitest/src/com/vaadin/testbench/elements/GridElement.java
index 5d85de4eb6..e04140a3e0 100644
--- a/uitest/src/com/vaadin/testbench/elements/GridElement.java
+++ b/uitest/src/com/vaadin/testbench/elements/GridElement.java
@@ -129,6 +129,22 @@ public class GridElement extends AbstractComponentElement {
         public void cancel() {
             findElement(By.className("v-grid-editor-cancel")).click();
         }
+
+        /**
+         * Gets the error message text, or <code>null</code> if no message is
+         * present.
+         */
+        public String getErrorMessage() {
+            WebElement messageWrapper = findElement(By
+                    .className("v-grid-editor-message"));
+            List<WebElement> divs = messageWrapper.findElements(By
+                    .tagName("div"));
+            if (divs.isEmpty()) {
+                return null;
+            } else {
+                return divs.get(0).getText();
+            }
+        }
     }
 
     /**
diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridEditorClientTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridEditorClientTest.java
index 2d8c9eb763..04f5e1558d 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridEditorClientTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/client/GridEditorClientTest.java
@@ -33,6 +33,7 @@ import org.openqa.selenium.WebElement;
 import org.openqa.selenium.interactions.Actions;
 
 import com.vaadin.shared.ui.grid.GridConstants;
+import com.vaadin.testbench.elements.GridElement.GridEditorElement;
 import com.vaadin.tests.components.grid.basicfeatures.GridBasicClientFeaturesTest;
 import com.vaadin.tests.components.grid.basicfeatures.GridBasicFeatures;
 
@@ -199,13 +200,23 @@ public class GridEditorClientTest extends GridBasicClientFeaturesTest {
     public void testErrorField() {
         selectMenuPath(EDIT_ROW_5);
 
+        GridEditorElement editor = getGridElement().getEditor();
+
         assertTrue("No errors should be present",
-                getEditor().findElements(By.className("error")).isEmpty());
+                editor.findElements(By.className("error")).isEmpty());
+        assertEquals("No error message should be present", null,
+                editor.getErrorMessage());
+
         selectMenuPath("Component", "Editor", "Toggle second editor error");
         getSaveButton().click();
 
-        assertEquals("Unexpected amount of error fields", 1, getEditor()
+        assertEquals("Unexpected amount of error fields", 1, editor
                 .findElements(By.className("error")).size());
+        assertEquals(
+                "Unexpedted error message",
+                "Syntethic fail of editor in column 2. "
+                        + "This message is so long that it doesn't fit into its box",
+                editor.getErrorMessage());
     }
 
     protected WebElement getSaveButton() {
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 e1567b4286..f37a94358a 100644
--- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java
+++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/server/GridEditorTest.java
@@ -38,7 +38,6 @@ 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;
-import com.vaadin.tests.tb3.newelements.FixedNotificationElement;
 
 public class GridEditorTest extends GridBasicFeaturesTest {
 
@@ -220,10 +219,9 @@ public class GridEditorTest extends GridBasicFeaturesTest {
         intField.clear();
         intField.sendKeys("banana phone");
         editor.save();
-        FixedNotificationElement n = $(FixedNotificationElement.class).first();
+
         assertEquals("Column 7: Could not convert value to Integer",
-                n.getCaption());
-        n.close();
+                editor.getErrorMessage());
         assertTrue("Field 7 should have been marked with an error after error",
                 editor.isFieldErrorMarked(7));
         editor.cancel();
@@ -231,6 +229,8 @@ public class GridEditorTest extends GridBasicFeaturesTest {
         selectMenuPath(EDIT_ITEM_100);
         assertFalse("Exception should not exist",
                 isElementPresent(NotificationElement.class));
+        assertEquals("There should be no editor error message", null,
+                editor.getErrorMessage());
     }
 
     @Test
diff --git a/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java b/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java
index 7509054957..232a3a780e 100644
--- a/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java
+++ b/uitest/src/com/vaadin/tests/widgetset/client/grid/GridBasicClientFeaturesWidget.java
@@ -125,9 +125,11 @@ public class GridBasicClientFeaturesWidget extends
         @Override
         public void save(EditorRequest<List<Data>> request) {
             if (secondEditorError) {
-                log.setText("Syntethic fail of editor in column 2");
-                request.failure(Collections.<Column<?, List<Data>>> singleton(grid
-                        .getColumn(2)));
+                request.failure(
+                        "Syntethic fail of editor in column 2. "
+                                + "This message is so long that it doesn't fit into its box",
+                        Collections.<Column<?, List<Data>>> singleton(grid
+                                .getColumn(2)));
                 return;
             }
             try {
@@ -153,7 +155,7 @@ public class GridBasicClientFeaturesWidget extends
                 request.success();
             } catch (Exception e) {
                 Logger.getLogger(getClass().getName()).warning(e.toString());
-                request.failure(null);
+                request.failure(null, null);
             }
         }