Implement bridging of plain text matrix mentions #99

Cadair · 2018-10-07T16:40:24Z

The Slack API removed the ability to just send slack @Cadair text and have slack convert it into a mention. This is an attempt to do this substitution automatically in the bridge.

~~I would like to remove the need for the @ here as well, which I have an idea how to do, but I am a little concerned about the performance impacts of all this.~~

This now correctly bridges plain text matrix mentions as slack mentions. This means that all matrix clients should give the correct behaviour on slack, making the bridge truly transparent. 🎉

This also builds on #98 because of many many conflicts.

Cadair · 2018-10-08T15:52:33Z

My concern with this as implemented is the potential effect on performance. This

matrix-appservice-slack/lib/substitutions.js

Lines 248 to 254 in 9975146

    
           const firstwords = makeFirstWordMap(displaymap); 
        
           // Now parse the message to find the intersection of every word in the 
        
           // message with every first word of all nicks. 
        
           const match_words = new Set(Object.keys(firstwords)); 
        
           const string_words = new Set(string.split(" ")); 
        
           const matches = [...string_words].filter(x => match_words.has(x));

block of code is being executed for each message, irrespective of if there is a match or not (this was not the case when matching @), which means that this:

matrix-appservice-slack/lib/substitutions.js

Lines 197 to 210 in 9975146

    
           function getDisplayMap(main, room_id) { 
        
               const store = main.getUserStore(); 
        
               return main.listGhostUsers(room_id) 
        
                   .then(users => Promise.all(users.map(user => store.select({id: user})))) 
        
                   .then(users => { 
        
                       const displaymap = {}; 
        
                       users.forEach(user => { 
        
                           if (user && user[0]) { 
        
                               displaymap[user[0].display_name] = user[0].id.split("_")[2].split(":")[0]; 
        
                           } 
        
                       }); 
        
                       return displaymap; 
        
                   }); 
        
           }

function, which calls out to room state and the user store is also being hit on each message.

I don't know enough about the way the bridge lib handles caching of both the room state store or the user store to know if this is an issue.

Cadair · 2018-10-08T16:04:45Z

This does also have the downside of being unable to handle the situation where you have two users on slack with the same display name. I guess the only way to semi-guard against this is to do some crazy regex on nicks of the form Cadair (@slack_cadair_UDAS26GDC:testmatrix.home.cadair.com) but I don't really fancy that frankly.

turt2live · 2018-10-16T05:16:49Z

(merged develop to make review easier - don't mind me)

turt2live

This looks largely okay to me, but @Half-Shot probably knows what is going on here more than I do

Half-Shot

Apart from the vars and one name change, I'm happy :)

Half-Shot · 2018-10-16T07:59:25Z

lib/SlackGhost.js

@@ -3,7 +3,7 @@
 var url = require('url');
 var https = require('https');
 var rp = require('request-promise');
-const Slackdown = require('Slackdown');
+const slackdown = require('Slackdown');


but why? :(

Half-Shot · 2018-10-17T12:01:45Z

lib/BridgedRoom.js

+        substitutions.matrixToSlack(message, this._main).then(body => {
+            var uri = (this._slack_bot_token) ? "https://slack.com/api/chat.postMessage" : this._slack_webhook_uri;
+
+            var sendMessageParams = {


Half-Shot · 2018-10-17T12:01:49Z

lib/BridgedRoom.js

-            sendMessageParams.headers = {
-                Authorization: 'Bearer ' + this._slack_bot_token
+        substitutions.matrixToSlack(message, this._main).then(body => {
+            var uri = (this._slack_bot_token) ? "https://slack.com/api/chat.postMessage" : this._slack_webhook_uri;


Half-Shot · 2018-10-17T12:02:01Z

lib/substitutions.js

+    // Note: This slower plainTextSlackMentions call is only used if there is not a pill in the message,
+    // meaning if we can we use the much simpler pill subs rather than this.
+    return plainTextSlackMentions(main, string, event.room_id).then(string => {
+        var ret = {


Half-Shot · 2018-10-17T12:02:07Z

lib/substitutions.js

+            link_names: 1  //This no longer works for nicks but is needed to make @channel work.
+        };
+        if (event.content.msgtype == "m.image" && event.content.url.indexOf("mxc://") === 0) {
+            var url = main.getUrlForMxc(event.content.url);


Half-Shot · 2018-10-17T12:02:40Z