Merge pull request #28 from apiaryio/miiila/bug-with-false-alarm-on-w…

…eekend Bug with false alarm on weekend
apiaryio · Oct 2, 2017 · 137d125 · 137d125
2 parents 844fbb9 + 6740fb7
commit 137d125
Show file tree

Hide file tree

Showing 12 changed files with 274 additions and 52 deletions.
diff --git a/package.json b/package.json
@@ -29,6 +29,7 @@
     "commander": "2.9.0",
     "debug": "2.6.7",
     "fs-access": "1.0.0",
+    "moment": "^2.18.1",
     "nconf": "0.8.4",
     "node-slackr": "0.1.3",
     "request": "2.81.0",

diff --git a/src/notify.js b/src/notify.js
@@ -3,6 +3,10 @@ const async = require('async');
 const debug = require('debug')('pagerduty-overrides:notifications');
 const pdApi = require('./pagerduty-api');
 
+function toUTCString(moment) {
+  return moment.format('ddd, D MMM YYYY HH:mm:ss z');
+}
+
 function createPagerDutyIncident(options, message, cb) {
   debug(`Creating PD incident ${JSON.stringify(message)} with options ${JSON.stringify(options)}`);
 
@@ -91,13 +95,13 @@ function formatMessage(messages, option = 'plain') {
     case 'plain':
       outputMessage = '_Following overlaps found:_\n';
       messages.forEach((message) => {
-        outputMessage += `${message.user}: ${message.schedules[0]} and ${message.schedules[1]} (the first starting on ${message.date.toUTCString()}, the second on ${message.crossDate.toUTCString()})\n`;
+        outputMessage += `${message.user}: ${message.schedules[0]} and ${message.schedules[1]} (from ${toUTCString(message.overlapStart)} to ${toUTCString(message.overlapEnd)})\n`;
       });
       break;
     case 'markdown':
       outputMessage = 'Following overlaps found:\n';
       messages.forEach((message) => {
-        outputMessage += `*${message.user}:* \`${message.schedules[0]}\` and \`${message.schedules[1]}\` (the first starting on ${message.date.toUTCString()}, the second on ${message.crossDate.toUTCString()})\n`;
+        outputMessage += `*${message.user}:* \`${message.schedules[0]}\` and \`${message.schedules[1]}\` (from ${toUTCString(message.overlapStart)} to ${toUTCString(message.overlapEnd)})\n`;
       });
       break;
     case 'json':
@@ -106,7 +110,7 @@ function formatMessage(messages, option = 'plain') {
         if (acc[curr.userId].userId == null) { acc[curr.userId].userId = curr.userId; }
         if (acc[curr.userId].user == null) { acc[curr.userId].user = curr.user; }
         if (acc[curr.userId].messages == null) { acc[curr.userId].messages = []; }
-        acc[curr.userId].messages.push(`${curr.schedules[0]} and ${curr.schedules[1]} (the first starting on ${curr.date.toUTCString()}, the second on ${curr.crossDate.toUTCString()})`);
+        acc[curr.userId].messages.push(`${curr.schedules[0]} and ${curr.schedules[1]} (from ${toUTCString(curr.overlapStart)} to ${toUTCString(curr.overlapEnd)})`);
         return acc;
       }
         , {});
@@ -168,4 +172,5 @@ function send(options, message, cb) {
 
 module.exports = {
   send,
+  toUTCString,
 };
diff --git a/src/pagerduty.js b/src/pagerduty.js
@@ -1,4 +1,5 @@
 const async = require('async');
+const moment = require('moment');
 const nconf = require('nconf');
 const _ = require('underscore');
 const debug = require('debug')('pagerduty-overrides');
@@ -8,15 +9,13 @@ const pdApi = require('./pagerduty-api');
 // Get schedule for ID and 2 weeks
 function getSchedule(id, cb) {
   if (nconf.get('WEEKS_TO_CHECK') > 0) {
-    const week = 7 * 86400 * 1000;
-    const timeNow = new Date();
-    const timeUntil = new Date(timeNow.getTime() + (nconf.get('WEEKS_TO_CHECK') * week));
+    const timeUntil = moment.utc().add(nconf.get('WEEKS_TO_CHECK'), 'w');
 
     const scheduleOpts = {
       qs: {
         'schedule_ids[]': id,
         until: timeUntil.toISOString(),
-        since: timeNow.toISOString(),
+        since: moment.utc().toISOString(),
       },
     };
 
@@ -87,12 +86,6 @@ function sendNotification(options, message, cb) {
   return notify.send(options, message, err => cb(err));
 }
 
-function getDayAbbrev(utcDay) {
-  const days = ['Sun', 'Mon', 'Tue', 'Wed', 'Thu', 'Fri', 'Sat'];
-
-  return days[utcDay];
-}
-
 function processSchedules(allSchedules, days = [], cb) {
   let callback = cb;
   let daysArray = days;
@@ -111,52 +104,60 @@ function processSchedules(allSchedules, days = [], cb) {
     debug('otherSchedules:', JSON.stringify(otherSchedules));
     schedule.entries.forEach((entry) => {
       debug('checking entry: ', JSON.stringify(entry));
-      const myStart = entry.start;
-      const myEnd = entry.end;
+      const myStart = moment.utc(entry.start);
+      const myEnd = moment.utc(entry.end);
       const myUserId = entry.user.id;
       const myUserName = entry.user.summary;
       if (duplicities.myUserName == null) { duplicities.myUserName = []; }
       otherSchedules.forEach((crossSchedule) => {
         crossSchedule.entries.forEach((crossCheckEntry) => {
           let overlap = false;
-          const startDate = new Date(myStart);
-          const day = getDayAbbrev(startDate.getUTCDay());
 
           const scheduleId = nconf.get(`schedulesNames:${schedule.id}`);
           const crossScheduleId = nconf.get(`schedulesNames:${crossSchedule.id}`);
+          const crossCheckStart = moment.utc(crossCheckEntry.start);
+          const crossCheckEnd = moment.utc(crossCheckEntry.end);
+          let message;
 
-          const message = {
-            user: myUserName,
-            userId: myUserId,
-            schedules: [scheduleId, crossScheduleId],
-            date: startDate,
-            crossDate: new Date(crossCheckEntry.start),
-          };
-
-          if ((myStart <= crossCheckEntry.start && crossCheckEntry.start < myEnd) &&
+          // is there an overlap?
+          if ((crossCheckStart < myEnd && myStart < crossCheckEnd) &&
               (crossCheckEntry.user.id === myUserId)) {
-            overlap = true;
-
-            if (Object.keys(daysArray).includes(day)) {
-              if (daysArray[day].start && daysArray[day].end) {
-                const exclusionStartTime = daysArray[day].start.split(':');
-                const exclusionEndTime = daysArray[day].end.split(':');
-                const exclusionStartDate = new Date(myStart);
-                exclusionStartDate.setUTCHours(exclusionStartTime[0]);
-                exclusionStartDate.setUTCMinutes(exclusionStartTime[1]);
-                const exclusionEndDate = new Date(myStart);
-                exclusionEndDate.setUTCHours(exclusionEndTime[0]);
-                exclusionEndDate.setUTCMinutes(exclusionEndTime[1]);
-
-
-                if (exclusionStartDate <= startDate && startDate < exclusionEndDate) {
-                  debug('excluded:', message);
+            // find overlapping inteval
+            const overlapStart = moment.max(myStart, crossCheckStart);
+            const overlapEnd = moment.min(crossCheckEnd, myEnd);
+
+            message = {
+              user: myUserName,
+              userId: myUserId,
+              schedules: [scheduleId, crossScheduleId],
+              overlapStart,
+              overlapEnd,
+            };
+
+            [overlapStart, overlapEnd].forEach((day) => {
+              overlap = true;
+              const overlappingDay = day.format('ddd');
+
+              if (Object.keys(daysArray).includes(overlappingDay)) {
+                if (daysArray[overlappingDay].start && daysArray[overlappingDay].end) {
+                  const exclusionStartTime = daysArray[overlappingDay].start.split(':');
+                  const exclusionEndTime = daysArray[overlappingDay].end.split(':');
+                  const exclusionStartDate = moment.utc(day);
+                  exclusionStartDate.hours(exclusionStartTime[0]);
+                  exclusionStartDate.minutes(exclusionStartTime[1]);
+                  const exclusionEndDate = moment.utc(day);
+                  exclusionEndDate.hours(exclusionEndTime[0]);
+                  exclusionEndDate.minutes(exclusionEndTime[1]);
+
+                  if (day.isBetween(exclusionStartDate, exclusionEndDate)) {
+                    debug('excluded:', message);
+                    overlap = false;
+                  }
+                } else {
                   overlap = false;
                 }
-              } else {
-                overlap = false;
               }
-            }
+            });
           }
 
           if (overlap && !duplicities.myUserName.includes(crossCheckEntry.start)) {

diff --git a/test/fixtures/bug-27-entries-cross.json b/test/fixtures/bug-27-entries-cross.json
@@ -0,0 +1,31 @@
+{
+  "total": null,
+  "oncalls": [
+    {
+      "escalation_policy": {
+        "id": "PIYVU5O",
+        "type": "escalation_policy_reference",
+        "summary": "Escalation Policy eaque ipsam porro",
+        "self": "https://api.pagerduty.com/escalation_policies/PIYVU5O",
+        "html_url": "https://apidocs.pagerduty.com/escalation_policies/PIYVU5O"
+      },
+      "escalation_level": 1,
+      "schedule": {
+        "id": "P538IZH",
+        "type": "schedule_reference",
+        "summary": "Schedule a quasi illum",
+        "self": "https://api.pagerduty.com/schedules/P538IZH",
+        "html_url": "https://apidocs.pagerduty.com/schedules/P538IZH"
+      },
+      "user": {
+        "summary": "Gregory",
+        "id": "PRT2T0A",
+        "type": "user_reference",
+        "self": "https://api.pagerduty.com/escalation_policies/PRT2T0",
+        "html_url": "https://apiary.pagerduty.com/escalation_policies/PRT2T0"
+      },
+      "end": "2012-08-18T00:00:00-04:00",
+      "start": "2012-08-16T16:00:00-04:00"
+    }
+  ]
+}
diff --git a/test/fixtures/bug-27-entries-plus-one-day.json b/test/fixtures/bug-27-entries-plus-one-day.json
@@ -0,0 +1,31 @@
+{
+  "total": null,
+  "oncalls": [
+    {
+      "escalation_policy": {
+        "id": "PIYVU5O",
+        "type": "escalation_policy_reference",
+        "summary": "Escalation Policy eaque ipsam porro",
+        "self": "https://api.pagerduty.com/escalation_policies/PIYVU5O",
+        "html_url": "https://apidocs.pagerduty.com/escalation_policies/PIYVU5O"
+      },
+      "escalation_level": 1,
+      "schedule": {
+        "id": "P538IZH",
+        "type": "schedule_reference",
+        "summary": "Schedule a quasi illum",
+        "self": "https://api.pagerduty.com/schedules/P538IZH",
+        "html_url": "https://apidocs.pagerduty.com/schedules/P538IZH"
+      },
+      "user": {
+        "summary": "Gregory",
+        "id": "PRT2T0A",
+        "type": "user_reference",
+        "self": "https://api.pagerduty.com/escalation_policies/PRT2T0",
+        "html_url": "https://apiary.pagerduty.com/escalation_policies/PRT2T0"
+      },
+      "end": "2012-08-21T00:00:00-04:00",
+      "start": "2012-08-17T16:00:00-04:00"
+    }
+  ]
+}
diff --git a/test/fixtures/bug-27-entries.json b/test/fixtures/bug-27-entries.json
@@ -0,0 +1,31 @@
+{
+  "total": null,
+  "oncalls": [
+    {
+      "escalation_policy": {
+        "id": "PIYVU5O",
+        "type": "escalation_policy_reference",
+        "summary": "Escalation Policy eaque ipsam porro",
+        "self": "https://api.pagerduty.com/escalation_policies/PIYVU5O",
+        "html_url": "https://apidocs.pagerduty.com/escalation_policies/PIYVU5O"
+      },
+      "escalation_level": 1,
+      "schedule": {
+        "id": "P538IZH",
+        "type": "schedule_reference",
+        "summary": "Schedule a quasi illum",
+        "self": "https://api.pagerduty.com/schedules/P538IZH",
+        "html_url": "https://apidocs.pagerduty.com/schedules/P538IZH"
+      },
+      "user": {
+        "summary": "Gregory",
+        "id": "PRT2T0A",
+        "type": "user_reference",
+        "self": "https://api.pagerduty.com/escalation_policies/PRT2T0",
+        "html_url": "https://apiary.pagerduty.com/escalation_policies/PRT2T0"
+      },
+      "end": "2012-08-18T00:00:00-04:00",
+      "start": "2012-08-17T16:00:00-04:00"
+    }
+  ]
+}
diff --git a/test/fixtures/bug-27-incident.json b/test/fixtures/bug-27-incident.json
@@ -0,0 +1,22 @@
+{
+  "incident": {
+    "type": "incident",
+    "title": "On-call overlap found!",
+    "service": {
+      "id": "PFARE53",
+      "type": "service_reference"
+    },
+    "body": {
+      "type": "incident_body",
+      "details": "Primary and Secondary (from Fri, 17 Aug 2012 20:00:00 UTC to Tue, 21 Aug 2012 04:00:00 UTC)"
+    },
+    "assignments": [
+      {
+        "assignee": {
+          "id": "PRT2T0A",
+          "type": "user_reference"
+        }
+      }
+    ]
+  }
+}
diff --git a/test/fixtures/entries-days.json b/test/fixtures/entries-days.json
@@ -24,7 +24,7 @@
         "self": "https://api.pagerduty.com/escalation_policies/PRT2T0",
         "html_url": "https://apiary.pagerduty.com/escalation_policies/PRT2T0"
       },
-      "end": "2012-08-18T:00:00-04:00",
+      "end": "2012-08-18T00:00:00-04:00",
       "start": "2012-08-17T16:00:00-04:00"
     },
     {

diff --git a/test/fixtures/incident.json b/test/fixtures/incident.json
@@ -8,7 +8,7 @@
     },
     "body": {
       "type": "incident_body",
-      "details": "Primary and Secondary (the first starting on Sun, 19 Aug 2012 16:00:00 GMT, the second on Sun, 19 Aug 2012 16:00:00 GMT)"
+      "details": "Primary and Secondary (from Sun, 19 Aug 2012 16:00:00 UTC to Mon, 20 Aug 2012 04:00:00 UTC)"
     },
     "assignments": [
       {

diff --git a/test/fixtures/incident2.json b/test/fixtures/incident2.json
@@ -8,7 +8,7 @@
     },
     "body": {
       "type": "incident_body",
-      "details": "Primary and Secondary (the first starting on Sun, 19 Aug 2012 04:00:00 GMT, the second on Sun, 19 Aug 2012 04:00:00 GMT)"
+      "details": "Primary and Secondary (from Sun, 19 Aug 2012 04:00:00 UTC to Sun, 19 Aug 2012 16:00:00 UTC)"
     },
     "assignments": [
       {

diff --git a/test/notify-test.js b/test/notify-test.js
@@ -1,4 +1,5 @@
 const { assert } = require('chai');
+const moment = require('moment');
 const nock = require('nock');
 const nconf = require('nconf');
 
@@ -14,7 +15,7 @@ describe('Test send message using notify.send for both', () => {
   let actual = null;
 
   before((done) => {
-    const overlapDate = new Date();
+    const overlapDate = moment.utc();
     let message = {
       user: 'Test user',
       userId: '1234',
@@ -24,7 +25,7 @@ describe('Test send message using notify.send for both', () => {
     };
 
     const expectBody = {
-      text: `Following overlaps found:\n*Test user:* \`TEST1\` and \`TEST2\` (the first starting on ${overlapDate.toUTCString()}, the second on ${overlapDate.toUTCString()})\n`,
+      text: `Following overlaps found:\n*Test user:* \`TEST1\` and \`TEST2\` (from ${notify.toUTCString(overlapDate)} to ${notify.toUTCString(overlapDate)})\n`,
       channel: '#channel-name',
     };
 
@@ -46,8 +47,8 @@ describe('Test send message using notify.send for both', () => {
         user: 'Test user',
         userId: '1234',
         schedules: ['TEST1', 'TEST2'],
-        date: overlapDate,
-        crossDate: overlapDate,
+        overlapStart: overlapDate,
+        overlapEnd: overlapDate,
       };
 
       return notify.send(options, [message], (err, result) => {