Fix bad message handling (#98)

This change set removes the caching of the last received message from the bot's discord client. The old behaviour meant if a message comes in during processing, it would potentially respond to the wrong channel.

The code has been updated to explicitly pass through the source message so the response is sent to the same channel.

Addresses #49

src/client/discord-client.spec.ts

@@ -180,86 +180,82 @@ describe('Discord client wrapper', () => {
     });
 
     describe('message sending', () => {
+      const mockUserName = 'bob-bobertson';
+
+      let mockMessage: IMock<discord.Message>;
+      let mockChannel: IMock<discord.TextChannel>;
+      let mockAuthor: IMock<discord.User>;
+
+      beforeEach(() => {
+        mockChannel = Mock.ofType();
+
+        mockAuthor = Mock.ofType();
+        mockAuthor.setup(a => a.username).returns(() => mockUserName);
+
+        mockMessage = Mock.ofType();
+        mockMessage.setup(m => m.channel).returns(() => mockChannel.object);
+        mockMessage.setup(s => s.author).returns(() => mockAuthor.object);
+      });
+
       it('should queue messages', () => {
         jest.spyOn(untypedClient, 'sendMessage').mockImplementation(() => {});
 
         const messages = ['one', 'two', 'three'];
-        client.queueMessages(messages);
+        client.queueMessages(mockMessage.object, messages);
 
         expect(untypedClient.sendMessage).toHaveBeenCalledTimes(messages.length);
       });
 
       it('should send queued messages', () => {
         let sendCount = 0;
-        const mockChannel = {
-          send: () => {
-            sendCount += 1;
-          }
-        };
-        untypedClient.lastMessage = { channel: mockChannel };
+        mockChannel.setup(c => c.send(It.isAny())).callback(() => (sendCount += 1));
 
         const messages = ['one', 'two', 'three'];
-        client.queueMessages(messages);
+        client.queueMessages(mockMessage.object, messages);
 
-        expect(sendCount).toBe(3);
+        expect(sendCount).toBe(messages.length);
       });
 
       it('should replace user string with last message user name if message is string', () => {
-        const expectedName = 'bob-bobertson';
         let lastMessage = '';
-        const mockChannel = {
-          send: (message: string) => (lastMessage = message)
-        };
-        untypedClient.lastMessage = {
-          channel: mockChannel,
-          author: { username: expectedName }
-        };
+
+        mockChannel.setup(c => c.send(It.isAny())).callback(message => (lastMessage = message));
 
         const messages = ['{£user} {£user} {£user}'];
-        client.queueMessages(messages);
+        client.queueMessages(mockMessage.object, messages);
 
-        expect(lastMessage).toBe(`${expectedName} ${expectedName} ${expectedName}`);
+        expect(lastMessage).toBe(`${mockUserName} ${mockUserName} ${mockUserName}`);
       });
 
       it('should replace name string with bot user name if message is string', () => {
         let lastMessage = '';
-        const mockChannel = {
-          send: (message: string) => (lastMessage = message)
-        };
-        untypedClient.lastMessage = { channel: mockChannel };
+
+        mockChannel.setup(c => c.send(It.isAny())).callback(message => (lastMessage = message));
 
         const messages = ['{£me}'];
-        client.queueMessages(messages);
+        client.queueMessages(mockMessage.object, messages);
 
         expect(lastMessage).toBe(MOCK_BOT_USERNAME);
       });
 
       it('should replace user string with last message user name in content if message is MessageOptions', () => {
-        const expectedName = 'bob-bobertson';
         let lastMessage: string | null | undefined = null;
-        const mockChannel = {
-          send: (message: discord.MessageOptions) => (lastMessage = message.content)
-        };
-        untypedClient.lastMessage = {
-          channel: mockChannel,
-          author: { username: expectedName }
-        };
+
+        mockChannel.setup(c => c.send(It.isAny())).callback(message => (lastMessage = message.content));
 
         const messageOptions = { content: '{£user} {£user} {£user}' };
-        client.queueMessages([messageOptions]);
+        client.queueMessages(mockMessage.object, [messageOptions]);
 
-        expect(lastMessage).toBe(`${expectedName} ${expectedName} ${expectedName}`);
+        expect(lastMessage).toBe(`${mockUserName} ${mockUserName} ${mockUserName}`);
       });
 
       it('should replace name string with bot user name in content if message is MessageOptions', () => {
         let lastMessage: string | null | undefined = null;
-        const mockChannel = {
-          send: (message: discord.MessageOptions) => (lastMessage = message.content)
-        };
-        untypedClient.lastMessage = { channel: mockChannel };
+
+        mockChannel.setup(c => c.send(It.isAny())).callback(message => (lastMessage = message.content));
 
         const messageOptions = { content: '{£me}' };
-        client.queueMessages([messageOptions]);
+        client.queueMessages(mockMessage.object, [messageOptions]);
 
         expect(lastMessage).toBe(MOCK_BOT_USERNAME);
       });

src/client/discord-client.ts

@@ -9,14 +9,12 @@ import { messageEvent, readyEvent } from './discord-events';
 
 export class DiscordClient extends EventEmitter implements Client {
   private client: DiscordApiClient;
-  private lastMessage: Message;
   private connected: boolean;
 
   constructor(private logger: Logger) {
     super();
 
     this.client = null;
-    this.lastMessage = null;
     this.connected = false;
   }
 
@@ -49,8 +47,8 @@ export class DiscordClient extends EventEmitter implements Client {
     return this.client.user;
   }
 
-  public queueMessages(messages: MessageType[]): void {
-    messages.forEach((message: MessageType) => this.sendMessage(message));
+  public queueMessages(source: Message, messages: MessageType[]): void {
+    messages.forEach((message: MessageType) => this.sendMessage(source, message));
   }
 
   public setPresence(data: PresenceData): void {
@@ -75,21 +73,20 @@ export class DiscordClient extends EventEmitter implements Client {
       return;
     }
 
-    this.lastMessage = message;
     this.emit(LifecycleEvents.MESSAGE, message);
   }
 
-  private performTextExpansion(input: string): string {
-    if (this.lastMessage.author) {
-      input = input.replace(/\{£user\}/g, this.lastMessage.author.username);
+  private performTextExpansion(source: Message, input: string): string {
+    if (source.author) {
+      input = input.replace(/\{£user\}/g, source.author.username);
     }
 
     input = input.replace(/\{£me\}/g, this.client.user.username);
 
     return input;
   }
 
-  private sendMessage(message: MessageType): void {
+  private sendMessage(source: Message, message: MessageType): void {
     if (!message) {
       return;
     }
@@ -100,7 +97,7 @@ export class DiscordClient extends EventEmitter implements Client {
         return;
       }
 
-      this.lastMessage.channel.send(this.performTextExpansion(message));
+      source.channel.send(this.performTextExpansion(source, message));
       return;
     }
 
@@ -115,9 +112,9 @@ export class DiscordClient extends EventEmitter implements Client {
     }
 
     if (messageOptions.content) {
-      messageOptions.content = this.performTextExpansion(messageOptions.content);
+      messageOptions.content = this.performTextExpansion(source, messageOptions.content);
     }
 
-    this.lastMessage.channel.send(messageOptions);
+    source.channel.send(messageOptions);
   }
 }