[ENG-3054] Allow all users to view and manage subscriptions (#96)

* Allow all users view subscription * update tests * update required scopes * Show all subscriptions in unsubscribe menu action
datadotworld · Apr 18, 2024 · 00112a0 · 00112a0
1 parent 1359f62
commit 00112a0
Show file tree

Hide file tree

Showing 6 changed files with 72 additions and 74 deletions.
diff --git a/manifest.yml b/manifest.yml
@@ -22,9 +22,6 @@ oauth_config:
   redirect_urls:
     - https://major-cougar-adequately.ngrok-free.app/api/v1/auth/oauth
   scopes:
-    user:
-      - links:read
-      - links:write
     bot:
       - channels:read
       - chat:write
@@ -40,7 +37,6 @@ settings:
     request_url: https://major-cougar-adequately.ngrok-free.app/api/v1/unfurl/action
     user_events:
       - app_uninstalled
-      - link_shared
       - tokens_revoked
     bot_events:
       - link_shared

diff --git a/server/controllers/__test__/command.test.js b/server/controllers/__test__/command.test.js
@@ -241,27 +241,22 @@ describe("Test Auth controller methods", () => {
   });
 
   it("should send appropiate message to slack when subscription is not present in channel", async done => {
-    const userid = "userid";
     const channelid = "channelid";
     const cmd = "unsubscribe owner/datasetid";
     const responseUrl = "responseUrl";
-    const token = "token";
 
     helper.getSubscriptionStatus = jest.fn(() => [false, false]);
     slack.sendResponse = jest.fn(() => Promise.resolve());
 
     await command.unsubscribeFromDatasetOrProject(
-      userid,
       channelid,
       cmd,
-      responseUrl,
-      token
+      responseUrl
     );
 
     expect(helper.getSubscriptionStatus).toHaveBeenCalledWith(
       "owner/datasetid",
-      channelid,
-      userid
+      channelid     
     );
     expect(slack.sendResponse).toHaveBeenCalledWith(responseUrl, {
       replace_original: false,
@@ -275,11 +270,9 @@ describe("Test Auth controller methods", () => {
   it(
     "should send appropiate message to slack when unsubscribeFromDatasetOrProject fails",
     async done => {
-      const userid = "userid";
       const channelid = "channelid";
       const cmd = "subscribe owner/datasetid";
       const responseUrl = "responseUrl";
-      const token = "token";
       const error = new Error("Test - error");
       const data = {
         replace_original: false,
@@ -291,17 +284,14 @@ describe("Test Auth controller methods", () => {
       helper.getSubscriptionStatus = jest.fn(() => Promise.reject(error));
 
       await command.unsubscribeFromDatasetOrProject(
-        userid,
         channelid,
         cmd,
         responseUrl,
-        token
       );
 
       expect(helper.getSubscriptionStatus).toHaveBeenCalledWith(
         "owner/datasetid",
         channelid,
-        userid
       );
 
       expect(slack.sendResponse).toHaveBeenCalledWith(responseUrl, data);
@@ -314,7 +304,6 @@ describe("Test Auth controller methods", () => {
   it(
     "should unsubscribe from project",
     async done => {
-      const userid = "userid";
       const channelId = "channelid";
       const cmd = "subscribe owner/datasetid";
       const responseUrl = "responseUrl";
@@ -336,7 +325,6 @@ describe("Test Auth controller methods", () => {
 
       await command.unsubscribeFromProject(
         channelId,
-        userid,
         cmd,
         responseUrl,
         token
@@ -353,34 +341,37 @@ describe("Test Auth controller methods", () => {
   it(
     "should unsubscribe from account",
     async done => {
-      const userid = "userid";
       const cmd = "subscribe agentid";
       const responseUrl = "responseUrl";
       const channelid = "channelid";
-      const token = "token";
       const message = "No problem! You'll no longer receive notifications about *agentid* here.";
       const data = { message };
       const slackData = {
         replace_original: false,
         delete_original: false,
         text: message
       };
+      const user = { dwAccessToken: "dwAccessToken" };
+      const subscription = {  slackUserId: "slackUserId" };
 
+      Subscription.findOne = jest.fn(() => Promise.resolve({ subscription }));
+      User.findOne = jest.fn(() => Promise.resolve({ user }));
       Subscription.destroy = jest.fn(() => Promise.resolve());
+
       helper.getSubscriptionStatus = jest.fn(() => [true, true]);
       dataworld.unsubscribeFromAccount = jest.fn(() =>
         Promise.resolve({ data })
       );
       slack.sendResponse = jest.fn(() => Promise.resolve());
 
       await command.unsubscribeFromAccount(
-        userid,
         channelid,
         cmd,
         responseUrl,
-        token
       );
       expect(helper.getSubscriptionStatus).toHaveBeenCalledTimes(1);
+      expect(User.findOne).toHaveBeenCalledTimes(1);
+      expect(Subscription.findOne).toHaveBeenCalledTimes(1);
       expect(Subscription.destroy).toHaveBeenCalledTimes(1);
       expect(dataworld.unsubscribeFromAccount).toHaveBeenCalledTimes(1);
       expect(slack.sendResponse).toHaveBeenCalledWith(responseUrl, slackData);

diff --git a/server/controllers/command.js b/server/controllers/command.js
@@ -208,12 +208,11 @@ const subscribeToAccount = async (
 };
 
 const unsubscribeFromDatasetOrProject = async (
-  userId,
   channelid,
   command,
-  responseUrl,
-  token
+  responseUrl
 ) => {
+  let token;
   try {
     const commandText = process.env.SLASH_COMMAND;
 
@@ -223,10 +222,23 @@ const unsubscribeFromDatasetOrProject = async (
     const [
       hasSubscriptionInChannel,
       removeDWSubscription
-    ] = await helper.getSubscriptionStatus(resourceId, channelid, userId);
+    ] = await helper.getSubscriptionStatus(resourceId, channelid);
 
     // If subscription belongs to channel and the actor(user), then go ahead and unsubscribe
     if (hasSubscriptionInChannel) {
+      const channelSubscription = await Subscription.findOne({
+        where: {
+          resourceId: `${commandParams.owner}/${commandParams.id}`,
+          channelId: channelid
+        }
+      });
+
+      const user = await User.findOne({
+        where: { slackId: channelSubscription.slackUserId }
+      });
+
+      token = user.dwAccessToken;
+
       // will be true if user subscribed to this resource in one channel
       if (removeDWSubscription) {
         // use dataworld wrapper to unsubscribe from dataset
@@ -241,7 +253,6 @@ const unsubscribeFromDatasetOrProject = async (
       await removeSubscriptionRecord(
         commandParams.owner,
         commandParams.id,
-        userId,
         channelid
       );
       // send successful unsubscription message to Slack
@@ -259,7 +270,6 @@ const unsubscribeFromDatasetOrProject = async (
     console.warn("Failed to unsubscribe from dataset : ", error.message);
     // Handle as project
     await unsubscribeFromProject(
-      userId,
       channelid,
       command,
       responseUrl,
@@ -269,7 +279,6 @@ const unsubscribeFromDatasetOrProject = async (
 };
 
 const unsubscribeFromProject = async (
-  userId,
   channelId,
   command,
   responseUrl,
@@ -282,7 +291,7 @@ const unsubscribeFromProject = async (
     const [
       hasSubscriptionInChannel,
       removeDWSubscription
-    ] = await helper.getSubscriptionStatus(resourceId, channelId, userId);
+    ] = await helper.getSubscriptionStatus(resourceId, channelId);
 
     if (removeDWSubscription) {
       await dataworld.unsubscribeFromProject(
@@ -295,7 +304,6 @@ const unsubscribeFromProject = async (
     await removeSubscriptionRecord(
       commandParams.owner,
       commandParams.id,
-      userId,
       channelId
     );
     // send successful unsubscription message to Slack
@@ -314,11 +322,9 @@ const unsubscribeFromProject = async (
 };
 
 const unsubscribeFromAccount = async (
-  userId,
   channelid,
   command,
-  responseUrl,
-  token
+  responseUrl
 ) => {
   const commandText = process.env.SLASH_COMMAND;
 
@@ -328,16 +334,26 @@ const unsubscribeFromAccount = async (
   const [
     hasSubscriptionInChannel,
     removeDWSubscription
-  ] = await helper.getSubscriptionStatus(resourceId, channelid, userId);
+  ] = await helper.getSubscriptionStatus(resourceId, channelid);
   if (hasSubscriptionInChannel) {
     try {
+      const channelSubscription = await Subscription.findOne({
+        where: {
+          resourceId: `${commandParams.owner}/${commandParams.id}`,
+          channelId: channelid
+        }
+      });
+
+      const user = await User.findOne({
+        where: { slackId: channelSubscription.slackUserId }
+      });
+
       if (removeDWSubscription) {
-        await dataworld.unsubscribeFromAccount(commandParams.id, token);
+        await dataworld.unsubscribeFromAccount(commandParams.id, user.dwAccessToken);
       }
       await removeSubscriptionRecord(
         commandParams.owner,
         commandParams.id,
-        userId,
         channelid
       );
       // send successful unsubscription message to Slack
@@ -373,10 +389,6 @@ const listSubscription = async (
       where: { channelId: channelid }
     });
 
-    const user = await User.findOne({
-      where: { slackId: userId }
-    });
-
     let message;
     let blocks;
     let options = [];
@@ -388,6 +400,10 @@ const listSubscription = async (
       await Promise.all(
         subscriptions.map(async subscription => {
           try {
+            const user = await User.findOne({
+              where: { slackId: subscription.slackUserId }
+            });
+
             let isProject = false;
             if (subscription.resourceId.includes("/")) {
               const data = subscription.resourceId.split("/");
@@ -410,15 +426,13 @@ const listSubscription = async (
               isProject
             );
             if (existsInDW) {
-              if (subscription.slackUserId === userId) {
-                options.push({
-                  "text": {
-                    "type": "plain_text",
-                    "text": subscription.resourceId
-                  },
-                  "value": subscription.resourceId
-                });
-              }
+              options.push({
+                "text": {
+                  "type": "plain_text",
+                  "text": subscription.resourceId
+                },
+                "value": subscription.resourceId
+              });
               blockText += `• ${baseUrl}/${subscription.resourceId
                 } \n *created by :* <@${subscription.slackUserId}> \n`;
             }
@@ -520,11 +534,11 @@ const addSubscriptionRecord = async (owner, id, userId, channelId) => {
   }
 };
 
-const removeSubscriptionRecord = async (owner, id, userId, channelId) => {
+const removeSubscriptionRecord = async (owner, id, channelId) => {
   // delete subscription
   const resourceId = owner ? `${owner}/${id}` : `${id}`;
   await Subscription.destroy({
-    where: { resourceId: resourceId, slackUserId: userId, channelId: channelId }
+    where: { resourceId: resourceId, channelId: channelId }
   });
 };
 
@@ -622,20 +636,16 @@ const subscribeOrUnsubscribe = (req, token) => {
       break;
     case UNSUBSCRIBE_DATASET_OR_PROJECT:
       unsubscribeFromDatasetOrProject(
-        req.body.user_id,
         req.body.channel_id,
         command,
-        responseUrl,
-        token
+        responseUrl
       );
       break;
     case UNSUBSCRIBE_ACCOUNT:
       unsubscribeFromAccount(
-        req.body.user_id,
         req.body.channel_id,
         command,
-        responseUrl,
-        token
+        responseUrl
       );
       break;
     default:
@@ -721,29 +731,25 @@ const handleButtonAction = async (payload, action, user) => {
   }
 };
 
-const handleMenuAction = async (payload, action, user) => {
+const handleMenuAction = async (payload, action) => {
   if (action.action_id === "unsubscribe_menu") {
     const value = action.selected_option.value;
     if (value.includes("/")) {
       //unsubscribe from project of dataset
       await unsubscribeFromDatasetOrProject(
-        payload.user.id,
         payload.channel.id,
         `unsubscribe ${value}`,
-        payload.response_url,
-        user.dwAccessToken
+        payload.response_url
       );
     } else {
       // unsubscribe from account
       await unsubscribeFromAccount(
-        payload.user.id,
         payload.channel.id,
         `unsubscribe ${value}`,
-        payload.response_url,
-        user.dwAccessToken
+        payload.response_url
       );
     }
-    // Update list of subscriptions
+    // Updated list of subscriptions
     await listSubscription(
       payload.response_url,
       payload.channel.id,
@@ -801,7 +807,7 @@ const performAction = async (req, res) => {
       if (action.type === "button") {
         await handleButtonAction(payload, action, user);
       } else if (action.type === "static_select") {
-        await handleMenuAction(payload, action, user);
+        await handleMenuAction(payload, action);
       } else {
         console.warn("Unknown action type : ", action.action_id)
       }