From a860100156108253b27a7751106ee7044256d6c2 Mon Sep 17 00:00:00 2001
From: Aurélien Bompard <aurelien@bompard.org>
Date: Nov 02 2017 11:16:37 +0000
Subject: Handle widget configuration errors better


---

diff --git a/hubs/static/client/app/components/AddWidget/AddWidgetDialog.js b/hubs/static/client/app/components/AddWidget/AddWidgetDialog.js
index efce753..83247e2 100644
--- a/hubs/static/client/app/components/AddWidget/AddWidgetDialog.js
+++ b/hubs/static/client/app/components/AddWidget/AddWidgetDialog.js
@@ -76,8 +76,19 @@ class AddWidgetDialog extends React.Component {
       this.state.selectedWidget.name,
       this.state.widgetConfig,
       this.props.position)
+    ).then(
+      result => {
+        this.props.handleDialogCloseClicked();
+      },
+      error => {
+        this.setState((prevState, props) => ({
+          selectedWidget: {
+            ...prevState.selectedWidget,
+            configError: error.message,
+          }
+        }));
+      }
     );
-    this.props.handleDialogCloseClicked(e);
   }
 
   render() {
diff --git a/hubs/static/client/app/components/SimpleWidgetConfig.js b/hubs/static/client/app/components/SimpleWidgetConfig.js
index 24d8e24..8c88482 100644
--- a/hubs/static/client/app/components/SimpleWidgetConfig.js
+++ b/hubs/static/client/app/components/SimpleWidgetConfig.js
@@ -39,6 +39,11 @@ export default class SimpleWidgetConfig extends React.Component {
           :
           fields
         }
+        { this.props.widget.configError &&
+          <div className="alert alert-danger my-2">
+            { this.props.widget.configError }
+          </div>
+        }
       </div>
     );
   }
diff --git a/hubs/static/client/app/core/actions/widget.js b/hubs/static/client/app/core/actions/widget.js
index e7784a4..c9da158 100644
--- a/hubs/static/client/app/core/actions/widget.js
+++ b/hubs/static/client/app/core/actions/widget.js
@@ -71,8 +71,7 @@ export function saveConfig(widgetId, config) {
         return dispatch(putConfigSucceeded(widgetId));
       },
       error => {
-        dispatch(addFlashMessage(error.message, "error"));
-        return putConfigFailure(error.message);
+        return dispatch(putConfigFailure(widgetId, error.message));
       })
   }
 }
@@ -137,8 +136,8 @@ export function addWidget(name, config, position) {
         return dispatch(fetchWidgets());
       },
       error => {
-        dispatch(addFlashMessage(error.message, "error"));
-        return widgetAddFailure(error.message);
+        dispatch(widgetAddFailure(error.message));
+        throw error;
       })
   }
 }
diff --git a/hubs/static/client/app/core/reducers/widget.js b/hubs/static/client/app/core/reducers/widget.js
index 524e024..1277ba8 100644
--- a/hubs/static/client/app/core/reducers/widget.js
+++ b/hubs/static/client/app/core/reducers/widget.js
@@ -22,6 +22,7 @@ export default function singleWidgetReducer(state, action) {
         ...state,
         // Optimism failed... ;-(
         config: state.old.config,
+        configError: action.message,
       };
     default:
       return state
diff --git a/hubs/tests/test_widget_validators.py b/hubs/tests/test_widget_validators.py
index 96d887f..77a2a54 100644
--- a/hubs/tests/test_widget_validators.py
+++ b/hubs/tests/test_widget_validators.py
@@ -17,7 +17,9 @@ class ValidatorsTest(APPTest):
 
     def test_integer(self):
         self.assertEqual(validators.Integer("1"), 1)
-        self.assertRaises(ValueError, validators.Integer, "text")
+        with self.assertRaises(ValueError) as cm:
+            validators.Integer("text")
+        self.assertEqual(str(cm.exception), "text is not an integer")
 
     @unittest.skip("Not implemented yet")
     def test_link(self):
diff --git a/hubs/tests/views/test_api_hub_widget.py b/hubs/tests/views/test_api_hub_widget.py
index 5684d29..c5e5689 100644
--- a/hubs/tests/views/test_api_hub_widget.py
+++ b/hubs/tests/views/test_api_hub_widget.py
@@ -177,6 +177,41 @@ class TestAPIHubWidgets(APPTest):
                 Widget.plugin == "about",
             ).count(), 2)
 
+    def test_post_invalid_config(self):
+        self.assertEqual(
+            Widget.query.join(Hub).filter(
+                Hub.name == "ralph",
+                Widget.plugin == "meetings",
+            ).count(), 1)
+        data = {
+            "name": "meetings",
+            "config": {
+                'calendar': 'infra',
+                'n_meetings': 'NOT-A-NUMBER',
+                },
+            'position': 'right',
+            }
+        user = FakeAuthorization('ralph')
+        with auth_set(app, user):
+            result = self.app.post(
+                '/api/hubs/ralph/widgets/',
+                content_type="application/json",
+                data=json.dumps(data))
+        self.assertEqual(result.status_code, 200)
+        self.assertEqual(
+            json.loads(result.get_data(as_text=True)),
+            {
+                "status": "ERROR",
+                "message": (
+                    "Invalid data provided: NOT-A-NUMBER is not an integer"
+                ),
+             })
+        self.assertEqual(
+            Widget.query.join(Hub).filter(
+                Hub.name == "ralph",
+                Widget.plugin == "meetings",
+            ).count(), 1)
+
 
 class TestAPIHubWidget(APPTest):
 
diff --git a/hubs/utils/views.py b/hubs/utils/views.py
index 0fe1e2b..d95d8d7 100644
--- a/hubs/utils/views.py
+++ b/hubs/utils/views.py
@@ -97,12 +97,7 @@ def create_widget_instance(hub, widget, position, config=None):
         left=(position == 'left'))
     flask.g.db.add(widget_instance)
     flask.g.db.flush()  # will populate Widget.config
-    try:
-        configure_widget_instance(widget_instance, config)
-    except WidgetConfigError as e:
-        flask.flash(e.args[0], "error")
-    else:
-        flask.flash("The widget has been added.")
+    configure_widget_instance(widget_instance, config)
     # reorder widgets
     reorder_widgets(hub, widget_instance.left)
     return widget_instance
@@ -128,7 +123,7 @@ def configure_widget_instance(widget_instance, widget_config):
     try:
         config = widget_instance.module.validate_parameters(values)
     except ValueError as err:
-        raise WidgetConfigError('Invalid data provided, error: %s' % err)
+        raise WidgetConfigError('Invalid data provided: %s' % err)
     # Updating in-place is not supported, it's a class property.
     cur_config = widget_instance.config or {}
     cur_config = cur_config.copy()
diff --git a/hubs/views/api/hub_widget.py b/hubs/views/api/hub_widget.py
index 3ef777d..c818cc4 100644
--- a/hubs/views/api/hub_widget.py
+++ b/hubs/views/api/hub_widget.py
@@ -50,7 +50,11 @@ def api_hub_widgets(hub):
         if widget_name not in available_widgets:
             flask.abort(400, 'Unknown widget')
         widget = registry[widget_name]
-        create_widget_instance(hub, widget, position, widget_config)
+        try:
+            create_widget_instance(hub, widget, position, widget_config)
+        except WidgetConfigError as e:
+            flask.g.db.rollback()
+            return flask.jsonify({"status": "ERROR", "message": e.args[0]})
         flask.g.db.commit()
         return flask.jsonify({"status": "OK"})
     user_can_config = hub.allows(user, "config")
diff --git a/hubs/widgets/validators.py b/hubs/widgets/validators.py
index 47f185a..ef19ad0 100644
--- a/hubs/widgets/validators.py
+++ b/hubs/widgets/validators.py
@@ -41,7 +41,10 @@ def Text(value):
 
 def Integer(value):
     """Raises an error if the value can't be converted to an integer."""
-    return int(value)
+    try:
+        return int(value)
+    except ValueError:
+        raise ValueError("{} is not an integer".format(value))
 
 
 def Link(value):

