Merge pull request #5560 from Countly/SER-1896-improve-crash-alerts

[alerts] improvements in crashes

Author: ar2rsawseen

plugins/alerts/api/alertModules/crashes.js

@@ -29,22 +29,28 @@ async function triggerByEvent(payload) {
     }
 
     //read batcher reads from db and cashes data for some time
-    const alert = await common.readBatcher.getOne("alerts", {
+    const alerts = await common.readBatcher.getMany("alerts", {
         selectedApps: app._id.toString(),
         alertDataType: "crashes",
         alertDataSubType: commonLib.TRIGGERED_BY_EVENT.crashes,
     });
 
-    if (!alert) {
+    if (!alerts || !alerts.length) {
         return;
     }
 
-    await commonLib.trigger({ alert, app, date: new Date }, log);
+    // trigger all alerts
+    await Promise.all(alerts.map(alert => commonLib.trigger({
+        alert,
+        app,
+        date: new Date,
+        extra: crashObject
+    }, log)));
 }
 
 
 module.exports.check = async function({ alertConfigs: alert, done, scheduledTo: date }) {
-    const app = await common.readBatcher.getOne("apps", { _id: ObjectId(alert.selectedApps[0]) });
+    const app = await common.readBatcher.getOne("apps", { _id: new ObjectId(alert.selectedApps[0]) });
     if (!app) {
         log.e(`App ${alert.selectedApps[0]} couldn't be found`);
         return done();
@@ -57,7 +63,7 @@ module.exports.check = async function({ alertConfigs: alert, done, scheduledTo:
 
     if (compareType === commonLib.COMPARE_TYPE_ENUM.MORE_THAN) {
         if (metricValue > compareValue) {
-            await commonLib.trigger({ alert, app, metricValue, date });
+            await commonLib.trigger({ alert, app, metricValue, date }, log);
         }
     }
     else {
@@ -73,7 +79,7 @@ module.exports.check = async function({ alertConfigs: alert, done, scheduledTo:
             : change <= -compareValue;
 
         if (shouldTrigger) {
-            await commonLib.trigger({ alert, app, date, metricValue, metricValueBefore });
+            await commonLib.trigger({ alert, app, date, metricValue, metricValueBefore }, log);
         }
     }
 

plugins/alerts/api/parts/common-lib.js

@@ -1,3 +1,5 @@
+/** @typedef {import("mongodb").ObjectId} ObjectId */
+
 /**
  * Alert document object
  * @typedef  {Object}        Alert
@@ -30,12 +32,22 @@
 
 /**
  * Paired app&alert and the metrics
- * @typedef  {Object} MatchedResult
- * @property {App}    app               - matched app object
- * @property {Alert}  alert             - matched alert object
- * @property {Date}   date              - alert trigger date
- * @property {number} metricValue       - metric value that matched the alert condition
- * @property {number} metricValueBefore - value that compared with metricValue
+ * @typedef  {Object}  MatchedResult
+ * @property {App}     app               - matched app object
+ * @property {Alert}   alert             - matched alert object
+ * @property {Date}    date              - alert trigger date
+ * @property {number=} metricValue       - metric value that matched the alert condition
+ * @property {number=} metricValueBefore - value that compared with metricValue
+ * @property {any=}    extra             - extra parameters to pass to e-mail builder
+ */
+
+/**
+ * Individual components of a date. modified version of the moment.prototype.toObject()
+ * @typedef  {Object} DateComponents
+ * @property {number} years
+ * @property {number} months
+ * @property {number} days
+ * @property {number} hours
  */
 
 const common = require('../../../../api/utils/common.js');
@@ -85,14 +97,11 @@ module.exports = {
  * Returns a date's components based on timezone.
  * @param   {Date}   date     - date to get its components
  * @param   {string} timezone - timezone string
- * @returns {object}          - { years, months, days, hours }
+ * @returns {DateComponents}  - { years, months, days, hours }
  */
 function getDateComponents(date, timezone) {
-    const dateComponents = moment(date).tz(timezone).toObject();
-    dateComponents.months += 1; // months are zero indexed
-    dateComponents.days = dateComponents.date;
-    delete dateComponents.date;
-    return dateComponents;
+    const { years, months, date: days, hours } = moment(date).tz(timezone).toObject();
+    return { years, months: months + 1, hours, days };
 }
 
 /**
@@ -126,6 +135,7 @@ async function determineAudience(alert) {
         }
         return getUserEmailsBasedOnGroups(alert.allGroups);
     }
+    return [];
 }
 /**
  * Retrieves user emails based on group IDs.
@@ -150,7 +160,7 @@ function getUserEmailsBasedOnGroups(groupIds) {
                     members.forEach((member) => {
                         memberEmails.push(member.email);
                     });
-                    resolve();
+                    resolve(true);
                 }
             });
         });
@@ -166,7 +176,7 @@ function getUserEmailsBasedOnGroups(groupIds) {
  * @returns {Promise<string>}        - compiled e-mail string
  */
 async function compileEmail(result) {
-    const { alert, app, metricValue, metricValueBefore } = result;
+    const { alert, app, metricValue, metricValueBefore, extra } = result;
     const host = await new Promise((res, rej) => mail.lookup((err, _host) => err ? rej(err) : res(_host)));
     const metrics = [];
     if (metricValue) {
@@ -179,14 +189,16 @@ async function compileEmail(result) {
         title: `Countly Alert`,
         alertName: alert.alertName,
         alertDataType: alert.alertDataType,
+        alertDataSubType: alert.alertDataSubType,
         subTitle: `Uh oh! It seems there's been some activity related to ` + alert.alertDataSubType + ` in the `,
         host,
         compareDescribe: alert.compareDescribe,
         apps: [{
             id: app._id.toString(),
             name: app.name,
             data: metrics
-        }]
+        }],
+        extra,
     });
 }
 
@@ -215,7 +227,7 @@ async function trigger(result, log) {
                     email,
                     subject,
                     emailBody,
-                    err => err ? rej(err) : res()
+                    err => err ? rej(err) : res(true)
                 )
             );
             // increase counter by date and email

plugins/alerts/frontend/public/templates/email.html

@@ -133,9 +133,11 @@
                                                     </tr>
                                                     <tr>
                                                         <td align="left" style="font-family: Inter; color: #333C48; font-size: 16px; font-weight:400; line-height: 22px;">
-                                                            Click to access the
-                                                            <span style="display: inline; font-family: Inter; font-weight: 400; font-size: 16px;"><%= alertDataType %></span>
-                                                            results.
+                                                            <% if (alertDataType === "crashes" && alertDataSubType === "new crash/error" && extra && extra._id) { %>
+                                                                <a href="<%= host %>/dashboard#/<%= apps[i].id %>/crashes/<%= extra._id %>">
+                                                                    Click to access the crash details.
+                                                                </a>
+                                                            <% } %>
                                                         </td>
                                                     </tr>
                                                 </table>