From 05e984c7bae905da3fd2f7c9aeaab534b6b32921 Mon Sep 17 00:00:00 2001 From: Chris Pyle Date: Sat, 21 Dec 2024 14:55:48 -0500 Subject: [PATCH] #3044 refactoring and minor fixes --- .../src/controllers/slack.controllers.ts | 16 ++ src/backend/src/integrations/slack.ts | 8 +- src/backend/src/routes/slack.routes.ts | 11 +- src/backend/src/services/slack.services.ts | 159 +++--------------- src/backend/src/utils/slack.utils.ts | 93 +++++++++- .../slackMessages.test.ts | 71 -------- .../{unmocked => unit}/announcements.test.ts | 0 .../{unmocked => unit}/design-review.test.ts | 0 .../{unmocked => unit}/notifications.test.ts | 0 .../{mocked => unit}/organization.test.ts | 0 .../{unmocked => unit}/recruitment.test.ts | 0 .../reimbursement-requests.test.ts | 0 .../{unmocked => unit}/team-type.test.ts | 0 .../tests/{unmocked => unit}/users.test.ts | 0 .../work-package-template.test.ts | 0 15 files changed, 135 insertions(+), 223 deletions(-) create mode 100644 src/backend/src/controllers/slack.controllers.ts rename src/backend/tests/{mocked => integration}/slackMessages.test.ts (84%) rename src/backend/tests/{unmocked => unit}/announcements.test.ts (100%) rename src/backend/tests/{unmocked => unit}/design-review.test.ts (100%) rename src/backend/tests/{unmocked => unit}/notifications.test.ts (100%) rename src/backend/tests/{mocked => unit}/organization.test.ts (100%) rename src/backend/tests/{unmocked => unit}/recruitment.test.ts (100%) rename src/backend/tests/{unmocked => unit}/reimbursement-requests.test.ts (100%) rename src/backend/tests/{unmocked => unit}/team-type.test.ts (100%) rename src/backend/tests/{unmocked => unit}/users.test.ts (100%) rename src/backend/tests/{unmocked => unit}/work-package-template.test.ts (100%) diff --git a/src/backend/src/controllers/slack.controllers.ts b/src/backend/src/controllers/slack.controllers.ts new file mode 100644 index 0000000000..def03d6870 --- /dev/null +++ b/src/backend/src/controllers/slack.controllers.ts @@ -0,0 +1,16 @@ +import { getWorkspaceId } from '../integrations/slack'; +import OrganizationsService from '../services/organizations.services'; +import slackServices from '../services/slack.services'; + +export default class SlackController { + static async processMessageEvent(event: any) { + try { + const organizations = await OrganizationsService.getAllOrganizations(); + const nerSlackWorkspaceId = await getWorkspaceId(); + const orgId = organizations.find((org) => org.slackWorkspaceId === nerSlackWorkspaceId)?.organizationId; + if (orgId) { + slackServices.processMessageSent(event, orgId); + } + } catch (error: unknown) {} + } +} diff --git a/src/backend/src/integrations/slack.ts b/src/backend/src/integrations/slack.ts index 7c80fa819a..b24615743e 100644 --- a/src/backend/src/integrations/slack.ts +++ b/src/backend/src/integrations/slack.ts @@ -182,12 +182,12 @@ export const getUsersInChannel = async (channelId: string) => { return members; } catch (error) { - throw new HttpException(500, 'Error getting members from a slack channel: ' + (error as any).data.error); + return []; } }; /** - * Given a slack channel id, prood.uces the name of the channel + * Given a slack channel id, produces the name of the channel * @param channelId the id of the slack channel * @returns the name of the channel */ @@ -199,7 +199,7 @@ export const getChannelName = async (channelId: string) => { const channelRes = await slack.conversations.info({ channel: channelId }); return channelRes.channel?.name || 'Unknown Channel'; } catch (error) { - throw new HttpException(500, 'Error getting slack channel name: ' + (error as any).data.error); + return; } }; @@ -216,7 +216,7 @@ export const getUserName = async (userId: string) => { const userRes = await slack.users.info({ user: userId }); return userRes.user?.profile?.display_name || userRes.user?.real_name || 'Unkown User'; } catch (error) { - throw new HttpException(500, 'Error getting slack user name: ' + (error as any).data.error); + return; } }; diff --git a/src/backend/src/routes/slack.routes.ts b/src/backend/src/routes/slack.routes.ts index 82b98b0a0b..a085ae4244 100644 --- a/src/backend/src/routes/slack.routes.ts +++ b/src/backend/src/routes/slack.routes.ts @@ -1,17 +1,10 @@ import { createEventAdapter } from '@slack/events-api'; -import slackServices from '../services/slack.services'; -import OrganizationsService from '../services/organizations.services'; -import { getWorkspaceId } from '../integrations/slack'; +import SlackController from '../controllers/slack.controllers'; export const slackEvents = createEventAdapter(process.env.SLACK_SIGNING_SECRET || ''); slackEvents.on('message', async (event) => { - const organizations = await OrganizationsService.getAllOrganizations(); - const nerSlackWorkspaceId = await getWorkspaceId(); - const orgId = organizations.find((org) => org.slackWorkspaceId === nerSlackWorkspaceId)?.organizationId; - if (orgId) { - slackServices.processMessageSent(event, orgId); - } + SlackController.processMessageEvent(event); }); slackEvents.on('error', (error) => { diff --git a/src/backend/src/services/slack.services.ts b/src/backend/src/services/slack.services.ts index 991af8fc18..90c50995a9 100644 --- a/src/backend/src/services/slack.services.ts +++ b/src/backend/src/services/slack.services.ts @@ -1,8 +1,9 @@ import UsersService from './users.services'; -import { getChannelName, getUserName, getUsersInChannel } from '../integrations/slack'; -import { User_Settings } from '@prisma/client'; +import { getChannelName, getUserName } from '../integrations/slack'; import AnnouncementService from './announcement.service'; import { Announcement } from 'shared'; +import prisma from '../prisma/prisma'; +import { blockToMentionedUsers, blockToString } from '../utils/slack.utils'; /** * Represents a slack event for a message in a channel. @@ -66,98 +67,6 @@ export interface SlackRichTextBlock { } export default class slackServices { - /** - * Converts a SlackRichTextBlock into a string representation for an announcement. - * @param block the block of information from slack - * @returns the string that will be combined with other block's strings to create the announcement - */ - private static async blockToString(block: SlackRichTextBlock): Promise { - switch (block.type) { - case 'broadcast': - return '@' + block.range; - case 'color': - return block.value ?? ''; - case 'channel': - //channels are represented as an id, get the name from the slack api - let channelName = block.channel_id; - try { - channelName = await getChannelName(block.channel_id ?? ''); - } catch (error) { - channelName = `ISSUE PARSING CHANNEL:${block.channel_id}`; - } - return '#' + channelName; - case 'date': - return new Date(block.timestamp ?? 0).toISOString(); - case 'emoji': - //if the emoji is a unicode emoji, convert the unicode to a string, - //if it is a slack emoji just use the name of the emoji - if (block.unicode) { - return String.fromCodePoint(parseInt(block.unicode, 16)); - } - return 'emoji:' + block.name; - case 'link': - if (block.text) { - return `${block.text}:(${block.url})`; - } - return block.url ?? ''; - case 'text': - return block.text ?? ''; - case 'user': - //users are represented as an id, get the name of the user from the slack api - let userName: string = block.user_id ?? ''; - try { - userName = (await getUserName(block.user_id ?? '')) ?? `Unknown User:${block.user_id}`; - } catch (error) { - userName = `Unknown_User:${block.user_id}`; - } - return '@' + userName; - case 'usergroup': - return `usergroup:${block.usergroup_id}`; - } - } - - /** - * Gets the users notified in a specific SlackRichTextBlock. - * @param block the block that may contain mentioned user/users - * @param usersSettings the settings of all the users in prisma - * @param channelId the id of the channel that the block is being sent in - * @returns an array of prisma user ids of users to be notified - */ - private static async blockToMentionedUsers( - block: SlackRichTextBlock, - usersSettings: User_Settings[], - channelId: string - ): Promise { - switch (block.type) { - case 'broadcast': - switch (block.range) { - case 'everyone': - return usersSettings.map((usersSettings) => usersSettings.userId); - case 'channel': - case 'here': - //@here behaves the same as @channel; notifies all the users in that channel - let slackIds: string[] = []; - try { - slackIds = await getUsersInChannel(channelId); - } catch (ignored) {} - return usersSettings - .filter((userSettings) => { - return slackIds.some((slackId) => slackId === userSettings.slackId); - }) - .map((user) => user.userId); - default: - return []; - } - case 'user': - return usersSettings - .filter((userSettings) => userSettings.slackId === block.user_id) - .map((userSettings) => userSettings.userId); - default: - //only broadcasts and specific user mentions add recievers to announcements - return []; - } - } - /** * Given a slack event representing a message in a channel, * make the appropriate announcement change in prisma. @@ -166,13 +75,8 @@ export default class slackServices { * @returns an annoucement if an announcement was processed and created/modified/deleted */ static async processMessageSent(event: SlackMessageEvent, organizationId: string): Promise { - let slackChannelName: string; //get the name of the channel from the slack api - try { - slackChannelName = await getChannelName(event.channel); - } catch (error) { - slackChannelName = `Unknown_Channel:${event.channel}`; - } + const slackChannelName: string = (await getChannelName(event.channel)) ?? `Unknown_Channel:${event.channel}`; const dateCreated = new Date(Number(event.event_ts)); //get the message that will be processed either as the event or within a subtype @@ -183,11 +87,7 @@ export default class slackServices { case 'message_deleted': //delete the message using the client_msg_id eventMessage = (event as SlackDeletedMessage).previous_message; - try { - return AnnouncementService.deleteAnnouncement(eventMessage.client_msg_id, organizationId); - } catch (ignored) { - return; - } + return AnnouncementService.deleteAnnouncement(eventMessage.client_msg_id, organizationId); case 'message_changed': eventMessage = (event as SlackUpdatedMessage).message; break; @@ -213,22 +113,14 @@ export default class slackServices { ); //get the name of the user that sent the message from slack - let userName: string = ''; - try { - userName = (await getUserName(eventMessage.user)) ?? ''; - } catch (ignored) {} + let userName = (await getUserName(eventMessage.user)) ?? ''; //if slack could not produce the name of the user, look for their name in prisma if (!userName) { - const userIdList = userSettings - .filter((userSetting) => userSetting.slackId === eventMessage.user) - .map((userSettings) => userSettings.userId); - if (userIdList.length !== 0) { - const prismaUserName = users.find((user) => user.userId === userIdList[0]); - userName = prismaUserName - ? `${prismaUserName?.firstName} ${prismaUserName?.lastName}` - : 'Unknown User:' + eventMessage.user; - } else { + try { + const userWithThatSlackId = await prisma.user.findFirst({ where: { userSettings: { slackId: eventMessage.user } } }); + userName = `${userWithThatSlackId?.firstName} ${userWithThatSlackId?.lastName}`; + } catch { userName = 'Unknown_User:' + eventMessage.user; } } @@ -236,12 +128,10 @@ export default class slackServices { //pull out the blocks of data from the metadata within the message event const richTextBlocks = eventMessage.blocks?.filter((eventBlock: any) => eventBlock.type === 'rich_text'); - if (richTextBlocks && richTextBlocks.length === 1) { + if (richTextBlocks && richTextBlocks.length > 0 && richTextBlocks[0].elements.length > 0) { for (const element of richTextBlocks[0].elements[0].elements) { - messageText += await slackServices.blockToString(element); - userIdsToNotify = userIdsToNotify.concat( - await slackServices.blockToMentionedUsers(element, userSettings, event.channel) - ); + messageText += await blockToString(element); + userIdsToNotify = userIdsToNotify.concat(await blockToMentionedUsers(element, userSettings, event.channel)); } } else { return; @@ -271,19 +161,14 @@ export default class slackServices { ); } catch (ignored) {} } - try { - return await AnnouncementService.createAnnouncement( - messageText, - userIdsToNotify, - dateCreated, - userName, - eventMessage.client_msg_id, - slackChannelName, - organizationId - ); - } catch (error) { - //if announcement does not have unique cient_msg_id disregard it - return; - } + return await AnnouncementService.createAnnouncement( + messageText, + userIdsToNotify, + dateCreated, + userName, + eventMessage.client_msg_id, + slackChannelName, + organizationId + ); } } diff --git a/src/backend/src/utils/slack.utils.ts b/src/backend/src/utils/slack.utils.ts index ecc264610b..ea98a1a698 100644 --- a/src/backend/src/utils/slack.utils.ts +++ b/src/backend/src/utils/slack.utils.ts @@ -1,6 +1,14 @@ import { ChangeRequest, daysBetween, Task, UserPreview, wbsPipe, calculateEndDate } from 'shared'; -import { User } from '@prisma/client'; -import { editMessage, reactToMessage, replyToMessageInThread, sendMessage } from '../integrations/slack'; +import { User, User_Settings } from '@prisma/client'; +import { + editMessage, + getChannelName, + getUserName, + getUsersInChannel, + reactToMessage, + replyToMessageInThread, + sendMessage +} from '../integrations/slack'; import { getUserFullName, getUserSlackId } from './users.utils'; import prisma from '../prisma/prisma'; import { HttpException } from './errors.utils'; @@ -11,6 +19,7 @@ import { addHours, meetingStartTimePipe } from './design-reviews.utils'; import { WorkPackageQueryArgs } from '../prisma-query-args/work-packages.query-args'; import { Prisma } from '@prisma/client'; import { userTransformer } from '../transformers/user.transformer'; +import { SlackRichTextBlock } from '../services/slack.services'; interface SlackMessageThread { messageInfoId: string; @@ -470,3 +479,83 @@ export const addSlackThreadsToChangeRequest = async (crId: string, threads: { ch ); await Promise.all(promises); }; + +/** + * Converts a SlackRichTextBlock into a string representation for an announcement. + * @param block the block of information from slack + * @returns the string that will be combined with other block's strings to create the announcement + */ +export const blockToString = async (block: SlackRichTextBlock) => { + switch (block.type) { + case 'broadcast': + return '@' + block.range; + case 'color': + return block.value ?? ''; + case 'channel': + //channels are represented as an id, get the name from the slack api + const channelName: string = + (await getChannelName(block.channel_id ?? '')) ?? `ISSUE PARSING CHANNEL:${block.channel_id}`; + return '#' + channelName; + case 'date': + return new Date(block.timestamp ?? 0).toISOString(); + case 'emoji': + //if the emoji is a unicode emoji, convert the unicode to a string, + //if it is a slack emoji just use the name of the emoji + if (block.unicode) { + return String.fromCodePoint(parseInt(block.unicode, 16)); + } + return 'emoji:' + block.name; + case 'link': + if (block.text) { + return `${block.text}:(${block.url})`; + } + return block.url ?? ''; + case 'text': + return block.text ?? ''; + case 'user': + //users are represented as an id, get the name of the user from the slack api + const userName: string = (await getUserName(block.user_id ?? '')) ?? `Unknown User:${block.user_id}`; + return '@' + userName; + case 'usergroup': + return `usergroup:${block.usergroup_id}`; + } +}; + +/** + * Gets the users notified in a specific SlackRichTextBlock. + * @param block the block that may contain mentioned user/users + * @param usersSettings the settings of all the users in prisma + * @param channelId the id of the channel that the block is being sent in + * @returns an array of prisma user ids of users to be notified + */ +export const blockToMentionedUsers = async ( + block: SlackRichTextBlock, + usersSettings: User_Settings[], + channelId: string +) => { + switch (block.type) { + case 'broadcast': + switch (block.range) { + case 'everyone': + return usersSettings.map((usersSettings) => usersSettings.userId); + case 'channel': + case 'here': + //@here behaves the same as @channel; notifies all the users in that channel + const slackIds: string[] = await getUsersInChannel(channelId); + return usersSettings + .filter((userSettings) => { + return slackIds.some((slackId) => slackId === userSettings.slackId); + }) + .map((user) => user.userId); + default: + return []; + } + case 'user': + return usersSettings + .filter((userSettings) => userSettings.slackId === block.user_id) + .map((userSettings) => userSettings.userId); + default: + //only broadcasts and specific user mentions add recievers to announcements + return []; + } +}; diff --git a/src/backend/tests/mocked/slackMessages.test.ts b/src/backend/tests/integration/slackMessages.test.ts similarity index 84% rename from src/backend/tests/mocked/slackMessages.test.ts rename to src/backend/tests/integration/slackMessages.test.ts index f8fc76f4ab..99319d0bb0 100644 --- a/src/backend/tests/mocked/slackMessages.test.ts +++ b/src/backend/tests/integration/slackMessages.test.ts @@ -12,7 +12,6 @@ import * as apiFunctions from '../../src/integrations/slack'; import AnnouncementService from '../../src/services/announcement.service'; import slackServices from '../../src/services/slack.services'; import { vi } from 'vitest'; -import { HttpException } from '../../src/utils/errors.utils'; vi.mock('../../src/integrations/slack', async (importOriginal) => { return { @@ -198,46 +197,6 @@ describe('Slack message tests', () => { expect(announcement?.usersReceived).toHaveLength(1); }); - it('Deals with errors from slack API', async () => { - vi.mocked(apiFunctions.getUserName).mockImplementation(() => { - throw new HttpException(500, 'sample error'); - }); - vi.mocked(apiFunctions.getChannelName).mockImplementation(() => { - throw new HttpException(500, 'sample error'); - }); - - const spy = vi.spyOn(AnnouncementService, 'createAnnouncement'); - - const announcement = await slackServices.processMessageSent( - createSlackMessageEvent('channel id', '1000000000000', 'slackWW', 'id_1', [ - { type: 'user', user_id: 'slackWW' }, - { type: 'text', text: ' prisma user and non-prisma user ' }, - { type: 'user', user_id: 'non-prisma-slack-id' } - ]), - orgId - ); - - expect(spy).toBeCalledTimes(1); - expect(spy).toBeCalledWith( - '@Unknown_User:slackWW prisma user and non-prisma user @Unknown_User:non-prisma-slack-id', - [wonderwoman.userId], - new Date(1000000000000), - 'Wonder Woman', - 'id_1', - 'Unknown_Channel:channel id', - orgId - ); - - expect(announcement?.text).toBe( - '@Unknown_User:slackWW prisma user and non-prisma user @Unknown_User:non-prisma-slack-id' - ); - expect(announcement?.dateCreated.toDateString()).toBe(new Date(1000000000000).toDateString()); - expect(announcement?.senderName).toBe('Wonder Woman'); - expect(announcement?.slackChannelName).toBe('Unknown_Channel:channel id'); - expect(announcement?.slackEventId).toBe('id_1'); - expect(announcement?.usersReceived).toHaveLength(1); - }); - it("Doesn't create an announcement if no one is mentioned", async () => { vi.mocked(apiFunctions.getUserName).mockReturnValue(Promise.resolve('Slack User Name')); vi.mocked(apiFunctions.getChannelName).mockReturnValue(Promise.resolve('Slack Channel Name')); @@ -256,36 +215,6 @@ describe('Slack message tests', () => { expect(announcement).toBeUndefined(); }); - it('Does nothing if an announcement with the same slack id has already been created', async () => { - vi.mocked(apiFunctions.getUserName).mockReturnValue(Promise.resolve('Slack User Name')); - vi.mocked(apiFunctions.getChannelName).mockReturnValue(Promise.resolve('Slack Channel Name')); - - const createSpy = vi.spyOn(AnnouncementService, 'createAnnouncement'); - - await slackServices.processMessageSent( - createSlackMessageEvent('channel id', '1000000000000', 'user name', 'id_1', [{ type: 'user', user_id: 'slackWW' }]), - orgId - ); - expect(createSpy).toBeCalledWith( - '@Slack User Name', - [wonderwoman.userId], - new Date(1000000000000), - 'Slack User Name', - 'id_1', - 'Slack Channel Name', - orgId - ); - - const announcement2 = await slackServices.processMessageSent( - createSlackMessageEvent('channel id', '1000000000000', 'user name', 'id_1', [ - { type: 'user', user_id: 'slackWW' }, - { type: 'text', text: ' added text' } - ]), - orgId - ); - expect(announcement2).toBeUndefined(); - }); - it('Updates an edit made to a message', async () => { vi.mocked(apiFunctions.getUserName).mockReturnValue(Promise.resolve('Slack User Name')); vi.mocked(apiFunctions.getChannelName).mockReturnValue(Promise.resolve('Slack Channel Name')); diff --git a/src/backend/tests/unmocked/announcements.test.ts b/src/backend/tests/unit/announcements.test.ts similarity index 100% rename from src/backend/tests/unmocked/announcements.test.ts rename to src/backend/tests/unit/announcements.test.ts diff --git a/src/backend/tests/unmocked/design-review.test.ts b/src/backend/tests/unit/design-review.test.ts similarity index 100% rename from src/backend/tests/unmocked/design-review.test.ts rename to src/backend/tests/unit/design-review.test.ts diff --git a/src/backend/tests/unmocked/notifications.test.ts b/src/backend/tests/unit/notifications.test.ts similarity index 100% rename from src/backend/tests/unmocked/notifications.test.ts rename to src/backend/tests/unit/notifications.test.ts diff --git a/src/backend/tests/mocked/organization.test.ts b/src/backend/tests/unit/organization.test.ts similarity index 100% rename from src/backend/tests/mocked/organization.test.ts rename to src/backend/tests/unit/organization.test.ts diff --git a/src/backend/tests/unmocked/recruitment.test.ts b/src/backend/tests/unit/recruitment.test.ts similarity index 100% rename from src/backend/tests/unmocked/recruitment.test.ts rename to src/backend/tests/unit/recruitment.test.ts diff --git a/src/backend/tests/unmocked/reimbursement-requests.test.ts b/src/backend/tests/unit/reimbursement-requests.test.ts similarity index 100% rename from src/backend/tests/unmocked/reimbursement-requests.test.ts rename to src/backend/tests/unit/reimbursement-requests.test.ts diff --git a/src/backend/tests/unmocked/team-type.test.ts b/src/backend/tests/unit/team-type.test.ts similarity index 100% rename from src/backend/tests/unmocked/team-type.test.ts rename to src/backend/tests/unit/team-type.test.ts diff --git a/src/backend/tests/unmocked/users.test.ts b/src/backend/tests/unit/users.test.ts similarity index 100% rename from src/backend/tests/unmocked/users.test.ts rename to src/backend/tests/unit/users.test.ts diff --git a/src/backend/tests/unmocked/work-package-template.test.ts b/src/backend/tests/unit/work-package-template.test.ts similarity index 100% rename from src/backend/tests/unmocked/work-package-template.test.ts rename to src/backend/tests/unit/work-package-template.test.ts