diff --git a/lib/discourse_chat/provider/slack/slack_message.rb b/lib/discourse_chat/provider/slack/slack_message.rb index 031c890..e1b4828 100644 --- a/lib/discourse_chat/provider/slack/slack_message.rb +++ b/lib/discourse_chat/provider/slack/slack_message.rb @@ -9,9 +9,10 @@ module DiscourseChat::Provider::SlackProvider def username if user - user['name'] + user["_transcript_username"] elsif @raw.key?("username") - @raw["username"] + # This is for bot messages + @raw["username"].gsub(' ', '_') end end @@ -36,8 +37,13 @@ module DiscourseChat::Provider::SlackProvider text = parts.length > 1 ? parts[1] : parts[0] if parts[0].start_with?('@') - user = @transcript.users.find { |u| u["id"] == parts[0].gsub('@', '') } - next "@#{user['name']}" + user_id = parts[0][1..-1] + if user = @transcript.users[user_id] + user_name = user['_transcript_username'] + else + user_name = user_id + end + next "@#{user_name}" end "[#{text}](#{link})" @@ -91,7 +97,7 @@ module DiscourseChat::Provider::SlackProvider def user return nil unless user_id = @raw["user"] - @transcript.users.find { |u| u["id"] == user_id } + @transcript.users[user_id] end end end diff --git a/lib/discourse_chat/provider/slack/slack_transcript.rb b/lib/discourse_chat/provider/slack/slack_transcript.rb index 6c86749..ed32619 100644 --- a/lib/discourse_chat/provider/slack/slack_transcript.rb +++ b/lib/discourse_chat/provider/slack/slack_transcript.rb @@ -197,7 +197,7 @@ module DiscourseChat::Provider::SlackProvider cursor = nil req = Net::HTTP::Post.new(URI('https://slack.com/api/users.list')) - @users = [] + @users = {} loop do break if cursor == "" req.set_form_data(token: SiteSetting.chat_integration_slack_access_token, limit: 200, cursor: cursor) @@ -206,9 +206,18 @@ module DiscourseChat::Provider::SlackProvider json = JSON.parse(response.body) return false unless json['ok'] cursor = json['response_metadata']['next_cursor'] - @users << json['members'] + json['members'].each do |user| + # Slack uses display_name and falls back to real_name if it is not set + if user['profile']['display_name'].blank? + user['_transcript_username'] = user['profile']['real_name'] + else + user['_transcript_username'] = user['profile']['display_name'] + end + user['_transcript_username'] = user['_transcript_username'].gsub(' ', '_') + @users[user['id']] = user + end end - @users&.flatten! + true end def load_chat_history(count: 500) 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 0461af6..0a4851f 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 @@ -197,7 +197,7 @@ describe 'Slack Command Controller', type: :request do context "with valid slack responses" do before do - 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"}}],"response_metadata":{"next_cursor":""}}') + stub_request(:post, "https://slack.com/api/users.list").to_return(body: '{"ok":true,"members":[{"id":"U5Z773QLS","profile":{"display_name":"david","real_name":"david","icon_24":"https://example.com/avatar"}}],"response_metadata":{"next_cursor":""}}') stub_request(:post, "https://slack.com/api/conversations.history").to_return(body: { ok: true, messages: messages_fixture }.to_json) end diff --git a/spec/lib/discourse_chat/provider/slack/slack_transcript_spec.rb b/spec/lib/discourse_chat/provider/slack/slack_transcript_spec.rb index 67b55b1..83d5107 100644 --- a/spec/lib/discourse_chat/provider/slack/slack_transcript_spec.rb +++ b/spec/lib/discourse_chat/provider/slack/slack_transcript_spec.rb @@ -14,8 +14,8 @@ RSpec.describe DiscourseChat::Provider::SlackProvider::SlackTranscript do }, { "type": "message", - "user": "U5Z773QLS", - "text": "Oooh a new discourse plugin???", + "user": "U5Z773QLZ", + "text": "Oooh a new discourse plugin <@U5Z773QLS> ???", "ts": "1501801643.056375" }, { @@ -62,10 +62,32 @@ RSpec.describe DiscourseChat::Provider::SlackProvider::SlackTranscript do { "type": "message", "user": "U5Z773QLS", - "text": "Let’s try some *bold text*", + "text": "Let’s try some *bold text* <@U5Z773QLZ> <@someotheruser>", "ts": "1501093331.439776" }, + ] + } + let(:users_fixture) { + [ + { + id: "U5Z773QLS", + name: "awesomeguyemail", + profile: { + image_24: "https://example.com/avatar", + display_name: "awesomeguy", + real_name: "actually just a guy" + } + }, + { + id: "U5Z773QLZ", + name: "otherguyemail", + profile: { + image_24: "https://example.com/avatar", + display_name: "", + real_name: "another guy" + } + } ] } @@ -78,7 +100,7 @@ RSpec.describe DiscourseChat::Provider::SlackProvider::SlackTranscript do it 'loads users correctly' do stub_request(:post, "https://slack.com/api/users.list") .with(body: { token: "abcde", "cursor": nil, "limit": "200" }) - .to_return(status: 200, body: { ok: true, members: [{ id: "U5Z773QLS", name: "awesomeguy", profile: { image_24: "https://example.com/avatar" } }], response_metadata: { next_cursor: "" } }.to_json) + .to_return(status: 200, body: { ok: true, members: users_fixture, response_metadata: { next_cursor: "" } }.to_json) expect(transcript.load_user_data).to be_truthy end @@ -101,7 +123,7 @@ RSpec.describe DiscourseChat::Provider::SlackProvider::SlackTranscript do 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" } }], response_metadata: { next_cursor: "" } }.to_json) + .to_return(status: 200, body: { ok: true, members: users_fixture, response_metadata: { next_cursor: "" } }.to_json) transcript.load_user_data end @@ -166,7 +188,7 @@ RSpec.describe DiscourseChat::Provider::SlackProvider::SlackTranscript do end it 'handles bold text' do - expect(transcript.messages.first.text).to eq("Let’s try some **bold text**") + expect(transcript.messages.first.text).to start_with("Let’s try some **bold text** ") end it 'handles links' do @@ -221,8 +243,19 @@ RSpec.describe DiscourseChat::Provider::SlackProvider::SlackTranscript do it 'handles usernames correctly' do expect(transcript.first_message.username).to eq('awesomeguy') # Normal user + expect(transcript.messages[1].username).to eq('Test_Community') # Bot user expect(transcript.messages[2].username).to eq(nil) # Unknown normal user - expect(transcript.messages[1].username).to eq('Test Community') # Bot user + # Normal user, display_name not set (fall back to real_name) + expect(transcript.messages[4].username).to eq('another_guy') + end + + it 'handles user mentions correctly' do + # User with display_name not set, unrecognized user + expect(transcript.first_message.text).to \ + eq('Let’s try some **bold text** @another_guy @someotheruser') + # Normal user + expect(transcript.messages[4].text).to \ + eq('Oooh a new discourse plugin @awesomeguy ???') end it 'handles avatars correctly' do @@ -241,9 +274,9 @@ RSpec.describe DiscourseChat::Provider::SlackProvider::SlackTranscript do [quote] [**View in #general on Slack**](https://slack.com/archives/G1234/p1501093331439776) - ![awesomeguy] **@awesomeguy:** Let’s try some **bold text** + ![awesomeguy] **@awesomeguy:** Let’s try some **bold text** @another_guy @someotheruser - **@Test Community:** + **@Test_Community:** > Discourse can now be integrated with Mattermost! - @david [/quote]