From 5db6363cbc756c6d3b939e3ab7e8a8b68f7cb239 Mon Sep 17 00:00:00 2001
From: Aurélien Bompard <aurelien@bompard.org>
Date: Nov 23 2017 14:43:44 +0000
Subject: Halp widget: only allow following existing hubs


Fixes: #443

---

diff --git a/hubs/static/client/app/components/CompletionInput.js b/hubs/static/client/app/components/CompletionInput.js
index a90cd07..e5b13c9 100644
--- a/hubs/static/client/app/components/CompletionInput.js
+++ b/hubs/static/client/app/components/CompletionInput.js
@@ -80,19 +80,19 @@ export default class CompletionInput extends React.Component {
       data: {q: value},
       dataType: 'json',
       cache: false,
-      success: function(result) {
+      success: (result) => {
         var suggestions = result.data;
         if (this.props.filterResults) {
           suggestions = suggestions.filter(this.props.filterResults);
         }
         this.setState({suggestions: suggestions});
-      }.bind(this),
-      error: function(xhr, status, err) {
+      },
+      error: (xhr, status, err) => {
         console.error(this.props.url, status, err.toString());
-      }.bind(this),
-      complete: function(xhr, status) {
+      },
+      complete: (xhr, status) => {
         this.setState({isLoading: false});
-      }.bind(this)
+      }
     });
   }
 
diff --git a/hubs/static/client/app/widgets/halp/Config.js b/hubs/static/client/app/widgets/halp/Config.js
index 4c61e76..a19beeb 100644
--- a/hubs/static/client/app/widgets/halp/Config.js
+++ b/hubs/static/client/app/widgets/halp/Config.js
@@ -68,7 +68,7 @@ const messages = defineMessages({
 });
 
 
-export default class Config extends React.Component {
+class Config extends React.Component {
 
   constructor(props) {
     super(props);
@@ -130,20 +130,6 @@ export default class Config extends React.Component {
         );
     });
 
-    /*
-    const IntlCompletionInput = injectIntl((props) => (
-        <CompletionInput
-          inputProps={{
-            name: "hub-add",
-            placeholder: props.intl.formatMessage(messages.placeholder),
-          }}
-          url={props.widget.urls.allHubs}
-          onChange={this.handleHubSelected}
-          />
-    ));
-              //<IntlCompletionInput {...this.props} />
-    */
-
     return (
       <div className="row widget-halp-config">
         <div className="col-sm-4 hubs-list">
@@ -162,12 +148,12 @@ export default class Config extends React.Component {
           <FormattedMessage {...messages.main_title} tagName="h4" />
           <FormattedMessage {...messages.main_subtitle} tagName="p" />
           <form onSubmit={this.handleAddClicked}>
-            <fieldset className="row">
+            <div className="form-row">
               <div className="col-sm-9">
                 <CompletionInput
                   inputProps={{
                     name: "hub-add",
-                    placeholder: "Enter team name here...",
+                    placeholder: this.props.intl.formatMessage(messages.placeholder),
                     value: this.state.selectedHub || "",
                   }}
                   url={this.props.widget.urls.allHubs}
@@ -179,28 +165,33 @@ export default class Config extends React.Component {
                   <FormattedMessage {...messages.add} />
                 </button>
               </div>
-            </fieldset>
+            </div>
           </form>
-          <fieldset className="row">
-            <label className="col-sm-6 form-control-label">
+          <div className="mt-3 form-inline">
+            <label className="form-control-label mr-2">
               <FormattedMessage {...messages.per_page} />
             </label>
-            <div className="col-sm-6">
               <input
                 name="per_page"
                 type="number"
                 value={this.props.values.per_page || "5"}
                 onChange={this.handlePerPageChanged}
-                className="form-control form-control-sm"
+                className="form-control form-control-sm col-2"
                 />
+          </div>
+          <div className="text-muted">
+            <FormattedMessage {...messages.per_page_help} tagName="small" />
+          </div>
+          { this.props.widget.configError &&
+            <div className="alert alert-warning my-2">
+              { this.props.widget.configError }
             </div>
-            <div className="col-sm-12 text-muted">
-              <FormattedMessage {...messages.per_page_help} tagName="small" />
-            </div>
-          </fieldset>
+          }
         </div>
       </div>
     );
   }
-
 }
+
+
+export default injectIntl(Config);
diff --git a/hubs/tests/widgets/test_halp.py b/hubs/tests/widgets/test_halp.py
index 004de8c..4690570 100644
--- a/hubs/tests/widgets/test_halp.py
+++ b/hubs/tests/widgets/test_halp.py
@@ -4,6 +4,7 @@ import json
 
 from hubs.app import app
 from hubs.models import Hub
+from hubs.utils.views import configure_widget_instance, WidgetConfigError
 from hubs.tests import widget_instance
 from hubs.widgets import registry
 from . import WidgetTest
@@ -147,7 +148,7 @@ class HalpViewsTestCase(WidgetTest):
             'name': 'halp',
             'params': [
                 {'default': [],
-                 'help': 'A comma-separated list of hubs to monitor.',
+                 'help': 'A list of hubs to monitor.',
                  'label': 'Hubs',
                  'name': 'hubs',
                  'renderTag': 'input',
@@ -344,6 +345,16 @@ class HalpViewsTestCase(WidgetTest):
                 },
             })
 
+    def test_follow_non_existing_hub(self):
+        # See #443
+        self.assertRaises(
+            WidgetConfigError,
+            configure_widget_instance,
+            self.widget, dict(hubs=["non-existing"], per_page=4)
+            )
+        self.assertDictEqual(
+            self.widget.config, dict(hubs=["fedora-devel"], per_page=3))
+
 
 class HalpFunctionsTestCase(WidgetTest):
     plugin = 'halp'  # The name in hubs.widgets.registry
diff --git a/hubs/widgets/halp/__init__.py b/hubs/widgets/halp/__init__.py
index 0164582..39732e0 100644
--- a/hubs/widgets/halp/__init__.py
+++ b/hubs/widgets/halp/__init__.py
@@ -7,6 +7,7 @@ import flask
 from hubs.widgets.base import Widget
 from hubs.widgets import validators
 from .views import hubs_suggest_view
+from .utils import listofhubs_validator
 
 
 class Halp(Widget):
@@ -26,8 +27,8 @@ class Halp(Widget):
             name="hubs",
             label="Hubs",
             default=[],
-            validator=validators.CommaSeparatedList,
-            help="A comma-separated list of hubs to monitor.",
+            validator=listofhubs_validator,
+            help="A list of hubs to monitor.",
         ), dict(
             name="per_page",
             label="Requests per page",
diff --git a/hubs/widgets/halp/utils.py b/hubs/widgets/halp/utils.py
index 7f29e0d..23ffec8 100644
--- a/hubs/widgets/halp/utils.py
+++ b/hubs/widgets/halp/utils.py
@@ -20,3 +20,22 @@ def find_hubs_for_msg(msg):
             HubConfig.chat_channel == msg["channel"]
         ).values(Hub.name)
         ]
+
+
+def listofhubs_validator(value):
+    """Fails if the value isn't a list.
+
+    If the value is a string, list, it will be interpreted as a comma-separated
+    list and converted to a Python list. If there is no comma in the original
+    value, it will produce a list with a single element. Whitespaces will be
+    stripped from the elements, so spaces are allowed around the commas.
+    """
+    if not value:
+        return []
+    if not isinstance(value, list):
+        raise ValueError("Expected a list")
+    hub_names = [h.name for h in Hub.query.all()]
+    for hub in value:
+        if hub not in hub_names:
+            raise ValueError("hub \"{}\" does not exist".format(hub))
+    return value