Merge pull request #479 from 18F/revert-471-leave-britney-alone-shes-ooo

Revert "Don't send messages to users whose status message indicates they are OOO"
18F · Nov 17, 2022 · 6fd5274 · 6fd5274
2 parents f7262f5 + e3c19a3
commit 6fd5274
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 183 deletions.
diff --git a/src/scripts/angry-tock.js b/src/scripts/angry-tock.js
@@ -2,7 +2,7 @@ const holidays = require("@18f/us-federal-holidays");
 const moment = require("moment-timezone");
 const scheduler = require("node-schedule");
 const {
-  slack: { postMessage, sendDirectMessage, slackUserIsOOO },
+  slack: { postMessage, sendDirectMessage },
   tock: { get18FTockSlackUsers, get18FTockTruants },
   helpMessage,
 } = require("../utils");
@@ -58,26 +58,12 @@ module.exports = (app, config = process.env) => {
 
     const tockSlackUsers = await get18FTockSlackUsers();
     const truants = await get18FTockTruants(m());
-
-    const truantTockEmails = new Set(truants.map(({ email }) => email));
-
-    const slackableTruants = tockSlackUsers.filter(({ email }) =>
-      truantTockEmails.has(email)
+    const slackableTruants = tockSlackUsers.filter((tockUser) =>
+      truants.some((truant) => truant.email === tockUser.email)
     );
 
-    const userIsOOO = new Map(
-      await Promise.all(
-        slackableTruants.map(async ({ slack_id: id }) => [
-          id,
-          await slackUserIsOOO(id),
-        ])
-      )
-    );
-
-    slackableTruants.forEach(async ({ slack_id: slackID }) => {
-      if (!userIsOOO.get(slackID)) {
-        sendDirectMessage(slackID, message);
-      }
+    slackableTruants.forEach(({ slack_id: slackID }) => {
+      sendDirectMessage(slackID, message);
     });
 
     if (!calm) {
@@ -90,18 +76,12 @@ module.exports = (app, config = process.env) => {
         );
 
         const report = [];
-        slackableTruants.forEach((u) => {
-          if (userIsOOO.get(u.slack_id)) {
-            report.push([
-              `• <@${u.slack_id}> (not notified on Slack - maybe OOO)`,
-            ]);
-          } else {
-            report.push([`• <@${u.slack_id}> (notified on Slack)`]);
-          }
-        });
-        nonSlackableTruants.forEach((u) => {
-          report.push([`• ${u.username} (not notified)`]);
-        });
+        slackableTruants.forEach((u) =>
+          report.push([`• <@${u.slack_id}> (notified on Slack)`])
+        );
+        nonSlackableTruants.forEach((u) =>
+          report.push([`• ${u.username} (not notified)`])
+        );
 
         const truantReport = {
           attachments: [

diff --git a/src/scripts/angry-tock.test.js b/src/scripts/angry-tock.test.js
@@ -3,7 +3,7 @@ const moment = require("moment-timezone");
 const {
   getApp,
   utils: {
-    slack: { postMessage, sendDirectMessage, slackUserIsOOO },
+    slack: { postMessage, sendDirectMessage },
     tock: { get18FTockSlackUsers, get18FTockTruants },
   },
 } = require("../utils/test");
@@ -58,13 +58,6 @@ describe("Angry Tock", () => {
         last_name: "Four",
         username: "employee4",
       },
-      {
-        id: "tock5",
-        email: "user@five",
-        first_name: "User",
-        last_name: "Five",
-        username: "employee5",
-      },
     ]);
 
     get18FTockSlackUsers.mockResolvedValue([
@@ -89,23 +82,7 @@ describe("Angry Tock", () => {
         slack_id: "slack3",
         user: "employee3",
       },
-      {
-        email: "user@five",
-        id: "tock5",
-        name: "User Five",
-        slack_id: "slack5",
-        user: "slack5",
-      },
     ]);
-
-    slackUserIsOOO.mockImplementation((id) => {
-      switch (id) {
-        case "slack5":
-          return true;
-        default:
-          return false;
-      }
-    });
   });
 
   afterAll(() => {
@@ -291,25 +268,15 @@ describe("Angry Tock", () => {
       text: ":angrytock: <https://tock.18f.gov|Tock your time>! You gotta!",
     });
 
-    // User 5 should be OOO, so make sure they weren't messaged
-    expect(sendDirectMessage).not.toHaveBeenCalledWith("slack5");
-
     expect(postMessage.mock.calls.length).toBe(2);
 
     const reportMessage = {
       attachments: [
         {
-          fallback: `
-• <@slack1> (notified on Slack)
-• <@slack2> (notified on Slack)
-• <@slack5> (not notified on Slack - maybe OOO)
-• employee4 (not notified)`.trim(),
+          fallback:
+            "• <@slack1> (notified on Slack)\n• <@slack2> (notified on Slack)\n• employee4 (not notified)",
           color: "#FF0000",
-          text: `
-• <@slack1> (notified on Slack)
-• <@slack2> (notified on Slack)
-• <@slack5> (not notified on Slack - maybe OOO)
-• employee4 (not notified)`.trim(),
+          text: "• <@slack1> (notified on Slack)\n• <@slack2> (notified on Slack)\n• employee4 (not notified)",
         },
       ],
       username: "Angry Tock",

diff --git a/src/scripts/optimistic-tock.js b/src/scripts/optimistic-tock.js
@@ -3,7 +3,7 @@ const moment = require("moment-timezone");
 const scheduler = require("node-schedule");
 const {
   optOut,
-  slack: { sendDirectMessage, slackUserIsOOO },
+  slack: { sendDirectMessage },
   tock: { get18FTockTruants, get18FTockSlackUsers },
   helpMessage,
 } = require("../utils");
@@ -61,10 +61,7 @@ module.exports = async (app, config = process.env) => {
 
     await Promise.all(
       truantTockSlackUsers.map(async ({ slack_id: slackID }) => {
-        const isOOO = await slackUserIsOOO(slackID);
-        if (!isOOO) {
-          await sendDirectMessage(slackID, message);
-        }
+        await sendDirectMessage(slackID, message);
       })
     );
   };

diff --git a/src/scripts/optimistic-tock.test.js b/src/scripts/optimistic-tock.test.js
@@ -4,7 +4,7 @@ const {
   getApp,
   utils: {
     optOut,
-    slack: { sendDirectMessage, slackUserIsOOO },
+    slack: { sendDirectMessage },
     tock: { get18FTockSlackUsers, get18FTockTruants },
   },
 } = require("../utils/test");
@@ -48,20 +48,6 @@ describe("Optimistic Tock", () => {
         last_name: "Two",
         username: "employee2",
       },
-      {
-        id: "tock4",
-        email: "user@four",
-        first_name: "User",
-        last_name: "Four",
-        username: "employee4",
-      },
-      {
-        id: "tock5",
-        email: "user@five",
-        first_name: "User",
-        last_name: "Five",
-        username: "employee5",
-      },
     ]);
 
     get18FTockSlackUsers.mockResolvedValue([
@@ -97,25 +83,7 @@ describe("Optimistic Tock", () => {
         user: "employee5",
         tz: "America/New_York",
       },
-      {
-        id: "tock6",
-        email: "user@six",
-        first_name: "User",
-        last_name: "Six",
-        slack_id: "slack 6",
-        username: "employee6",
-        tz: "America/New_York",
-      },
     ]);
-
-    slackUserIsOOO.mockImplementation((id) => {
-      switch (id) {
-        case "slack 5":
-          return true;
-        default:
-          return false;
-      }
-    });
   });
 
   afterAll(() => {

diff --git a/src/utils/slack.js b/src/utils/slack.js
@@ -98,14 +98,6 @@ const getSlackUsersInConversation = async ({
     return allUsers.filter(({ id }) => channelUsers.includes(id));
   });
 
-const getSlackUserStatusText = async (userId) =>
-  cache(`get status for user ${userId}`, 10, async () => {
-    const {
-      profile: { status_text: status },
-    } = await defaultClient.users.profile.get({ user: userId });
-    return status;
-  });
-
 const postEphemeralMessage = async (message, { SLACK_TOKEN } = process.env) => {
   await defaultClient.chat.postEphemeral({
     ...message,
@@ -155,21 +147,14 @@ const sendDirectMessage = async (
   );
 };
 
-const slackUserIsOOO = async (userId) => {
-  const statusText = await module.exports.getSlackUserStatusText(userId);
-  return /\b(ooo|out of( the)? office|vacation)\b/i.test(statusText);
-};
-
 module.exports = {
   addEmojiReaction,
   getChannelID,
   getSlackUsers,
   getSlackUsersInConversation,
-  getSlackUserStatusText,
   postEphemeralMessage,
   postEphemeralResponse,
   postMessage,
   sendDirectMessage,
   setClient,
-  slackUserIsOOO,
 };
diff --git a/src/utils/slack.test.js b/src/utils/slack.test.js
@@ -1,24 +1,20 @@
-const slack = require("./slack");
-
 const {
   addEmojiReaction,
   getChannelID,
   getSlackUsers,
   getSlackUsersInConversation,
-  getSlackUserStatusText,
   postEphemeralMessage,
   postEphemeralResponse,
   postMessage,
   sendDirectMessage,
   setClient,
-  slackUserIsOOO,
-} = slack;
+} = require("./slack");
 
 describe("utils / slack", () => {
   const defaultClient = {
     chat: { postEphemeral: jest.fn(), postMessage: jest.fn() },
     conversations: { list: jest.fn(), open: jest.fn() },
-    users: { list: jest.fn(), profile: { get: jest.fn() } },
+    users: { list: jest.fn() },
   };
   const config = { SLACK_TOKEN: "slack token" };
 
@@ -152,19 +148,6 @@ describe("utils / slack", () => {
     ]);
   });
 
-  it("gets a Slack user's status text", async () => {
-    defaultClient.users.profile.get.mockResolvedValue({
-      profile: { status_text: "text" },
-    });
-
-    const status = await getSlackUserStatusText("slack id");
-
-    expect(defaultClient.users.profile.get).toHaveBeenCalledWith({
-      user: "slack id",
-    });
-    expect(status).toEqual("text");
-  });
-
   it("can post an ephemeral message", async () => {
     const msg = { this: "is", my: "message" };
 
@@ -245,47 +228,4 @@ describe("utils / slack", () => {
       token: "slack token",
     });
   });
-
-  describe("tells you if a user is OOO", () => {
-    slack.getSlackUserStatusText = jest.fn();
-
-    it("says no if their status is empty", async () => {
-      slack.getSlackUserStatusText.mockResolvedValue("");
-      const isOOO = await slackUserIsOOO("bob");
-
-      expect(isOOO).toEqual(false);
-    });
-
-    describe("says yes if...", () => {
-      it("their status is OOO, but with mixed caps (case-insensitive check)", async () => {
-        slack.getSlackUserStatusText.mockResolvedValue("I am OoO for a while");
-        const isOOO = await slackUserIsOOO("bob");
-
-        expect(isOOO).toEqual(true);
-      });
-
-      it("their status is 'out of office'", async () => {
-        slack.getSlackUserStatusText.mockResolvedValue("out of office");
-        const isOOO = await slackUserIsOOO("bob");
-
-        expect(isOOO).toEqual(true);
-      });
-
-      it("their status is 'out of the office'", async () => {
-        slack.getSlackUserStatusText.mockResolvedValue(
-          "I will be out of the office until I am not"
-        );
-        const isOOO = await slackUserIsOOO("bob");
-
-        expect(isOOO).toEqual(true);
-      });
-
-      it("their status mentions vacation", async () => {
-        slack.getSlackUserStatusText.mockResolvedValue("On vacation, friends!");
-        const isOOO = await slackUserIsOOO("bob");
-
-        expect(isOOO).toEqual(true);
-      });
-    });
-  });
 });