Support editing/removing multipart Slack messages

src/BaseSlackHandler.ts

@@ -50,6 +50,7 @@ export interface ISlackEventMessageAttachment {
 }
 
 export interface ISlackMessageEvent extends ISlackEvent {
+    team?: string;
     team_domain?: string;
     team_id?: string;
     user?: string;

src/BridgedRoom.ts

@@ -241,13 +241,13 @@
             id: `INTEG-${this.inboundId}`,
             matrix_id: this.matrixRoomId,
             remote: {
                 id: this.slackChannelId!,
                 name: this.slackChannelName!,
                 slack_team_id: this.slackTeamId!,
                 slack_type: this.slackType!,
                 slack_private: this.isPrivate!,
                 webhook_uri: this.slackWebhookUri!,
                 puppet_owner: this.puppetOwner!,
             },
             remote_id: this.inboundId,
         };
@@ -256,7 +256,7 @@
     }
 
     public async getClientForRequest(userId: string): Promise<{id: string, client: WebClient}|null> {
         const puppet = await this.main.clientFactory.getClientForUserWithId(this.SlackTeamId!, userId);
         if (puppet) {
             return puppet;
         }
@@ -269,7 +269,7 @@
         return null;
     }
 
     public async onMatrixReaction(message: any): Promise<void> {
         const relatesTo = message.content["m.relates_to"];
         const eventStore = this.main.datastore;
         const event = await eventStore.getEventByMatrixId(message.room_id, relatesTo.event_id);
@@ -699,8 +699,9 @@
                 await new Promise((r) => setTimeout(r, PUPPET_INCOMING_DELAY_MS));
             }
         }
-        if (this.recentSlackMessages.includes(message.ts)) {
+        if (this.recentSlackMessages.includes(message.event_ts ?? message.ts)) {
             // We sent this, ignore.
+            log.debug('Ignoring message recently sent by us');
             return;
         }
         try {
@@ -712,8 +713,8 @@
             }
             this.slackSendLock = this.slackSendLock.then(() => {
                 // Check again
-                if (this.recentSlackMessages.includes(message.ts)) {
-                    // We sent this, ignore
+                if (this.recentSlackMessages.includes(message.event_ts ?? message.ts)) {
+                    log.debug('Ignoring message recently sent by us');
                     return;
                 }
                 return this.handleSlackMessage(message, ghost).catch((ex) => {
@@ -878,7 +879,7 @@
                 formatted_body: `<a href="${link}">${file.name}</a>`,
                 msgtype: "m.text",
             };
-            await ghost.sendMessage(this.matrixRoomId, messageContent, channelId, slackEventId);
+            await ghost.sendMessage(this.matrixRoomId, messageContent, channelId, slackEventId, { attachment_id: file.id });
             return;
         }
 
@@ -917,7 +918,7 @@
                 formatted_body: htmlCode,
                 msgtype: "m.text",
             };
-            await ghost.sendMessage(this.matrixRoomId, messageContent, channelId, slackEventId);
+            await ghost.sendMessage(this.matrixRoomId, messageContent, channelId, slackEventId, { attachment_id: file.id });
             return;
         }
 
@@ -954,6 +955,7 @@
             slackFileToMatrixMessage(file, fileContentUri, thumbnailContentUri),
             channelId,
             slackEventId,
+            { attachment_id: file.id },
         );
     }
 
@@ -1018,6 +1020,30 @@
             const newMessageRich = substitutions.slackToMatrix(message.text!);
             const newMessage = ghost.prepareBody(newMessageRich);
 
+            // Check if any of the attachments have been deleted.
+            // Slack unfortunately puts a "tombstone" in both message versions in this event,
+            // so let's try to remove every single one even if we may have deleted it before.
+            for (const file of message.message?.files ?? []) {
+                if (file.mode === 'tombstone') {
+                    const events = await this.main.datastore.getEventsBySlackId(channelId, message.previous_message!.ts);
+                    const event = events.find(e => e._extras.attachment_id === file.id);
+                    if (event) {
+                        const team = message.team_id ? await this.main.datastore.getTeam(message.team_id) : null;
+                        if (!team) {
+                            log.warn("Cannot determine team for message", message, "so we cannot delete attachment", file.id);
+                            continue;
+                        }
+                        try {
+                            await this.deleteMessage(message, event, team);
+                        } catch (err) {
+                            log.warn(`Failed to delete attachment ${file.id}:`, err);
+                        }
+                    } else {
+                        log.warn(`Tried to remove tombstoned attachmend ${file.id} but we didn't find a Matrix event for it`);
+                    }
+                }
+            }
+
             // The substitutions might make the messages the same
             if (previousMessage === newMessage) {
                 log.debug("Ignoring edit message because messages are the same post-substitutions.");
@@ -1238,6 +1264,39 @@
             this.recentSlackMessages.shift();
         }
     }
+
+    public async deleteMessage(msg: ISlackMessageEvent, event: EventEntry, team: TeamEntry): Promise<void> {
+        const previousMessage = msg.previous_message;
+        if (!previousMessage) {
+            throw new Error(`Cannot delete message with no previous_message: ${JSON.stringify(msg)}`);
+        }
+
+        // Try to determine the Matrix user responsible for deleting the message, fallback to our main bot if all else fails
+        if (!previousMessage.user) {
+            log.warn("We don't know the original sender of", previousMessage, "will try to remove with our bot");
+        }
+
+        const isOurMessage = previousMessage.subtype === 'bot_message' && (previousMessage.bot_id === team.bot_id);
+
+        if (previousMessage.user && !isOurMessage) {
+            try {
+                const ghost = await this.main.ghostStore.get(previousMessage.user, previousMessage.team_domain, previousMessage.team);
+                await ghost.redactEvent(event.roomId, event.eventId);
+                return;
+            } catch (err) {
+                log.warn(`Failed to remove message on behalf of ${previousMessage.user}, falling back to our bot`);
+            }
+        }
+
+        try {
+            const botClient = this.main.botIntent.matrixClient;
+            await botClient.redactEvent(event.roomId, event.eventId, "Deleted on Slack");
+        } catch (err) {
+            throw new Error(
+                `Failed to remove message ${JSON.stringify(previousMessage)} with our Matrix bot. insufficient power level? Error: ${err}`
+            );
+        }
+    }
 }
 
 /**

src/SlackEventHandler.ts

@@ -18,6 +18,7 @@
 import { BridgedRoom } from "./BridgedRoom";
 import { Main, METRIC_RECEIVED_MESSAGE } from "./Main";
 import { Logger } from "matrix-appservice-bridge";
+import { EventEntry, TeamEntry } from "./datastore/Models";
 const log = new Logger("SlackEventHandler");
 
 /**
@@ -283,17 +284,18 @@
             if (msg.message.bot_id !== undefined) {
                 // Check the edit wasn't sent by us
                 if (msg.message.bot_id === team.bot_id) {
+                    log.debug('Ignoring a message_changed since it was sent by us');
                     return;
                 } else {
                     msg.user_id = msg.message.bot_id;
                 }
             }
         } else if (msg.subtype === "message_deleted" && msg.deleted_ts) {
-            const originalEvent = await this.main.datastore.getEventBySlackId(msg.channel, msg.deleted_ts);
-            if (originalEvent) {
-                const botClient = this.main.botIntent.matrixClient;
-                await botClient.redactEvent(originalEvent.roomId, originalEvent.eventId);
-                return;
+            try {
+                const events = await this.main.datastore.getEventsBySlackId(msg.channel, msg.deleted_ts!);
+                await Promise.all(events.map(event => room.deleteMessage(msg, event, team)));
+            } catch (err) {
+                log.error(err);
             }
             // If we don't have the event
             throw Error("unknown_message");

src/SlackGhost.ts

@@ -19,7 +19,7 @@ import * as Slackdown from "Slackdown";
 import { ISlackUser } from "./BaseSlackHandler";
 import { WebClient } from "@slack/web-api";
 import { BotsInfoResponse, UsersInfoResponse } from "./SlackResponses";
-import { UserEntry, Datastore } from "./datastore/Models";
+import { UserEntry, Datastore, EventEntryExtra } from "./datastore/Models";
 import axios from "axios";
 
 const log = new Logger("SlackGhost");
@@ -321,6 +321,13 @@ export class SlackGhost {
         return Slackdown.parse(body);
     }
 
+    public async redactEvent(roomId: string, eventId: string) {
+        if (!this._intent) {
+            throw Error('No intent associated with ghost');
+        }
+        await this._intent.matrixClient.redactEvent(roomId, eventId);
+    }
+
     public async sendInThread(roomId: string, text: string, slackRoomId: string,
         slackEventTs: string, replyEvent: IMatrixReplyEvent): Promise<void> {
         const content = {
@@ -368,7 +375,13 @@ export class SlackGhost {
         await this.sendMessage(roomId, content, slackRoomID, slackEventTS);
     }
 
-    public async sendMessage(roomId: string, msg: Record<string, unknown>, slackRoomId: string, slackEventTs: string): Promise<{event_id: string}> {
+    public async sendMessage(
+        roomId: string,
+        msg: Record<string, unknown>,
+        slackRoomId: string,
+        slackEventTs: string,
+        eventExtras?: EventEntryExtra,
+    ): Promise<{event_id: string}> {
         if (!this._intent) {
             throw Error('No intent associated with ghost');
         }
@@ -383,6 +396,7 @@ export class SlackGhost {
             matrixEvent.event_id,
             slackRoomId,
             slackEventTs,
+            eventExtras,
         );
 
         return {

src/datastore/Models.ts

@@ -50,6 +50,7 @@ export interface EventEntry {
 }
 
 export interface EventEntryExtra {
+    attachment_id?: string;
     slackThreadMessages?: string[];
 }
 
@@ -113,6 +114,7 @@ export interface Datastore extends ProvisioningStore {
     upsertEvent(roomId: string, eventId: string, channelId: string, ts: string, extras?: EventEntryExtra): Promise<null>;
     upsertEvent(roomIdOrEntry: EventEntry): Promise<null>;
     getEventByMatrixId(roomId: string, eventId: string): Promise<EventEntry|null>;
+    getEventsBySlackId(channelId: string, ts: string): Promise<EventEntry[]>;
     getEventBySlackId(channelId: string, ts: string): Promise<EventEntry|null>;
     deleteEventByMatrixId(roomId: string, eventId: string): Promise<null>;
 

src/datastore/NedbDatastore.ts

@@ -241,6 +241,10 @@ export class NedbDatastore implements Datastore {
         return this.storedEventToEventEntry(storedEvent);
     }
 
+    public async getEventsBySlackId(channelId: string, ts: string): Promise<EventEntry[]> {
+        return this.getEventBySlackId(channelId, ts).then(e => e ? [e] : []);
+    }
+
     public async getEventBySlackId(channelId: string, ts: string): Promise<EventEntry|null> {
         const storedEvent = await this.eventStore.getEntryByRemoteId(channelId, ts);
         if (!storedEvent) {

src/datastore/postgres/PgDatastore.ts

@@ -64,7 +64,7 @@ export interface SchemaRunUserMessage {
 type SchemaRunFn = (db: IDatabase<unknown>) => Promise<void|{userMessages: SchemaRunUserMessage[]}>;
 
 export class PgDatastore implements Datastore, ClientEncryptionStore, ProvisioningStore {
-    public static readonly LATEST_SCHEMA = 16;
+    public static readonly LATEST_SCHEMA = 17;
     public readonly postgresDb: IDatabase<any>;
 
     constructor(connectionString: string) {
@@ -178,17 +178,25 @@ export class PgDatastore implements Datastore, ClientEncryptionStore, Provisioni
             });
     }
 
+    public async getEventsBySlackId(slackChannel: string, slackTs: string): Promise<EventEntry[]> {
+        return this.postgresDb.manyOrNone(
+            "SELECT * FROM events WHERE slackChannel = ${slackChannel} AND slackTs = ${slackTs}",
+            { slackChannel, slackTs }
+        ).then(entries => entries.map(e => ({
+            roomId: e.roomid,
+            eventId: e.eventid,
+            slackChannelId: slackChannel,
+            slackTs,
+            _extras: JSON.parse(e.extras) as EventEntryExtra,
+        })));
+    }
+
+    /**
+     * @deprecated One Slack event may map to many Matrix events -- use getEventsBySlackId()
+     */
     public async getEventBySlackId(slackChannel: string, slackTs: string): Promise<EventEntry|null> {
-        log.debug(`getEventBySlackId: ${slackChannel} ${slackTs}`);
-        return this.postgresDb.oneOrNone(
-            "SELECT * FROM events WHERE slackChannel = ${slackChannel} AND slackTs = ${slackTs} LIMIT 1",
-            { slackChannel, slackTs }, e => e && {
-                roomId: e.roomid,
-                eventId: e.eventid,
-                slackChannelId: slackChannel,
-                slackTs,
-                _extras: JSON.parse(e.extras),
-            });
+        const events = await this.getEventsBySlackId(slackChannel, slackTs);
+        return events.find(e => !e._extras.attachment_id) ?? null;
     }
 
     public async deleteEventByMatrixId(roomId: string, eventId: string): Promise<null> {

src/datastore/postgres/schema/v17.ts

@@ -0,0 +1,11 @@
+import { IDatabase } from "pg-promise";
+
+export const runSchema = async(db: IDatabase<unknown>) => {
+    await db.none(`
+        ALTER TABLE events DROP CONSTRAINT cons_events_unique;
+    `);
+
+    await db.none(`
+        ALTER TABLE events ADD CONSTRAINT cons_events_unique UNIQUE(eventid, roomid, slackchannel, slackts, extras);
+    `);
+};