diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 42d306d..a501a65 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -11,6 +11,8 @@ in development (improvement) * Split out the functionality of ``src/stackstorm.js`` into ``stackstorm_api.js`` and refactor it to be a JS old style class with a wrapper [PR #187, PR #190] (improvement) +* Fix the Mattermost adapter to use the ``adapter.post`` function instead of emitting the + ``slack-attachment`` event [PR #192] (bug fix) 0.9.6 ----- diff --git a/src/lib/adapters/mattermost.js b/src/lib/adapters/mattermost.js index d1a3957..cb575d0 100644 --- a/src/lib/adapters/mattermost.js +++ b/src/lib/adapters/mattermost.js @@ -17,6 +17,7 @@ var env = process.env; var util = require('util'); var utils = require('./../utils'); +var messages = require('./../slack-messages'); var SlackLikeAdapter = require('./slack-like'); @@ -29,15 +30,21 @@ util.inherits(MattermostAdapter, SlackLikeAdapter); MattermostAdapter.prototype.postData = function(data) { var self = this; - - var recipient, attachment_color, split_message, + var attachment_color, split_message, envelope, attachment, pretext = ""; + // We capture robot here so the `sendChunk` closure captures the correct + // `this` + var robot = self.robot; // If we are supposed to whisper to a single user, use a direct message if (data.whisper && data.user) { - recipient = data.user; + envelope = { + room: data.user + }; } else { // Otherwise, message the channel - recipient = data.channel; + envelope = { + room: data.channel + }; // If we aren't supposed to whisper, then we at least at-mention the user pretext = (data.user && !data.whisper) ? util.format('@%s: ', data.user) : ""; } @@ -56,7 +63,6 @@ MattermostAdapter.prototype.postData = function(data) { } split_message = utils.splitMessage(self.formatData(data.message)); - if (split_message.text) { // Default values var content = { @@ -65,12 +71,57 @@ MattermostAdapter.prototype.postData = function(data) { }; // Override the default values with values from `data.extra.mattermost` if (data.extra && data.extra.mattermost) { + // Backwards compatibility + + // Action: + // + // result: + // format: ... + // message: Message text + // extra: + // mattermost: + // author_name: Jira_Bot + // author_link: "https://stackstorm.com" + // author_icon: "https://stackstorm.com/favicon.ico" + // color: "#042A60" + // fallback: "Info about Jira ticket {{ execution.result.result.key }}" + // title: "{{ execution.result.result.key }}" + // title_link: "{{ execution.result.result.url }}" + // fields: + // - + // title: Summary + // value: "{{ execution.result.result.summary }}" + // short: false + // + // becomes: + // + // { + // "message": "Message text", + // "props": { + // "attachments": [ + // { + // "author_name": "Jira Bot", + // "author_link": "https://stackstorm.com", + // "author_icon": "https://stackstorm.com/favicon.ico", + // "color": "#042A60", + // "fallback": "Info about Jira ticket {{ execution.result.result.key }}", + // "title": "{{ execution.result.result.key }}", + // "title_link": "{{ execution.result.result.url }}", + // "fields": [ + // { + // "title": "Summary", + // "value": "{{ execution.result.result.summary }}", + // "short": false + // } + // ] + // } + // ] + // } + // } + for (var attrname in data.extra.mattermost) { content[attrname] = data.extra.mattermost[attrname]; } } - // We capture robot here so the `sendMessage` closure captures the correct - // `this` - var robot = self.robot; var chunks = split_message.text.match(/[\s\S]{1,3800}/g); // We define a recursive closure that calls itself with the next data to @@ -85,23 +136,25 @@ MattermostAdapter.prototype.postData = function(data) { Inorder to accept the matteruser adapter message attachments changed the attachement json */ - attachment = { - room: recipient, - attachments: content.attachments ? content.attachments : content, + var message = { + props: { + attachments: (content.attachments ? content.attachments : [content]) + }, // There is likely a bug here - `split_message.text` being a true-y // value does not imply that `split_message.pretext` is also non-empty, // but we unconditionally set `text` to // `pretext + split_message.pretext` on the first message - text: i === 0 ? pretext + split_message.pretext : null + message: i === 0 ? pretext + split_message.pretext : null }; - robot.emit('slack-attachment', attachment); + + robot.adapter.send(envelope, message); if (chunks.length > ++i) { setTimeout(function(){ sendChunk(i); }, 300); } }; sendChunk(0); } else { - self.robot.messageRoom.call(self.robot, recipient, pretext + split_message.pretext); + robot.messageRoom.call(self.robot, envelope.room, pretext + split_message.pretext); } }; diff --git a/src/lib/slack-messages.js b/src/lib/slack-messages.js index 52afe04..6161199 100644 --- a/src/lib/slack-messages.js +++ b/src/lib/slack-messages.js @@ -16,6 +16,11 @@ var utils = require('./utils.js'); +// NOTE: This is being reused between the Slack adapter and the Mattermost +// adapter. If we need to tweak it for Mattermost, we should refactor it +// into a class, then inherit from it and override specific methods in +// Mattermost adapter. + var buildMessagesWithChunkedFieldValue = function (msg) { var msgs; diff --git a/test/dummy-adapters.js b/test/dummy-adapters.js index a9d5ddd..2e89021 100644 --- a/test/dummy-adapters.js +++ b/test/dummy-adapters.js @@ -27,6 +27,14 @@ function SlackBot(logger) { this.client = new MockSlackClient(logger); } +function MockMattermostAdapter(logger) { + this.logger = logger; +} + +MockMattermostAdapter.prototype.send = function(envelope, message) { + this.logger.debug('Sending ' + JSON.stringify(message) + ' to ' + JSON.stringify(envelope)); +}; + function MockBotFrameworkAdapter(logger) { this.logger = logger; } @@ -35,5 +43,8 @@ MockBotFrameworkAdapter.prototype.send = function(envelope, message) { this.logger.info('Sending ' + JSON.stringify(message) + ' to ' + JSON.stringify(envelope)); }; -module.exports.MockSlackAdapter = SlackBot; -module.exports.MockBotFrameworkAdapter = MockBotFrameworkAdapter; +module.exports = { + MockSlackAdapter: SlackBot, + MockMattermostAdapter: MockMattermostAdapter, + MockBotFrameworkAdapter: MockBotFrameworkAdapter +}; diff --git a/test/test-postdata.js b/test/test-postdata.js index ce1ba21..9ce6b5f 100644 --- a/test/test-postdata.js +++ b/test/test-postdata.js @@ -29,6 +29,7 @@ var chai = require("chai"), util = require('util'); var MockSlackAdapter = dummyAdapters.MockSlackAdapter; +var MockMattermostAdapter = dummyAdapters.MockMattermostAdapter; var MockBotFrameworkAdapter = dummyAdapters.MockBotFrameworkAdapter; chai.use(sinonChai); @@ -147,26 +148,26 @@ describe("slack post data", function() { 'succeeded', '1'), input = { - user: 'stanley', - channel: '#stackstorm', - message: message, - whisper: false, - extra: { - slack: { - icon_emoji: ":slack:", - username: "SlackBot", - attachments: [ - { - color: "dfdfdf", - mrkdwn_in: ["text","pretext"], - pretext: "@stanley: ", - text: message, - fallback: message + user: 'stanley', + channel: '#stackstorm', + message: message, + whisper: false, + extra: { + slack: { + icon_emoji: ":slack:", + username: "SlackBot", + attachments: [ + { + color: "dfdfdf", + mrkdwn_in: ["text","pretext"], + pretext: "@stanley: ", + text: message, + fallback: message + } + ] } - ] - } - } - }; + } + }; adapter.postData(input); expect(robot.adapter.client.send).to.have.been.calledOnce; expect(robot.adapter.client.send).to.have.been.calledWith( @@ -183,26 +184,26 @@ describe("slack post data", function() { 'succeeded', '1'), input = { - user: 'stanley', - channel: '#stackstorm', - message: message, - whisper: false, - extra: { - slack: { - icon_emoji: ":slack:", - username: "SlackBot", - attachments: [ - { - color: "5fff5f", - text: 'A'+(new Array(3500).join('B'))+'C' - }, { - color: "ff5f5f", - text: 'X'+(new Array(3500).join('Y'))+'Z' + user: 'stanley', + channel: '#stackstorm', + message: message, + whisper: false, + extra: { + slack: { + icon_emoji: ":slack:", + username: "SlackBot", + attachments: [ + { + color: "5fff5f", + text: 'A'+(new Array(3500).join('B'))+'C' + }, { + color: "ff5f5f", + text: 'X'+(new Array(3500).join('Y'))+'Z' + } + ] } - ] - } - } - }; + } + }; adapter.postData(input); expect(robot.adapter.client.send).to.have.been.calledWith( { "id": "#stackstorm", "room": "#stackstorm", "user": "stanley" }, @@ -228,21 +229,21 @@ describe("slack post data", function() { 'succeeded', '1'), input = { - user: 'stanley', - channel: '#stackstorm', - message: message, - whisper: false, - extra: { - color: "e5e5e5", - slack: { - color: "dfdfdf", - mrkdwn_in: ["text","pretext"], - pretext: "@stanley: ", - text: message, - fallback: message - } - } - }; + user: 'stanley', + channel: '#stackstorm', + message: message, + whisper: false, + extra: { + color: "e5e5e5", + slack: { + color: "dfdfdf", + mrkdwn_in: ["text","pretext"], + pretext: "@stanley: ", + text: message, + fallback: message + } + } + }; adapter.postData(input); expect(robot.adapter.client.send).to.have.been.calledOnce; expect(robot.adapter.client.send).to.have.been.calledWith( @@ -418,14 +419,14 @@ describe("msteams post data", function () { describe("mattermost post data", function() { var logger = new Log('info'); - var robot = new Robot(false, new MockSlackAdapter(logger)); + var robot = new Robot(false, new MockMattermostAdapter(logger)); var adapter = adapters.getAdapter('mattermost', robot); env.ST2_MATTERMOST_SUCCESS_COLOR = 'dfdfdf'; env.ST2_MATTERMOST_FAIL_COLOR = 'danger'; it('should post to room and mention a user', function() { - robot.emit = sinon.spy(); + robot.adapter.send = sinon.spy(); var input = { user: 'stanley', channel: '#stackstorm', @@ -435,24 +436,27 @@ describe("mattermost post data", function() { var user = util.format('@%s: ', input.user); adapter.postData(input); - expect(robot.emit).to.have.been.calledOnce; - expect(robot.emit).to.have.been.calledWith( - 'slack-attachment', + expect(robot.adapter.send).to.have.been.calledOnceWith( { - attachments: { - color: env.ST2_MATTERMOST_SUCCESS_COLOR, - fallback: "normal boring text", - mrkdwn_in: ["text", "pretext"], - text: "normal boring text" + room: '#stackstorm' + }, + { + props: { + attachments: [ + { + color: env.ST2_MATTERMOST_SUCCESS_COLOR, + fallback: "normal boring text", + mrkdwn_in: ["text", "pretext"], + text: "normal boring text" + } + ] }, - room: input.channel, - text: user + "NORMAL PRETEXT" - } - ); + message: user + "NORMAL PRETEXT" + }); }); it('should post to room and not mention a user', function() { - robot.emit = sinon.spy(); + robot.adapter.send = sinon.spy(); var input = { channel: '#stackstorm', message: util.format('NORMAL PRETEXT{~}normal boring text'), @@ -461,20 +465,23 @@ describe("mattermost post data", function() { var user = util.format('@%s: ', input.user); adapter.postData(input); - expect(robot.emit).to.have.been.calledOnce; - expect(robot.emit).to.have.been.calledWith( - 'slack-attachment', + expect(robot.adapter.send).to.have.been.calledOnceWith( + { + room: input.channel + }, { - attachments: { - color: env.ST2_MATTERMOST_SUCCESS_COLOR, - fallback: "normal boring text", - mrkdwn_in: ["text", "pretext"], - text: "normal boring text" + props: { + attachments: [ + { + color: env.ST2_MATTERMOST_SUCCESS_COLOR, + fallback: "normal boring text", + mrkdwn_in: ["text", "pretext"], + text: "normal boring text" + } + ] }, - room: input.channel, - text: "NORMAL PRETEXT" - } - ); + message: "NORMAL PRETEXT" + }); }); it('should just post messgae with pretext to room', function() { @@ -496,7 +503,7 @@ describe("mattermost post data", function() { }); it('should post success formatted slack attachment', function() { - robot.emit = sinon.spy(); + robot.adapter.send = sinon.spy(); var input = { user: 'stanley', channel: '#stackstorm', @@ -509,25 +516,28 @@ describe("mattermost post data", function() { var user = util.format('@%s: ', input.user); adapter.postData(input); - expect(robot.emit).to.have.been.calledOnce; - expect(robot.emit).to.have.been.calledWith( - 'slack-attachment', + expect(robot.adapter.send).to.have.been.calledOnceWith( + { + room: input.channel + }, { - attachments: { - color: env.ST2_MATTERMOST_SUCCESS_COLOR, - fallback: input.message, - mrkdwn_in: ["text", "pretext"], - text: input.message + props: { + attachments: [ + { + color: env.ST2_MATTERMOST_SUCCESS_COLOR, + fallback: input.message, + mrkdwn_in: ["text", "pretext"], + text: input.message + } + ] }, - room: input.channel, - text: user - } - ); + message: user + }); }); it('should split a long attachment into chunks', function() { this.clock = sinon.useFakeTimers(); - robot.emit = sinon.spy(); + robot.adapter.send = sinon.spy(); var input = { user: 'stanley', channel: '#stackstorm', @@ -538,155 +548,167 @@ describe("mattermost post data", function() { chunks = input.message.match(/[\s\S]{1,3800}/g); adapter.postData(input); - expect(robot.emit).to.have.been.calledWith( - 'slack-attachment', + expect(robot.adapter.send).to.have.been.calledWith( { - attachments: { - color: env.ST2_MATTERMOST_SUCCESS_COLOR, - fallback: input.message, - mrkdwn_in: ["text", "pretext"], - text: chunks[0], - fallback: chunks[0] + room: input.channel + }, + { + props: { + attachments: [ + { + color: env.ST2_MATTERMOST_SUCCESS_COLOR, + fallback: chunks[0], + mrkdwn_in: ["text", "pretext"], + text: chunks[0] + } + ] }, - room: input.channel, - text: user - } - ); + message: user + }); this.clock.tick(500); - expect(robot.emit).to.have.been.calledWith( - 'slack-attachment', + expect(robot.adapter.send).to.have.been.calledWith( { - attachments: { - color: env.ST2_MATTERMOST_SUCCESS_COLOR, - fallback: input.message, - mrkdwn_in: ["text", "pretext"], - text: chunks[1], - fallback: chunks[1] + room: input.channel + }, + { + props: { + attachments: [ + { + color: env.ST2_MATTERMOST_SUCCESS_COLOR, + fallback: chunks[1], + mrkdwn_in: ["text", "pretext"], + text: chunks[1] + } + ] }, - room: input.channel, - text: user - } - ); - expect(robot.emit).to.have.been.calledTwice; + message: user + }); + this.clock.tick(500); + expect(robot.adapter.send).to.have.been.calledTwice; this.clock.restore(); }); it('should post success with custom color', function() { - robot.emit = sinon.spy(); + robot.adapter.send = sinon.spy(); var message = util.format('%s\nstatus : %s\nexecution: %s', 'Short message', 'succeeded', '1'), input = { - user: 'stanley', - channel: '#stackstorm', - message: message, - whisper: false, - extra: { - color: 'CUSTOM_COLOR' - } - }; + user: 'stanley', + channel: '#stackstorm', + message: message, + whisper: false, + extra: { + color: 'CUSTOM_COLOR' + } + }; var user = util.format('@%s: ', input.user); adapter.postData(input); - expect(robot.emit).to.have.been.calledOnce; - expect(robot.emit).to.have.been.calledWith( - 'slack-attachment', + expect(robot.adapter.send).to.have.been.calledOnceWith( { - attachments: { - color: 'CUSTOM_COLOR', - fallback: input.message, - mrkdwn_in: ["text", "pretext"], - text: input.message + room: '#stackstorm' + }, + { + props: { + attachments: [ + { + color: 'CUSTOM_COLOR', + fallback: input.message, + mrkdwn_in: ["text", "pretext"], + text: input.message + } + ], }, - room: input.channel, - text: user - } - ); + message: user + }); }); it('should post success formatted slack attachment with extra', function() { - robot.emit = sinon.spy(); + robot.adapter.send = sinon.spy(); var message = util.format('%s\nstatus : %s\nexecution: %s', 'Short message', 'succeeded', '1'), input = { - user: 'stanley', - channel: '#stackstorm', - message: message, - whisper: false, - extra: { - mattermost: { - icon_emoji: ":mattermost:", - username: "MattermostBot", - attachments: [ - { - color: "dfdfdf", - mrkdwn_in: ["text","pretext"], - pretext: "@stanley: ", - text: message, - fallback: message + user: 'stanley', + channel: '#stackstorm', + message: message, + whisper: false, + extra: { + mattermost: { + icon_emoji: ":mattermost:", + username: "MattermostBot", + attachments: [ + { + color: "dfdfdf", + mrkdwn_in: ["text","pretext"], + pretext: "@stanley: ", + text: message, + fallback: message + } + ] } - ] - } - } - }; + } + }; adapter.postData(input); - expect(robot.emit).to.have.been.calledOnce; - expect(robot.emit).to.have.been.calledWith( - 'slack-attachment', + expect(robot.adapter.send).to.have.been.calledOnceWith( { - attachments: input.extra.mattermost.attachments, - room: input.channel, - text: "@stanley: " - } - ); + room: input.channel + }, + { + props: { + attachments: input.extra.mattermost.attachments + }, + message: "@stanley: " + }); }); it('should whisper a slack-attachment', function() { - robot.emit = sinon.spy(); + robot.adapter.send = sinon.spy(); var message = util.format('%s\nstatus : %s\nexecution: %s', 'Short message', 'succeeded', '1'), input = { - user: 'stanley', - channel: '#stackstorm', - message: message, - whisper: true, - extra: { - mattermost: { - icon_emoji: ":mattermost:", - username: "MattermostBot", - attachments: [ - { - color: "dfdfdf", - mrkdwn_in: ["text","pretext"], - pretext: "@stanley: ", - text: message, - fallback: message + user: 'stanley', + channel: '#stackstorm', + message: message, + whisper: true, + extra: { + mattermost: { + icon_emoji: ":mattermost:", + username: "MattermostBot", + attachments: [ + { + color: "dfdfdf", + mrkdwn_in: ["text","pretext"], + pretext: "@stanley: ", + text: message, + fallback: message + } + ] } - ] - } - } - }; + } + }; adapter.postData(input); - expect(robot.emit).to.have.been.calledOnce; - expect(robot.emit).to.have.been.calledWith( - 'slack-attachment', + expect(robot.adapter.send).to.have.been.calledOnceWith( { - attachments: input.extra.mattermost.attachments, - room: input.user, - text: '' - } - ); + room: input.user + }, + { + props: { + attachments: input.extra.mattermost.attachments, + }, + message: '' + }); }); it('should post fail formatted slack attachment', function() { - robot.emit = sinon.spy(); + robot.adapter.send = sinon.spy(); var input = { user: 'stanley', channel: '#stackstorm', @@ -699,20 +721,23 @@ describe("mattermost post data", function() { var user = util.format('@%s: ', input.user); adapter.postData(input); - expect(robot.emit).to.have.been.calledOnce; - expect(robot.emit).to.have.been.calledWith( - 'slack-attachment', + expect(robot.adapter.send).to.have.been.calledOnceWith( { - attachments: { - color: env.ST2_MATTERMOST_FAIL_COLOR, - fallback: input.message, - mrkdwn_in: ["text", "pretext"], - text: input.message + room: input.channel + }, + { + props: { + attachments: [ + { + color: env.ST2_MATTERMOST_FAIL_COLOR, + fallback: input.message, + mrkdwn_in: ["text", "pretext"], + text: input.message + } + ] }, - room: input.channel, - text: user - } - ); + message: user + }); }); });