fixed a bug with queues (#101)

amthorn · web-flow · commit 93a15c54a28b · 2021-07-04T19:38:00.000-04:00
diff --git a/services/bot/src/commands/base.ts b/services/bot/src/commands/base.ts
@@ -134,6 +134,23 @@ export abstract class CommandBase {
             return 'There are no projects registered.';
         }
     }
+    /**
+     * Returns the default queue or the queue identified for the given initiative context.
+     *
+     * @param initiative - This is the initiative for which to get the named queue.
+     * @param project - This is the project for which to get the queue on.
+     * @todo Throw errors instead of returning error string.
+     * @returns Queue model or error string for the given initiative context.
+     */
+    public static async getQueue (initiative: IInitiative, project: IProject): Promise<IQueue | string> {
+        const queueName = initiative.data.queue?.toUpperCase() || project.currentQueue;
+        const queueObject = project.queues.filter(i => i.name === queueName);
+        if (queueObject.length > 0) {
+            return queueObject[0];
+        } else {
+            return `A queue with name "${initiative.data.queue?.toUpperCase()}" does not exist.`;
+        }
+    }
 
     /**
      * Adds the invoking user to the database.
@@ -179,28 +196,25 @@ export abstract class CommandBase {
      *
      * @access public
      * @static
-     * @param queues - A list of all queues to search through.
      * @param queue - The name of the queue to which to add the user.
      * @param user - The user to add to the queue.
      * @returns The queue that was updated.
      */
-    public static addToQueue (queues: IQueue[], queue: string, user: IPerson): IQueue {
-        LOGGER.verbose(`Queue Name: ${queue}`);
-        LOGGER.verbose(`Queues: ${queues}`);
-        const queueObject = queues.filter(i => i.name === queue.toUpperCase())[0];
+    public static addToQueue (queue: IQueue, user: IPerson): IQueue {
+        LOGGER.verbose(`Queue Name: ${queue.name}`);
         const now = new Date();
-        const atHeadTime = queueObject.members.length === 0 ? now : null;
-        queueObject.members.push({
+        const atHeadTime = queue.members.length === 0 ? now : null;
+        queue.members.push({
             person: user,
             enqueuedAt: now,
             atHeadTime: atHeadTime
         });
-        queueObject.history.push({
-            name: queueObject.name,
-            members: queueObject.members,
+        queue.history.push({
+            name: queue.name,
+            members: queue.members,
             time: new Date()
         });
-        return queueObject;
+        return queue;
     }
 
     /**
@@ -232,31 +246,29 @@ export abstract class CommandBase {
      * @access public
      * @static
      * @async
-     * @param queues - A list of all queues to search through.
      * @param queue - The name of the queue to which to remove the user.
      * @param user - The user to remove from the queue.
      * @param user.id - The user ID for the user to remove from the queue.
      * @param user.displayName - The display name for the user to remove from the queue.
      * @returns The queue that was updated.
      */
-    public static async removeFromQueue (queues: IQueue[], queue: string, user: IRemoveFromQueueUser): Promise<IQueue | string> {
-        const queueObject = queues.filter(i => i.name === queue)[0];
-        const idx = queueObject.members.findIndex(p => p.person.displayName === user.displayName);
+    public static async removeFromQueue (queue: IQueue, user: IRemoveFromQueueUser): Promise<IQueue | string> {
+        const idx = queue.members.findIndex(p => p.person.displayName === user.displayName);
         if (idx === -1) {
-            return `User "${user.displayName}" was not found in queue "${queueObject.name}"`;
+            return `User "${user.displayName}" was not found in queue "${queue.name}"`;
         } else {
-            const removed = queueObject.members.splice(idx, 1)[0];
-            queueObject.history.push({
-                name: queueObject.name,
-                members: queueObject.members,
+            const removed = queue.members.splice(idx, 1)[0];
+            queue.history.push({
+                name: queue.name,
+                members: queue.members,
                 time: new Date()
             });
             const now = new Date().getTime();
 
             // If we removed the person at the head of the queue and
             // there are more people in the queue
-            if (queueObject.members.length > 0 && idx === 0) {
-                const head = queueObject.members[0];
+            if (queue.members.length > 0 && idx === 0) {
+                const head = queue.members[0];
                 head.atHeadTime = new Date();
             }
             // Update person object
@@ -283,7 +295,7 @@ export abstract class CommandBase {
             LOGGER.verbose(await PERSON_MODEL.find({ id: removed.person.id }).exec());
 
             await PERSON_MODEL.updateOne({ id: removed.person.id }, updateData).exec();
-            return queueObject;
+            return queue;
         }
     }
 
diff --git a/services/bot/src/commands/queue/addMe.ts b/services/bot/src/commands/queue/addMe.ts
@@ -27,19 +27,20 @@ export class AddMe extends CommandBase implements ICommand {
         const project = await CommandBase.getProject(initiative);
         if (typeof project === 'string') return String(project);
 
+        // Get queue
+        let queue = await CommandBase.getQueue(initiative, project);
+        if (typeof queue === 'string') return String(queue);
+
         // Create user if they don't exist
         const user = await CommandBase.addUser(initiative.user.id, initiative.user.displayName);
 
-        // Get queue
-        const queueName = initiative.data.queue?.toUpperCase() || project.currentQueue;
-
         // Add to end of queue
-        const queue = CommandBase.addToQueue(project.queues, queueName, user);
+        queue = CommandBase.addToQueue(queue, user);
 
         // Save the project
         await project.save();
 
         // Return response
-        return `Successfully added "${user.displayName}" to queue "${queueName}".\n\n${CommandBase.queueToString(queue)}`;
+        return `Successfully added "${user.displayName}" to queue "${queue.name}".\n\n${CommandBase.queueToString(queue)}`;
     }
 }
diff --git a/services/bot/src/commands/queue/addPerson.ts b/services/bot/src/commands/queue/addPerson.ts
@@ -32,6 +32,10 @@ export class AddPerson extends CommandBase implements ICommand {
         const project = await CommandBase.getProject(initiative) as ProjectDocument;
         if (typeof project === 'string') return String(project);
 
+        // Get queue
+        let queue = await CommandBase.getQueue(initiative, project);
+        if (typeof queue === 'string') return String(queue);
+
         if (initiative.mentions.length > 1) {
             return 'You cannot add more than one person at once';
         } else if (initiative.mentions.length == 0) {
@@ -43,16 +47,13 @@ export class AddPerson extends CommandBase implements ICommand {
         // Create user if they don't exist
         const user = await CommandBase.addUser(webexId, webexPerson.displayName);
 
-        // Get queue
-        const queueName = initiative.data.queue?.toUpperCase() || project.currentQueue;
-
         // Add to end of queue
-        const queue = CommandBase.addToQueue(project.queues, queueName, user);
+        queue = CommandBase.addToQueue(queue, user);
 
         // Save the project
         await project.save();
 
         // Return response
-        return `Successfully added "${user.displayName}" to queue "${queueName}".\n\n${CommandBase.queueToString(queue)}`;
+        return `Successfully added "${user.displayName}" to queue "${queue.name}".\n\n${CommandBase.queueToString(queue)}`;
     }
 }
diff --git a/services/bot/src/commands/queue/removeMe.ts b/services/bot/src/commands/queue/removeMe.ts
@@ -28,11 +28,11 @@ export class RemoveMe extends CommandBase implements ICommand {
         const project = await CommandBase.getProject(initiative);
         if (typeof project === 'string') return String(project);
 
-        // Get queue
-        const queueName = initiative.data.queue?.toUpperCase() || project.currentQueue;
+        let queue = await CommandBase.getQueue(initiative, project);
+        if (typeof queue === 'string') return String(queue);
 
         // remove only the first instance
-        const queue = await CommandBase.removeFromQueue(project.queues, queueName, { id: initiative.user.id, displayName: initiative.user.displayName });
+        queue = await CommandBase.removeFromQueue(queue, { id: initiative.user.id, displayName: initiative.user.displayName });
         if (typeof queue === 'string') return String(queue);
 
         // Save the project
@@ -49,6 +49,6 @@ export class RemoveMe extends CommandBase implements ICommand {
             tag += ', you\'re at the front of the queue!';
         }
         // Return response
-        return `Successfully removed "${initiative.user.displayName}" from queue "${queueName}".\n\n${CommandBase.queueToString(queue)}${tag ? '\n\n' + tag : ''}`;
+        return `Successfully removed "${initiative.user.displayName}" from queue "${queue.name}".\n\n${CommandBase.queueToString(queue)}${tag ? '\n\n' + tag : ''}`;
     }
 }
diff --git a/services/bot/src/commands/queue/removePerson.ts b/services/bot/src/commands/queue/removePerson.ts
@@ -33,7 +33,8 @@ export class RemovePerson extends CommandBase implements ICommand {
         if (typeof project === 'string') return String(project);
 
         // Get queue
-        const queueName = initiative.data.queue?.toUpperCase() || project.currentQueue;
+        let queue = await CommandBase.getQueue(initiative, project);
+        if (typeof queue === 'string') return String(queue);
 
         if (initiative.mentions.length > 1) {
             return 'You cannot remove more than one person at once';
@@ -44,7 +45,7 @@ export class RemovePerson extends CommandBase implements ICommand {
         const webexPerson = await BOT.people.get(webexId);
 
         // remove only the first instance
-        const queue = await CommandBase.removeFromQueue(project.queues, queueName, { id: webexId, displayName: webexPerson.displayName });
+        queue = await CommandBase.removeFromQueue(queue, { id: webexId, displayName: webexPerson.displayName });
         if (typeof queue === 'string') return String(queue);
 
         // Save the project
@@ -61,6 +62,6 @@ export class RemovePerson extends CommandBase implements ICommand {
             tag += ', you\'re at the front of the queue!';
         }
         // Return response
-        return `Successfully removed "${webexPerson.displayName}" from queue "${queueName}".\n\n${CommandBase.queueToString(queue)}${tag ? '\n\n' + tag : ''}`;
+        return `Successfully removed "${webexPerson.displayName}" from queue "${queue.name}".\n\n${CommandBase.queueToString(queue)}${tag ? '\n\n' + tag : ''}`;
     }
 }
diff --git a/services/bot/tests/commands/queue/addMe.test.ts b/services/bot/tests/commands/queue/addMe.test.ts
@@ -28,6 +28,16 @@ describe('Adding me to a queue errors when it should', () => {
         expect(await new AddMe().relax({ ...TEST_INITIATIVE, user: SUPER_ADMIN })).toEqual('There are no projects registered.');
         expect(await PROJECT_MODEL.find({}).exec()).toHaveLength(0);
     });
+    test('errors when non-default queue is specified that does not exist', async () => {
+        const project = await CREATE_PROJECT();
+        const queue = project.queues.filter(i => i.name === project.currentQueue)[0];
+        expect(queue.members).toHaveLength(0);
+        expect(await new AddMe().relax({ ...TEST_INITIATIVE, data: { queue: 'FOO' } })).toEqual(
+            'A queue with name "FOO" does not exist.'
+        );
+        expect(queue.members).toHaveLength(0);
+        expect(await PERSON_MODEL.find({ id: STANDARD_USER.id }).exec()).toHaveLength(0);
+    });
 });
 
 describe('Adding me to a queue works appropriately', () => {
diff --git a/services/bot/tests/commands/queue/addPerson.test.ts b/services/bot/tests/commands/queue/addPerson.test.ts
@@ -46,6 +46,19 @@ describe('Adding a person to a queue errors when it should', () => {
         );
         expect(await PERSON_MODEL.find({ id: USER_ID }).exec()).toHaveLength(0);
     });
+    test('errors when non-default queue is specified that does not exist', async () => {
+        const project = await CREATE_PROJECT();
+        const queue = project.queues.filter(i => i.name === project.currentQueue)[0];
+        expect(queue.members).toHaveLength(0);
+        expect(await new AddPerson().relax({
+            ...TEST_INITIATIVE,
+            data: { queue: 'FOO' },
+            user: PROJECT_ADMIN,
+            mentions: ['1']
+        })).toEqual('A queue with name "FOO" does not exist.');
+        expect(queue.members).toHaveLength(0);
+        expect(await PERSON_MODEL.find({ id: STANDARD_USER.id }).exec()).toHaveLength(0);
+    });
 });
 
 describe('Adding a person to a queue works appropriately', () => {
diff --git a/services/bot/tests/commands/queue/removeMe.test.ts b/services/bot/tests/commands/queue/removeMe.test.ts
@@ -40,6 +40,17 @@ describe('Removing me from a queue works appropriately', () => {
         expect(await PERSON_MODEL.find({ id: STANDARD_USER.id }).exec()).toHaveLength(0);
     });
 
+    test('errors when non-default queue is specified that does not exist', async () => {
+        const project = await CREATE_PROJECT();
+        const queue = project.queues.filter(i => i.name === project.currentQueue)[0];
+        expect(queue.members).toHaveLength(0);
+        expect(await new RemoveMe().relax({ ...TEST_INITIATIVE, data: { queue: 'FOO' } })).toEqual(
+            'A queue with name "FOO" does not exist.'
+        );
+        expect(queue.members).toHaveLength(0);
+        expect(await PERSON_MODEL.find({ id: STANDARD_USER.id }).exec()).toHaveLength(0);
+    });
+
     test('errors when there is someone in the queue but its not the user', async () => {
         const project = await CREATE_PROJECT();
         const queue = project.queues.filter(i => i.name === project.currentQueue)[0];
diff --git a/services/bot/tests/commands/queue/removePerson.test.ts b/services/bot/tests/commands/queue/removePerson.test.ts
@@ -68,6 +68,19 @@ describe('Removing a person from a queue errors when it should', () => {
         project = (await PROJECT_MODEL.find({ name: project.name }).exec())[0];
         expect(project.queues.filter(i => i.name === project.currentQueue)[0].members).toHaveLength(0);
     });
+    test('errors when non-default queue is specified that does not exist', async () => {
+        const project = await CREATE_PROJECT();
+        const queue = project.queues.filter(i => i.name === project.currentQueue)[0];
+        expect(queue.members).toHaveLength(0);
+        expect(await new RemovePerson().relax({
+            ...TEST_INITIATIVE,
+            data: { queue: 'FOO' },
+            user: PROJECT_ADMIN,
+            mentions: ['1', '2']
+        })).toEqual('A queue with name "FOO" does not exist.');
+        expect(queue.members).toHaveLength(0);
+        expect(await PERSON_MODEL.find({ id: STANDARD_USER.id }).exec()).toHaveLength(0);
+    });
 });
 
 describe('Removing a person from a queue works appropriately', () => {

Original file line number	Diff line number	Diff line change
`@@ -28,11 +28,11 @@ export class RemoveMe extends CommandBase implements ICommand {`
`28`	`28`	`const project = await CommandBase.getProject(initiative);`
`29`	`29`	`if (typeof project === 'string') return String(project);`
`30`	`30`
`31`		`- // Get queue`
`32`		`- const queueName = initiative.data.queue?.toUpperCase() \|\| project.currentQueue;`
	`31`	`+ let queue = await CommandBase.getQueue(initiative, project);`
	`32`	`+ if (typeof queue === 'string') return String(queue);`
`33`	`33`
`34`	`34`	`// remove only the first instance`
`35`		`- const queue = await CommandBase.removeFromQueue(project.queues, queueName, { id: initiative.user.id, displayName: initiative.user.displayName });`
	`35`	`+ queue = await CommandBase.removeFromQueue(queue, { id: initiative.user.id, displayName: initiative.user.displayName });`
`36`	`36`	`if (typeof queue === 'string') return String(queue);`
`37`	`37`
`38`	`38`	`// Save the project`
`@@ -49,6 +49,6 @@ export class RemoveMe extends CommandBase implements ICommand {`
`49`	`49`	`tag += ', you\'re at the front of the queue!';`
`50`	`50`	`}`
`51`	`51`	`// Return response`
`52`		- return `Successfully removed "${initiative.user.displayName}" from queue "${queueName}".\n\n${CommandBase.queueToString(queue)}${tag ? '\n\n' + tag : ''}`;
	`52`	+ return `Successfully removed "${initiative.user.displayName}" from queue "${queue.name}".\n\n${CommandBase.queueToString(queue)}${tag ? '\n\n' + tag : ''}`;
`53`	`53`	`}`
`54`	`54`	`}`