From 00112a0e8b7764d555dfd8f8c7f096598808ee4b Mon Sep 17 00:00:00 2001 From: "Kenny(Kehinde) Shittu" Date: Thu, 18 Apr 2024 12:11:10 +0100 Subject: [PATCH] [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 --- manifest.yml | 4 - server/controllers/__test__/command.test.js | 27 ++---- server/controllers/command.js | 96 +++++++++++---------- server/helpers/__test__/helper.test.js | 4 +- server/helpers/helper.js | 5 +- server/routes/__test__/command.test.js | 10 ++- 6 files changed, 72 insertions(+), 74 deletions(-) diff --git a/manifest.yml b/manifest.yml index 7fbe72a..45116b0 100644 --- 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 index f42437e..30be7bd 100644 --- 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,11 +341,9 @@ 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 = { @@ -365,8 +351,13 @@ describe("Test Auth controller methods", () => { 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 }) @@ -374,13 +365,13 @@ describe("Test Auth controller methods", () => { 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 index 94a9ad9..bed2b1e 100755 --- 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) } diff --git a/server/helpers/__test__/helper.test.js b/server/helpers/__test__/helper.test.js index cfaf3a8..92fcebe 100644 --- a/server/helpers/__test__/helper.test.js +++ b/server/helpers/__test__/helper.test.js @@ -109,7 +109,7 @@ describe("Test helper methods", () => { const [ hasSubscriptionInChannel, removeDWsubscription - ] = await helper.getSubscriptionStatus(resourceid, channelId, userId); + ] = await helper.getSubscriptionStatus(resourceid, channelId); expect(Subscription.findAll).toHaveBeenCalledTimes(1); expect(hasSubscriptionInChannel).toBeTruthy(); @@ -129,7 +129,7 @@ describe("Test helper methods", () => { const [ hasSubscriptionInChannel, removeDWsubscription - ] = await helper.getSubscriptionStatus(resourceid, channelid, userId); + ] = await helper.getSubscriptionStatus(resourceid, channelid); expect(Subscription.findAll).toHaveBeenCalledTimes(1); expect(hasSubscriptionInChannel).toBeFalsy(); diff --git a/server/helpers/helper.js b/server/helpers/helper.js index 5f241cd..3ca1569 100644 --- a/server/helpers/helper.js +++ b/server/helpers/helper.js @@ -109,12 +109,11 @@ const shouldRetry = (error) => { return error.response && error.response.status === 429; } -const getSubscriptionStatus = async (resourceid, channelid, userId) => { +const getSubscriptionStatus = async (resourceid, channelid) => { try { const subscriptions = await Subscription.findAll({ where: { - resourceId: resourceid, - slackUserId: userId + resourceId: resourceid } }); const channelSubscriptions = collection.filter(subscriptions, function(o) { diff --git a/server/routes/__test__/command.test.js b/server/routes/__test__/command.test.js index e741efe..68f5214 100644 --- a/server/routes/__test__/command.test.js +++ b/server/routes/__test__/command.test.js @@ -26,6 +26,7 @@ const helper = require("../../helpers/helper"); const fixtures = require("../../jest/fixtures"); const Team = require("../../models").Team; +const User = require("../../models").User; const Channel = require("../../models").Channel; const Subscription = require("../../models").Subscription; @@ -238,10 +239,14 @@ describe("POST /api/v1/command/action - Process an action", () => { const isAssociated = true; const dwAccessToken = "dwAccessToken"; const user = { dwAccessToken }; + const subscription = { slackUserId: "slackUserId" }; const message = `No problem! You'll no longer receive notifications about *${dwDatasetId}* here.`; const response = { data: { message } }; Team.findOne = jest.fn(() => Promise.resolve({ teamId, botAccessToken })); + User.findOne = jest.fn(() => Promise.resolve(user)); + Subscription.findOne = jest.fn(() => Promise.resolve({ subscription })); + Channel.findOrCreate = jest.fn(() => Promise.resolve([{}, true])); Subscription.destroy = jest.fn(() => Promise.resolve()); auth.checkSlackAssociationStatus = jest.fn(() => @@ -264,14 +269,15 @@ describe("POST /api/v1/command/action - Process an action", () => { ); expect(Team.findOne).toHaveBeenCalledTimes(2); expect(Channel.findOrCreate).toHaveBeenCalledTimes(1); + expect(User.findOne).toHaveBeenCalledTimes(1); + expect(Subscription.findOne).toHaveBeenCalledTimes(1); expect(Subscription.destroy).toHaveBeenCalledTimes(1); expect(auth.checkSlackAssociationStatus).toHaveBeenCalledWith( payloadObject.user.id ); expect(helper.getSubscriptionStatus).toHaveBeenCalledWith( resourceId, - payloadObject.channel.id, - payloadObject.user.id + payloadObject.channel.id ); const parts = resourceId.split("/"); expect(dataworld.unsubscribeFromDataset).toHaveBeenCalledWith(