Move notify to AlertSubscription; adjust email destination to take multiple addresses

redash/destinations/email.py

@@ -9,14 +9,22 @@ class Email(BaseDestination):
 
     @classmethod
     def configuration_schema(cls):
-        return {}
+        return {
+            "type": "object",
+            "properties": {
+                "addresses": {
+                    "type": "string"
+                },
+            },
+            "required": ["addresses"]
+        }
 
     @classmethod
     def icon(cls):
         return 'fa-envelope'
 
     def notify(self, alert, query, user, new_state, app, host, options):
-        recipients = [user.email]
+        recipients = [email for email in options.get('addresses').split(',') if email]
         html = """
         Check <a href="{host}/alerts/{alert_id}">alert</a> / check <a href="{host}/queries/{query_id}">query</a>.
         """.format(host=host, alert_id=alert.id, query_id=query.id)

redash/handlers/alerts.py

@@ -38,9 +38,8 @@ class AlertResource(BaseResource):
 class AlertListResource(BaseResource):
     def post(self):
         req = request.get_json(True)
-        require_fields(req, ('options', 'name', 'query_id', 'destination_id'))
+        require_fields(req, ('options', 'name', 'query_id'))
 
-        destination = models.NotificationDestination.get_by_id_and_org(req['destination_id'], self.current_org)
         query = models.Query.get_by_id_and_org(req['query_id'], self.current_org)
         require_access(query.groups, self.current_user, view_only)
 
@@ -58,17 +57,6 @@ class AlertListResource(BaseResource):
             'object_type': 'alert'
         })
 
-        # TODO: should be in model?
-        models.AlertSubscription.create(alert=alert, user=self.current_user,
-                                        destination=destination)
-
-        self.record_event({
-            'action': 'subscribe',
-            'timestamp': int(time.time()),
-            'object_id': alert.id,
-            'object_type': 'alert'
-        })
-
         return alert.to_dict()
 
     @require_permission('list_alerts')
@@ -79,14 +67,17 @@ class AlertListResource(BaseResource):
 class AlertSubscriptionListResource(BaseResource):
     def post(self, alert_id):
         req = request.get_json(True)
-        require_fields(req, ('destination_id',))
 
-        destination = models.NotificationDestination.get_by_id_and_org(req['destination_id'], self.current_org)
         alert = models.Alert.get_by_id_and_org(alert_id, self.current_org)
         require_access(alert.groups, self.current_user, view_only)
+        kwargs = {'alert': alert, 'user': self.current_user}
+
+        if 'destination_id' in req:
+            destination = models.NotificationDestination.get_by_id_and_org(req['destination_id'], self.current_org)
+            kwargs['destination'] = destination
+
+        subscription = models.AlertSubscription.create(**kwargs)
 
-        subscription = models.AlertSubscription.create(alert=alert_id, user=self.current_user,
-                                                       destination=destination)
         self.record_event({
             'action': 'subscribe',
             'timestamp': int(time.time()),

redash/models.py

@@ -1100,7 +1100,7 @@ class NotificationDestination(BelongsToOrgMixin, BaseModel):
 
 class AlertSubscription(ModelTimestampsMixin, BaseModel):
     user = peewee.ForeignKeyField(User)
-    destination = peewee.ForeignKeyField(NotificationDestination, default=1)
+    destination = peewee.ForeignKeyField(NotificationDestination, null=True)
     alert = peewee.ForeignKeyField(Alert, related_name="subscriptions")
 
     class Meta:
@@ -1126,6 +1126,19 @@ class AlertSubscription(ModelTimestampsMixin, BaseModel):
     def all(cls, alert_id):
         return AlertSubscription.select(AlertSubscription, User).join(User).where(AlertSubscription.alert==alert_id)
 
+    def notify(self, alert, query, user, new_state, app, host):
+        if self.destination:
+            return self.destination.notify(alert, query, user, new_state,
+                                           app, host)
+        else:
+            # User email subscription, so create an email destination object
+            config = {'email': self.user.email}
+            schema = get_configuration_schema_for_destination_type('email')
+            options = ConfigurationContainer(json.dumps(config), schema)
+            destination = get_destination('email', options)
+            return destination.notify(alert, query, user, new_state,
+                                           app, host, options)
+
 
 all_models = (Organization, Group, DataSource, DataSourceGroup, User, QueryResult, Query, Alert, Dashboard, Visualization, Widget, Event, NotificationDestination, AlertSubscription)
 

redash/tasks.py

@@ -361,7 +361,7 @@ def check_alerts_for_query(self, query_id):
             
             for subscription in alert.subscriptions:
                 try:
-                    subscription.destination.notify(alert, query, subscription.user, new_state, app, host) 
+                    subscription.notify(alert, query, subscription.user, new_state, app, host) 
                 except Exception as e:
                     logger.warn("Exception: {}".format(e))