From a3efa33e536fb8835ccf327642703221cda9bb70 Mon Sep 17 00:00:00 2001 From: Ava Thorn Date: Tue, 25 May 2021 01:24:25 -0400 Subject: [PATCH] Fixed bugs and added test coverage (#49) --- docs/administration.md | 4 +- docs/queues.md | 2 +- services/bot/src/commands/admins/create.ts | 1 + services/bot/src/commands/admins/remove.ts | 1 + .../src/commands/graphs/queueLengthHistory.ts | 6 +- services/bot/src/commands/queue/addPerson.ts | 2 +- services/bot/src/commands/queue/get.ts | 2 +- .../bot/src/commands/queue/removePerson.ts | 2 +- services/bot/src/parser.ts | 3 +- .../graphs/getQueueLengthHistory.test.ts | 28 ++++++++- services/bot/tests/parser.test.ts | 62 ++++++++++++++++++- 11 files changed, 102 insertions(+), 11 deletions(-) diff --git a/docs/administration.md b/docs/administration.md index 4e4d853..53f2011 100644 --- a/docs/administration.md +++ b/docs/administration.md @@ -32,7 +32,7 @@ You can add individual admins to a project by tagging them using the following c ``` === "Regex" ``` - ^\s*create admin [\w\s]+\s*$ + ^\s*create admin [\S]+\s*$ ``` ???+ important @@ -67,7 +67,7 @@ You can remove individual admins to a project by tagging them using the followin ``` === "Regex" ``` - ^\s*(delete|remove) admin [\w\s]+\s*$ + ^\s*(delete|remove) admin [\S]+\s*$ ``` ???+ important diff --git a/docs/queues.md b/docs/queues.md index 19edeb4..bd42990 100644 --- a/docs/queues.md +++ b/docs/queues.md @@ -274,7 +274,7 @@ You can list out the members of a queue by issueing the following command: ``` === "Regex" ``` - ^\s*get queue\s*$ + ^\s*get queue( \w+)?\s*$ ``` ???+ example diff --git a/services/bot/src/commands/admins/create.ts b/services/bot/src/commands/admins/create.ts index e40bd5f..d65f6f8 100644 --- a/services/bot/src/commands/admins/create.ts +++ b/services/bot/src/commands/admins/create.ts @@ -14,6 +14,7 @@ export class Create extends CommandBase implements ICommand { public readonly AUTHORIZATION: Auth = Auth.PROJECT_ADMIN; public readonly COMMAND_TYPE: CommandType = CommandType.CREATE; public readonly COMMAND_BASE: string = 'admin'; + public readonly ARGS: string = '{name:\\S+?}'; public readonly DESCRIPTION: string = 'Adds a target project admin'; /* eslint-enable jsdoc/require-jsdoc */ diff --git a/services/bot/src/commands/admins/remove.ts b/services/bot/src/commands/admins/remove.ts index 0bfa142..47dc247 100644 --- a/services/bot/src/commands/admins/remove.ts +++ b/services/bot/src/commands/admins/remove.ts @@ -14,6 +14,7 @@ export class Remove extends CommandBase implements ICommand { public readonly AUTHORIZATION: Auth = Auth.PROJECT_ADMIN; public readonly COMMAND_TYPE: CommandType = CommandType.OPERATION; public readonly COMMAND_BASE: string = 'remove admin'; + public readonly ARGS: string = '{name:\\S+?}'; public readonly DESCRIPTION: string = 'Removes a user as a project admin'; /* eslint-enable jsdoc/require-jsdoc */ diff --git a/services/bot/src/commands/graphs/queueLengthHistory.ts b/services/bot/src/commands/graphs/queueLengthHistory.ts index 14affae..3d0ca94 100644 --- a/services/bot/src/commands/graphs/queueLengthHistory.ts +++ b/services/bot/src/commands/graphs/queueLengthHistory.ts @@ -15,6 +15,7 @@ export class Get extends CommandBase implements ICommand { public readonly AUTHORIZATION: Auth = Auth.NONE; public readonly COMMAND_TYPE: CommandType = CommandType.GET; public readonly COMMAND_BASE: string = 'queue length history'; + public readonly QUEUE: boolean = true; public readonly DESCRIPTION: string = 'creates a graph of the queue length history'; /* eslint-enable jsdoc/require-jsdoc */ @@ -32,7 +33,10 @@ export class Get extends CommandBase implements ICommand { const project = await CommandBase.getProject(initiative); if (typeof project === 'string') return String(project); - const queue = project.queues.filter(i => i.name === project.currentQueue)[0]; + // Get queue + const queueName = initiative.data.queue?.toUpperCase() || project.currentQueue; + + const queue = project.queues.filter(i => i.name === queueName)[0]; if (queue.history.length > 0) { const chart = await new QueueLength(queue).url; diff --git a/services/bot/src/commands/queue/addPerson.ts b/services/bot/src/commands/queue/addPerson.ts index f81be24..8185adb 100644 --- a/services/bot/src/commands/queue/addPerson.ts +++ b/services/bot/src/commands/queue/addPerson.ts @@ -14,7 +14,7 @@ export class AddPerson extends CommandBase implements ICommand { public readonly AUTHORIZATION: Auth = Auth.PROJECT_ADMIN; public readonly COMMAND_TYPE: CommandType = CommandType.OPERATION; public readonly COMMAND_BASE: string = 'add person'; - public readonly ARGS: string = '{name:.+?}'; + public readonly ARGS: string = '{name:\\S+?}'; public readonly DESCRIPTION: string = 'Adds a tagged person to the current queue'; public readonly QUEUE: boolean = true; /* eslint-enable jsdoc/require-jsdoc */ diff --git a/services/bot/src/commands/queue/get.ts b/services/bot/src/commands/queue/get.ts index 549cd97..cb01afb 100644 --- a/services/bot/src/commands/queue/get.ts +++ b/services/bot/src/commands/queue/get.ts @@ -10,7 +10,7 @@ export class Get extends CommandBase implements ICommand { /* eslint-disable jsdoc/require-jsdoc */ public readonly AUTHORIZATION: Auth = Auth.NONE; public readonly COMMAND_TYPE: CommandType = CommandType.GET; - public readonly COMMAND_BASE: string = 'queue( {queue:[\\w\\s]+})?'; + public readonly COMMAND_BASE: string = 'queue( {queue:[\\w]+})?'; public readonly DESCRIPTION: string = 'Gets the current queue and shows the contents of the queue'; /* eslint-enable jsdoc/require-jsdoc */ diff --git a/services/bot/src/commands/queue/removePerson.ts b/services/bot/src/commands/queue/removePerson.ts index 9428405..3c5d14a 100644 --- a/services/bot/src/commands/queue/removePerson.ts +++ b/services/bot/src/commands/queue/removePerson.ts @@ -13,7 +13,7 @@ export class RemovePerson extends CommandBase implements ICommand { public readonly AUTHORIZATION: Auth = Auth.PROJECT_ADMIN; public readonly COMMAND_TYPE: CommandType = CommandType.OPERATION; public readonly COMMAND_BASE: string = 'remove person'; - public readonly ARGS: string = '{name:.+?}'; + public readonly ARGS: string = '{name:\\S+?}'; public readonly QUEUE: boolean = true; public readonly DESCRIPTION: string = 'Removes a tagged person from the current queue'; /* eslint-enable jsdoc/require-jsdoc */ diff --git a/services/bot/src/parser.ts b/services/bot/src/parser.ts index 3bf1327..92e48e5 100644 --- a/services/bot/src/parser.ts +++ b/services/bot/src/parser.ts @@ -24,7 +24,7 @@ export class Parser { let messageData = null; if (request.body.resource == 'attachmentActions') { messageData = await BOT.attachmentActions.get(messageId); - } else { // if (request.body.resource == 'messages') { + } else { messageData = await BOT.messages.get(messageId); } @@ -36,6 +36,7 @@ export class Parser { const destination: Destination = {}; const mentions: string[] = messageData.mentionedPeople || []; + LOGGER.verbose(`Mentions: ${mentions}`); // Email is not sent for attachmentActions. Thus, use the personId as first // priority so that "card" commands and regular text commands will have the same // information parsed and correlated into mongo diff --git a/services/bot/tests/commands/graphs/getQueueLengthHistory.test.ts b/services/bot/tests/commands/graphs/getQueueLengthHistory.test.ts index 00e4cf8..87b136c 100644 --- a/services/bot/tests/commands/graphs/getQueueLengthHistory.test.ts +++ b/services/bot/tests/commands/graphs/getQueueLengthHistory.test.ts @@ -7,7 +7,7 @@ import { AddMe } from '../../../src/commands/queue/addMe'; import { PROJECT_MODEL } from '../../../src/models/project'; import { BOT } from '../../../src/bot'; import MockDate from 'mockdate'; -import { CREATE_PROJECT, TEST_INITIATIVE, STRICT_DATE, STANDARD_USER } from '../../util'; +import { CREATE_PROJECT, TEST_INITIATIVE, STRICT_DATE, STANDARD_USER, CREATE_QUEUE } from '../../util'; TEST_INITIATIVE.rawCommand = 'get largest queue depth'; TEST_INITIATIVE.action = new Get(); @@ -52,6 +52,32 @@ describe('Getting queue length history works appropriately', () => { 'toPersonId': 'notReal' }); }); + test('Should return the correct largest queue result for non-default queue', async () => { + const project = await CREATE_PROJECT(); + await CREATE_QUEUE(project, 'FOO'); + let queue = project.queues.filter(i => i.name === 'FOO')[0]; + expect(queue.members).toHaveLength(0); + expect(queue.history).toHaveLength(0); + + await new AddMe().relax({ ...TEST_INITIATIVE, data: { queue: 'FOO' } }); + queue = (await PROJECT_MODEL.find({ name: project.name }).exec())[0].queues.filter(i => i.name === 'FOO')[0]; + expect(queue.history).toHaveLength(1); + await new AddMe().relax({ ...TEST_INITIATIVE, data: { queue: 'FOO' } }); + queue = (await PROJECT_MODEL.find({ name: project.name }).exec())[0].queues.filter(i => i.name === 'FOO')[0]; + expect(queue.history).toHaveLength(2); + await new AddMe().relax({ ...TEST_INITIATIVE, data: { queue: 'FOO' } }); + queue = (await PROJECT_MODEL.find({ name: project.name }).exec())[0].queues.filter(i => i.name === 'FOO')[0]; + expect(queue.history).toHaveLength(3); // eslint-disable-line @typescript-eslint/no-magic-numbers + expect(queue.history[2].members).toHaveLength(3); // eslint-disable-line @typescript-eslint/no-magic-numbers + expect(queue.history[2].time).toEqual(new Date(STRICT_DATE)); + + expect(await new Get().relax({ ...TEST_INITIATIVE, data: { queue: 'FOO' } })).toEqual(''); + expect(BOT.messages.create).toBeCalledWith({ + 'files': ['myShortUrl'], + 'markdown': 'Click [here](myShortUrl) to see your chart in a browser!', + 'toPersonId': 'notReal' + }); + }); }); describe('Getting queue depth history errors appropriately', () => { diff --git a/services/bot/tests/parser.test.ts b/services/bot/tests/parser.test.ts index 427ca0b..fee0d33 100644 --- a/services/bot/tests/parser.test.ts +++ b/services/bot/tests/parser.test.ts @@ -88,22 +88,80 @@ describe('Parser is working', () => { describe('add person', () => { test('Add person no queue specification', async () => { - BOT.messages.get.mockReturnValue({ text: 'add person FOO', mentionedPeople: ['fooId'] }); + BOT.messages.get.mockReturnValue({ text: 'add person FOO', mentionedPeople: ['fooId2'] }); const result = await new Parser().parse(MOCK_REQUEST); expect(result.action).toEqual(expect.objectContaining({ DESCRIPTION: 'Adds a tagged person to the current queue' })); expect(result.data).toEqual(expect.objectContaining({ queue: undefined })); + expect(result.mentions).toEqual(['fooId2']); }); test('Add person with queue specification', async () => { await CREATE_QUEUE(project as ProjectDocument, 'FOOBAR'); BOT.messages.get - .mockReturnValue({ text: 'add person FOO to queue FOOBAR', mentionedPeople: ['fooId'] }); + .mockReturnValue({ text: 'add person FOO to queue FOOBAR', mentionedPeople: ['fooId2'] }); const result = await new Parser().parse(MOCK_REQUEST); expect(result.action).toEqual(expect.objectContaining({ DESCRIPTION: 'Adds a tagged person to the current queue' })); expect(result.data).toEqual(expect.objectContaining({ name: 'foo', queue: 'foobar' })); + expect(result.mentions).toEqual(['fooId2']); + }); + }); + describe('get queue length history', () => { + test('get queue length history no queue specification', async () => { + BOT.messages.get.mockReturnValue({ text: 'get queue length history' }); + const result = await new Parser().parse(MOCK_REQUEST); + expect(result.action).toEqual(expect.objectContaining({ + DESCRIPTION: 'creates a graph of the queue length history' + })); + expect(result.data).toEqual({ queue: undefined }); + }); + test('get queue length history with queue specification', async () => { + BOT.messages.get.mockReturnValue({ text: 'get queue length history for queue FOOBAR' }); + const result = await new Parser().parse(MOCK_REQUEST); + expect(result.action).toEqual(expect.objectContaining({ + DESCRIPTION: 'creates a graph of the queue length history' + })); + expect(result.data).toEqual(expect.objectContaining({ queue: 'foobar' })); + }); + }); + describe('create admin', () => { + test('create admin no queue specification', async () => { + BOT.messages.get.mockReturnValue({ text: 'create admin FOO', mentionedPeople: ['fooId2'] }); + const result = await new Parser().parse(MOCK_REQUEST); + expect(result.action).toEqual(expect.objectContaining({ + DESCRIPTION: 'Adds a target project admin' + })); + expect(result.data).toEqual({ name: 'foo' }); + expect(result.mentions).toEqual(['fooId2']); + }); + test('create admin with queue specification', async () => { + await CREATE_QUEUE(project as ProjectDocument, 'FOOBAR'); + BOT.messages.get + .mockReturnValue({ text: 'create admin FOO on queue FOOBAR', mentionedPeople: ['fooId2'] }); + const result = await new Parser().parse(MOCK_REQUEST); + // Not a real command + expect(result.action).toEqual(undefined); + }); + }); + describe('remove admin', () => { + test('remove admin no queue specification', async () => { + BOT.messages.get.mockReturnValue({ text: 'remove admin FOO', mentionedPeople: ['fooId2'] }); + const result = await new Parser().parse(MOCK_REQUEST); + expect(result.action).toEqual(expect.objectContaining({ + DESCRIPTION: 'Removes a user as a project admin' + })); + expect(result.data).toEqual({ name: 'foo' }); + expect(result.mentions).toEqual(['fooId2']); + }); + test('remove admin with queue specification', async () => { + await CREATE_QUEUE(project as ProjectDocument, 'FOOBAR'); + BOT.messages.get + .mockReturnValue({ text: 'remove admin FOO on queue FOOBAR', mentionedPeople: ['fooId2'] }); + const result = await new Parser().parse(MOCK_REQUEST); + // Not a real command + expect(result.action).toEqual(undefined); }); });