diff --git a/lib/discourse_chat/provider/slack/slack_command_controller.rb b/lib/discourse_chat/provider/slack/slack_command_controller.rb index 8d636db..6c1a011 100644 --- a/lib/discourse_chat/provider/slack/slack_command_controller.rb +++ b/lib/discourse_chat/provider/slack/slack_command_controller.rb @@ -52,9 +52,9 @@ module DiscourseChat::Provider::SlackProvider return { text: I18n.t("chat_integration.provider.slack.transcript.api_required") } end - requested_messages = 10 - + requested_messages = nil first_message_ts = nil + slack_url_regex = /^https:\/\/\S+\.slack\.com\/archives\/\S+\/p([0-9]{16})\/?$/ if tokens.size > 1 && match = slack_url_regex.match(tokens[1]) first_message_ts = match.captures[0].insert(10, '.') @@ -66,12 +66,19 @@ module DiscourseChat::Provider::SlackProvider end end - transcript = SlackTranscript.load_transcript(slack_channel_id: slack_channel_id, - channel_name: channel_name, - requested_messages: requested_messages, - first_message_ts: first_message_ts) + error_message = { text: I18n.t("chat_integration.provider.slack.transcript.error") } - return { text: I18n.t("chat_integration.provider.slack.transcript.error") } unless transcript + return error_message unless transcript = SlackTranscript.new(channel_name: channel_name, channel_id: slack_channel_id) + return error_message unless transcript.load_user_data + return error_message unless transcript.load_chat_history + + if first_message_ts + return error_message unless transcript.set_first_message_by_ts(first_message_ts) + elsif requested_messages + transcript.set_first_message_by_index(-requested_messages) + else + transcript.set_first_message_by_index(-10) unless transcript.guess_first_message + end return transcript.build_slack_ui @@ -92,16 +99,16 @@ module DiscourseChat::Provider::SlackProvider first_message = (action_name == 'first_message') ? changed_val : constant_val last_message = (action_name == 'first_message') ? constant_val : changed_val - transcript = SlackTranscript.load_transcript(slack_channel_id: json[:channel][:id], - channel_name: "##{json[:channel][:name]}", - first_message_ts: first_message, - last_message_ts: last_message) + error_message = { text: I18n.t("chat_integration.provider.slack.transcript.error") } - return { text: I18n.t("chat_integration.provider.slack.transcript.error") } unless transcript + return error_message unless transcript = SlackTranscript.new(channel_name: "##{json[:channel][:name]}", channel_id: json[:channel][:id]) + return error_message unless transcript.load_user_data + return error_message unless transcript.load_chat_history - message = transcript.build_slack_ui + return error_message unless transcript.set_first_message_by_ts(first_message) + return error_message unless transcript.set_last_message_by_ts(last_message) - return message + return transcript.build_slack_ui end def slack_token_valid? diff --git a/lib/discourse_chat/provider/slack/slack_provider.rb b/lib/discourse_chat/provider/slack/slack_provider.rb index 1e1496c..9e08c75 100644 --- a/lib/discourse_chat/provider/slack/slack_provider.rb +++ b/lib/discourse_chat/provider/slack/slack_provider.rb @@ -151,5 +151,5 @@ module DiscourseChat::Provider::SlackProvider end require_relative "slack_message_formatter.rb" -require_relative "slack_transcript_helper.rb" +require_relative "slack_transcript.rb" require_relative "slack_command_controller.rb" diff --git a/lib/discourse_chat/provider/slack/slack_transcript_helper.rb b/lib/discourse_chat/provider/slack/slack_transcript.rb similarity index 61% rename from lib/discourse_chat/provider/slack/slack_transcript_helper.rb rename to lib/discourse_chat/provider/slack/slack_transcript.rb index 77074b8..82025fc 100644 --- a/lib/discourse_chat/provider/slack/slack_transcript_helper.rb +++ b/lib/discourse_chat/provider/slack/slack_transcript.rb @@ -10,6 +10,8 @@ module DiscourseChat::Provider::SlackProvider return user['name'] elsif @raw.key?("username") return @raw["username"] + else + return nil end end @@ -82,39 +84,83 @@ module DiscourseChat::Provider::SlackProvider end class SlackTranscript - attr_reader :users, :channel_id + attr_reader :users, :channel_id, :messages - def initialize(raw_history:, raw_users:, channel_id:, channel_name:, requested_messages: nil, first_message_ts: nil, last_message_ts: nil) + def initialize(channel_name:, channel_id:) + @channel_name = channel_name + @channel_id = channel_id - requested_messages ||= 10 + @first_message_index = 0 + @last_message_index = -1 # We can use negative array indicies to select the last message - fancy! + end - raw_messages = raw_history['messages'].reverse - # Build some message objects - @messages = [] - raw_messages.each_with_index do |message, index| - next unless message["type"] == "message" - this_message = SlackMessage.new(message, self) - @messages << this_message + def set_first_message_by_ts(ts) + message_index = @messages.find_index { |m| m.ts == ts } + @first_message_index = message_index if message_index + end - # Auto set first and last based on requested_messages - @first_message = this_message if index == raw_messages.length - requested_messages - @last_message = this_message if index == raw_messages.length - 1 + def set_last_message_by_ts(ts) + message_index = @messages.find_index { |m| m.ts == ts } + @last_message_index = message_index if message_index + end + + def set_first_message_by_index(val) + @first_message_index = val if @messages[val] + end + + def set_last_message_by_index(val) + @last_message_index = val if @messages[val] + end + + # Apply a heuristic to decide which is the first message in the current conversation + def guess_first_message(skip_messages: 5) # Can skip the last n messages + + possible_first_messages = @messages[0..-skip_messages] + + # Work through the messages in order. If a gap is found, this could be the first message + new_first_message_index = nil + previous_message_ts = @messages[-skip_messages].ts.split('.').first.to_i + possible_first_messages.each_with_index do |message, index| + + # Calculate the time since the last message + this_ts = message.ts.split('.').first.to_i + time_since_previous_message = this_ts - previous_message_ts + + # If greater than 3 minutes, this could be the first message + if time_since_previous_message > 3.minutes + new_first_message_index = index + end + + previous_message_ts = this_ts end - @first_message = @messages.find { |m| m.ts == first_message_ts } || @first_message if first_message_ts - @last_message = @messages.find { |m| m.ts == last_message_ts } || @last_message if last_message_ts + if new_first_message_index + @first_message_index = new_first_message_index + return true + else + return false + end + end - @first_message_index = @messages.index(@first_message) - @last_message_index = @messages.index(@last_message) + def first_message + return @messages[@first_message_index] + end - @users = raw_users['members'] - @channel_id = channel_id - @channel_name = channel_name + def last_message + return @messages[@last_message_index] + end + + # These two methods convert potentially negative array indices into positive ones + def first_message_number + return @first_message_index < 0 ? @messages.length + @first_message_index : @first_message_index + end + def last_message_number + return @last_message_index < 0 ? @messages.length + @last_message_index : @last_message_index end def build_transcript post_content = "[quote]\n" - post_content << "[**#{I18n.t('chat_integration.provider.slack.transcript.view_on_slack', name: @channel_name)}**](#{@first_message.url})\n" + post_content << "[**#{I18n.t('chat_integration.provider.slack.transcript.view_on_slack', name: @channel_name)}**](#{first_message.url})\n" all_avatars = {} @@ -162,41 +208,41 @@ module DiscourseChat::Provider::SlackProvider return { text: "<#{link}|#{I18n.t("chat_integration.provider.slack.transcript.post_to_discourse")}>", attachments: [ { - pretext: I18n.t("chat_integration.provider.slack.transcript.first_message_pretext", n: @messages.length - @first_message_index), - fallback: "#{@first_message.username} - #{@first_message.raw_text}", + pretext: I18n.t("chat_integration.provider.slack.transcript.first_message_pretext", n: @messages.length - first_message_number), + fallback: "#{first_message.username} - #{first_message.raw_text}", color: "#007AB8", - author_name: @first_message.username, - author_icon: @first_message.avatar, - text: @first_message.raw_text, + author_name: first_message.username, + author_icon: first_message.avatar, + text: first_message.raw_text, footer: I18n.t("chat_integration.provider.slack.transcript.posted_in", name: @channel_name), - ts: @first_message.ts, - callback_id: @last_message.ts, + ts: first_message.ts, + callback_id: last_message.ts, actions: [ { name: "first_message", text: I18n.t("chat_integration.provider.slack.transcript.change_first_message"), type: "select", - options: first_message_options = @messages[ [(@first_message_index - 20), 0].max .. @last_message_index] + options: first_message_options = @messages[ [(first_message_number - 20), 0].max .. last_message_number] .map { |m| { text: "#{m.username}: #{m.text}", value: m.ts } } } ], }, { - pretext: I18n.t("chat_integration.provider.slack.transcript.last_message_pretext", n: @messages.length - @last_message_index), - fallback: "#{@last_message.username} - #{@last_message.raw_text}", + pretext: I18n.t("chat_integration.provider.slack.transcript.last_message_pretext", n: @messages.length - last_message_number), + fallback: "#{last_message.username} - #{last_message.raw_text}", color: "#007AB8", - author_name: @last_message.username, - author_icon: @last_message.avatar, - text: @last_message.raw_text, + author_name: last_message.username, + author_icon: last_message.avatar, + text: last_message.raw_text, footer: I18n.t("chat_integration.provider.slack.transcript.posted_in", name: @channel_name), - ts: @last_message.ts, - callback_id: @first_message.ts, + ts: last_message.ts, + callback_id: first_message.ts, actions: [ { name: "last_message", text: I18n.t("chat_integration.provider.slack.transcript.change_last_message"), type: "select", - options: @messages[@first_message_index..(@last_message_index + 20)] + options: @messages[first_message_number..(last_message_number + 20)] .map { |m| { text: "#{m.username}: #{m.text}", value: m.ts } } } ], @@ -206,7 +252,7 @@ module DiscourseChat::Provider::SlackProvider } end - def self.load_user_data + def load_user_data http = Net::HTTP.new("slack.com", 443) http.use_ssl = true @@ -216,10 +262,12 @@ module DiscourseChat::Provider::SlackProvider return false unless response.kind_of? Net::HTTPSuccess json = JSON.parse(response.body) return false unless json['ok'] - return json + + @users = json['members'] + end - def self.load_chat_history(slack_channel_id:, count: 500) + def load_chat_history(count: 500) http = Net::HTTP.new("slack.com", 443) http.use_ssl = true @@ -227,7 +275,7 @@ module DiscourseChat::Provider::SlackProvider data = { token: SiteSetting.chat_integration_slack_access_token, - channel: slack_channel_id, + channel: @channel_id, count: count } @@ -236,21 +284,17 @@ module DiscourseChat::Provider::SlackProvider return false unless response.kind_of? Net::HTTPSuccess json = JSON.parse(response.body) return false unless json['ok'] - return json - end - def self.load_transcript(slack_channel_id:, channel_name:, requested_messages: nil, first_message_ts: nil, last_message_ts: nil) - return false unless raw_users = self.load_user_data + raw_messages = json['messages'].reverse - return false unless raw_history = self.load_chat_history(slack_channel_id: slack_channel_id) + # Build some message objects + @messages = [] + raw_messages.each_with_index do |message, index| + next unless message["type"] == "message" + this_message = SlackMessage.new(message, self) + @messages << this_message + end - self.new(raw_history: raw_history, - raw_users: raw_users, - channel_id: slack_channel_id, - channel_name: channel_name, - requested_messages: requested_messages, - first_message_ts: first_message_ts, - last_message_ts: last_message_ts) end end diff --git a/spec/lib/discourse_chat/provider/slack/slack_command_controller_spec.rb b/spec/lib/discourse_chat/provider/slack/slack_command_controller_spec.rb index f2690bf..a11b502 100644 --- a/spec/lib/discourse_chat/provider/slack/slack_command_controller_spec.rb +++ b/spec/lib/discourse_chat/provider/slack/slack_command_controller_spec.rb @@ -111,23 +111,120 @@ describe 'Slack Command Controller', type: :request do end describe 'post transcript' do + let(:messages_fixture) { + [ + { + "type": "message", + "user": "U6JSSESES", + "text": "Yeah, should make posting slack transcripts much easier", + "ts": "1501801665.062694" + }, + { + "type": "message", + "user": "U5Z773QLS", + "text": "Oooh a new discourse plugin???", + "ts": "1501801643.056375" + }, + { + "type": "message", + "user": "U6E2W7R8C", + "text": "Which one?", + "ts": "1501801634.053761" + }, + { + "type": "message", + "user": "U6JSSESES", + "text": "So, who's interested in the new ?", + "ts": "1501801629.052212" + }, + { + "text": "", + "username": "Test Community", + "bot_id": "B6C6JNUDN", + "attachments": [ + { + "author_name": "@david", + "fallback": "Discourse can now be integrated with Mattermost! - @david", + "text": "Hey , what do you think about this?", + "title": "Discourse can now be integrated with Mattermost! [Announcements] ", + "id": 1, + "title_link": "http://localhost:3000/t/discourse-can-now-be-integrated-with-mattermost/51/4", + "color": "283890", + "mrkdwn_in": [ + "text" + ] + } + ], + "type": "message", + "subtype": "bot_message", + "ts": "1501615820.949638" + }, + { + "type": "message", + "user": "U5Z773QLS", + "text": "Let’s try some *bold text*", + "ts": "1501093331.439776" + }, + + ] + } + before do SiteSetting.chat_integration_slack_access_token = 'abcde' end - it 'generates a transcript properly' do - stub1 = stub_request(:post, "https://slack.com/api/users.list").to_return(body: '{"ok":true,"members":[{"id":"U5Z773QLS","name":"david","profile":{"icon_24":"https://example.com/avatar"}}]}') - stub2 = stub_request(:post, "https://slack.com/api/channels.history").to_return(body: '{"ok":true,"messages":[{"type":"message","user":"U5Z773QLS","text":"And this is a slack message with an attachment: ","attachments":[{"title":"Discourse Meta","title_link":"https:\/\/meta.discourse.org","text":"Discussion about the next-generation open source Discourse forum software","fallback":"Discourse Meta","thumb_url":"https:\/\/discourse-meta.s3-us-west-1.amazonaws.com\/original\/3X\/c\/b\/cb4bec8901221d4a646e45e1fa03db3a65e17f59.png","from_url":"https:\/\/meta.discourse.org","thumb_width":350,"thumb_height":349,"service_icon":"https:\/\/discourse-meta.s3-us-west-1.amazonaws.com\/original\/3X\/c\/b\/cb4bec8901221d4a646e45e1fa03db3a65e17f59.png","service_name":"meta.discourse.org","id":1}],"ts":"1500910064.045243"},{"type":"message","user":"U5Z773QLS","text":"Hello world, this is a slack message","ts":"1500910051.036792"}],"has_more":true}') + context "with valid slack responses" do + before do + stub1 = stub_request(:post, "https://slack.com/api/users.list").to_return(body: '{"ok":true,"members":[{"id":"U5Z773QLS","name":"david","profile":{"icon_24":"https://example.com/avatar"}}]}') + stub2 = stub_request(:post, "https://slack.com/api/channels.history").to_return(body: { ok: true, messages: messages_fixture }.to_json) + end - post "/chat-integration/slack/command.json", - text: "post 2", - channel_name: 'general', - channel_id: 'C6029G78F', - token: token + it 'generates the transcript UI properly' do + post "/chat-integration/slack/command.json", + text: "post", + channel_name: 'general', + channel_id: 'C6029G78F', + token: token - json = JSON.parse(response.body) + json = JSON.parse(response.body) + expect(json["attachments"].length).to eq(2) + end - # expect(json["text"]).to include(I18n.t("chat_integration.provider.slack.post_to_discourse")) + it 'can select by url' do + post "/chat-integration/slack/command.json", + text: "post https://sometestslack.slack.com/archives/C6029G78F/p1501801629052212", + channel_name: 'general', + channel_id: 'C6029G78F', + token: token + + json = JSON.parse(response.body) + expect(json["attachments"].length).to eq(2) + expect(json["attachments"][0]["ts"]).to eq("1501801629.052212") + end + + it 'can select by count' do + post "/chat-integration/slack/command.json", + text: "post 4", + channel_name: 'general', + channel_id: 'C6029G78F', + token: token + + json = JSON.parse(response.body) + expect(json["attachments"].length).to eq(2) + expect(json["attachments"][0]["ts"]).to eq("1501801629.052212") + end + + it 'can auto select' do + post "/chat-integration/slack/command.json", + text: "post", + channel_name: 'general', + channel_id: 'C6029G78F', + token: token + + json = JSON.parse(response.body) + expect(json["attachments"].length).to eq(2) + expect(json["attachments"][0]["ts"]).to eq("1501615820.949638") + end end it 'deals with failed API calls correctly' do diff --git a/spec/lib/discourse_chat/provider/slack/slack_transcript_spec.rb b/spec/lib/discourse_chat/provider/slack/slack_transcript_spec.rb new file mode 100644 index 0000000..3492210 --- /dev/null +++ b/spec/lib/discourse_chat/provider/slack/slack_transcript_spec.rb @@ -0,0 +1,237 @@ +require 'rails_helper' + +RSpec.describe DiscourseChat::Provider::SlackProvider::SlackTranscript do + + let(:messages_fixture) { + [ + { + "type": "message", + "user": "U6JSSESES", + "text": "Yeah, should make posting slack transcripts much easier", + "ts": "1501801665.062694" + }, + { + "type": "message", + "user": "U5Z773QLS", + "text": "Oooh a new discourse plugin???", + "ts": "1501801643.056375" + }, + { + "type": "message", + "user": "U6E2W7R8C", + "text": "Which one?", + "ts": "1501801634.053761" + }, + { + "type": "message", + "user": "U6JSSESES", + "text": "So, who's interested in the new ?", + "ts": "1501801629.052212" + }, + { + "text": "", + "username": "Test Community", + "bot_id": "B6C6JNUDN", + "attachments": [ + { + "author_name": "@david", + "fallback": "Discourse can now be integrated with Mattermost! - @david", + "text": "Hey , what do you think about this?", + "title": "Discourse can now be integrated with Mattermost! [Announcements] ", + "id": 1, + "title_link": "http://localhost:3000/t/discourse-can-now-be-integrated-with-mattermost/51/4", + "color": "283890", + "mrkdwn_in": [ + "text" + ] + } + ], + "type": "message", + "subtype": "bot_message", + "ts": "1501615820.949638" + }, + { + "type": "message", + "user": "U5Z773QLS", + "text": "Let’s try some *bold text*", + "ts": "1501093331.439776" + }, + + ] + } + + let(:transcript) { described_class.new(channel_name: "#general", channel_id: "G1234") } + before do + SiteSetting.chat_integration_slack_access_token = "abcde" + end + + describe 'loading users' do + it 'loads users correctly' do + stub_request(:post, "https://slack.com/api/users.list") + .with(body: { token: "abcde" }) + .to_return(status: 200, body: { ok: true, members: [{ id: "U5Z773QLS", name: "awesomeguy", profile: { image_24: "https://example.com/avatar" } }] }.to_json) + + expect(transcript.load_user_data).to be_truthy + end + + it 'handles failed connection' do + stub_request(:post, "https://slack.com/api/users.list") + .to_return(status: 500, body: '') + + expect(transcript.load_user_data).to be_falsey + end + + it 'handles slack failure' do + stub_request(:post, "https://slack.com/api/users.list") + .to_return(status: 200, body: { ok: false }.to_json) + + expect(transcript.load_user_data).to be_falsey + end + end + + context 'with loaded users' do + before do + stub_request(:post, "https://slack.com/api/users.list") + .to_return(status: 200, body: { ok: true, members: [{ id: "U5Z773QLS", name: "awesomeguy", profile: { image_24: "https://example.com/avatar" } }] }.to_json) + transcript.load_user_data + end + + describe 'loading history' do + it 'loads messages correctly' do + stub_request(:post, "https://slack.com/api/channels.history") + .with(body: hash_including(token: "abcde", channel: 'G1234')) + .to_return(status: 200, body: { ok: true, messages: messages_fixture }.to_json) + + expect(transcript.load_chat_history).to be_truthy + end + + it 'handles failed connection' do + stub_request(:post, "https://slack.com/api/channels.history") + .to_return(status: 500, body: {}.to_json) + + expect(transcript.load_chat_history).to be_falsey + end + + it 'handles slack failure' do + stub_request(:post, "https://slack.com/api/channels.history") + .to_return(status: 200, body: { ok: false }.to_json) + + expect(transcript.load_chat_history).to be_falsey + end + end + + context 'with loaded messages' do + before do + stub_request(:post, "https://slack.com/api/channels.history") + .with(body: hash_including(token: "abcde", channel: 'G1234')) + .to_return(status: 200, body: { ok: true, messages: messages_fixture }.to_json) + transcript.load_chat_history + end + + it 'loads in chronological order' do # API presents in reverse chronological + expect(transcript.messages.first.ts).to eq('1501093331.439776') + end + + it 'handles bold text' do + expect(transcript.messages.first.text).to eq("Let’s try some **bold text**") + end + + it 'handles links' do + expect(transcript.messages[2].text).to eq("So, who's interested in the new [discourse plugin](https://meta.discourse.org)?") + end + + it 'includes attachments' do + expect(transcript.messages[1].attachments.first).to eq("Discourse can now be integrated with Mattermost! - @david") + end + + it 'can generate URL' do + expect(transcript.messages.first.url).to eq("https://slack.com/archives/G1234/p1501093331439776") + end + + it 'gives correct first and last messages' do + expect(transcript.first_message_number).to eq(0) + expect(transcript.last_message_number).to eq(transcript.messages.length - 1) + + expect(transcript.first_message.ts).to eq('1501093331.439776') + expect(transcript.last_message.ts).to eq('1501801665.062694') + end + + it 'can change first and last messages by index' do + expect(transcript.set_first_message_by_index(999)).to be_falsey + expect(transcript.set_first_message_by_index(1)).to be_truthy + + expect(transcript.set_last_message_by_index(-2)).to be_truthy + + expect(transcript.first_message.ts).to eq('1501615820.949638') + expect(transcript.last_message.ts).to eq('1501801643.056375') + end + + it 'can change first and last messages by ts' do + expect(transcript.set_first_message_by_ts('blah')).to be_falsey + expect(transcript.set_first_message_by_ts('1501615820.949638')).to be_truthy + + expect(transcript.set_last_message_by_ts('1501801629.052212')).to be_truthy + + expect(transcript.first_message_number).to eq(1) + expect(transcript.last_message_number).to eq(2) + end + + it 'can guess the first message' do + expect(transcript.guess_first_message(skip_messages: 1)).to eq(true) + expect(transcript.first_message.ts).to eq('1501801629.052212') + end + + it 'handles usernames correctly' do + expect(transcript.first_message.username).to eq('awesomeguy') # Normal user + expect(transcript.messages[2].username).to eq(nil) # Unknown normal user + expect(transcript.messages[1].username).to eq('Test Community') # Bot user + end + + it 'handles avatars correctly' do + expect(transcript.first_message.avatar).to eq("https://example.com/avatar") # Normal user + expect(transcript.messages[1].avatar).to eq(nil) # Bot user + end + + it 'creates a transcript correctly' do + transcript.set_last_message_by_index(1) + + text = transcript.build_transcript + + # Rubocop doesn't like this, but we really do need trailing whitespace in the string + # rubocop:disable TrailingWhitespace + expected = <<~END + [quote] + [**View in #general on Slack**](https://slack.com/archives/G1234/p1501093331439776) + + ![awesomeguy] **@awesomeguy:** Let’s try some **bold text** + + **@Test Community:** + > Discourse can now be integrated with Mattermost! - @david + + [/quote] + + [awesomeguy]: https://example.com/avatar + END + # rubocop:enable TrailingWhitespace + + expect(text).to eq(expected) + end + + it 'creates the slack UI correctly' do + transcript.set_last_message_by_index(1) + ui = transcript.build_slack_ui + + first_ui = ui[:attachments][0] + last_ui = ui[:attachments][1] + + # The callback IDs are used to keep track of what the other option is + expect(first_ui[:callback_id]).to eq(transcript.last_message.ts) + expect(last_ui[:callback_id]).to eq(transcript.first_message.ts) + + # The timestamps should match up to the actual messages + expect(first_ui[:ts]).to eq(transcript.first_message.ts) + expect(last_ui[:ts]).to eq(transcript.last_message.ts) + end + end + end +end