Merge branch 'ee-44596-double-title-merge-request-message' into 'master'

EE port: Fix double title in merge request chat messages

See merge request gitlab-org/gitlab-ee!7370

app/models/project_services/chat_message/merge_message.rb

@@ -2,6 +2,8 @@
 
 module ChatMessage
   class MergeMessage < BaseMessage
+    prepend ::EE::ChatMessage::MergeMessage
+
     attr_reader :merge_request_iid
     attr_reader :source_branch
     attr_reader :target_branch
@@ -11,8 +13,6 @@ module ChatMessage
     def initialize(params)
       super
 
-      @action = params[:object_attributes][:action]
-
       obj_attr = params[:object_attributes]
       obj_attr = HashWithIndifferentAccess.new(obj_attr)
       @merge_request_iid = obj_attr[:iid]
@@ -50,7 +50,7 @@ module ChatMessage
     end
 
     def merge_request_message
-      "#{user_combined_name} #{state_or_action_text} #{merge_request_link} in #{project_link}: #{title}"
+      "#{user_combined_name} #{state_or_action_text} #{merge_request_link} in #{project_link}"
     end
 
     def merge_request_link
@@ -65,8 +65,9 @@ module ChatMessage
       "#{project_url}/merge_requests/#{merge_request_iid}"
     end
 
+    # overridden in EE
     def state_or_action_text
-      @action == 'approved' ? @action : state
+      state
     end
   end
 end

changelogs/unreleased/44596-double-title-merge-request-message.yml

+---
+title: Fix double title in merge request chat messages.
+merge_request:
+author: Kukovskii Vladimir
+type: fixed

ee/app/models/project_services/ee/chat_message/merge_message.rb

+# frozen_string_literal: true
+
+module EE
+  module ChatMessage
+    module MergeMessage
+      extend ActiveSupport::Concern
+      extend ::Gitlab::Utils::Override
+
+      prepended do
+        attr_reader :action
+      end
+
+      def initialize(params)
+        super
+
+        @action = params[:object_attributes][:action]
+      end
+
+      override :state_or_action_text
+      def state_or_action_text
+        action == 'approved' ? action : super
+      end
+    end
+  end
+end

ee/spec/models/project_services/chat_message/merge_message_spec.rb

+require 'spec_helper'
+
+RSpec.describe ChatMessage::MergeMessage do
+  subject { described_class.new(args) }
+
+  let(:args) do
+    {
+      user: {
+          name: 'Test User',
+          username: 'test.user',
+          avatar_url: 'http://someavatar.com'
+      },
+      project_name: 'project_name',
+      project_url: 'http://somewhere.com',
+
+      object_attributes: {
+        title: "Merge Request title\nSecond line",
+        id: 10,
+        iid: 100,
+        assignee_id: 1,
+        url: 'http://url.com',
+        state: 'opened',
+        description: 'merge request description',
+        source_branch: 'source_branch',
+        target_branch: 'target_branch'
+      }
+    }
+  end
+
+  context 'approval' do
+    before do
+      args[:object_attributes][:action] = 'approved'
+    end
+
+    it 'returns a message regarding approval of merge requests' do
+      expect(subject.pretext).to eq(
+        'Test User (test.user) approved <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> '\
+        'in <http://somewhere.com|project_name>')
+      expect(subject.attachments).to be_empty
+    end
+  end
+end

spec/models/project_services/chat_message/merge_message_spec.rb

@@ -33,7 +33,7 @@ describe ChatMessage::MergeMessage do
     context 'open' do
       it 'returns a message regarding opening of merge requests' do
         expect(subject.pretext).to eq(
-          'Test User (test.user) opened <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>: *Merge Request title*')
+          'Test User (test.user) opened <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>')
         expect(subject.attachments).to be_empty
       end
     end
@@ -44,25 +44,12 @@ describe ChatMessage::MergeMessage do
       end
       it 'returns a message regarding closing of merge requests' do
         expect(subject.pretext).to eq(
-          'Test User (test.user) closed <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>: *Merge Request title*')
+          'Test User (test.user) closed <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> in <http://somewhere.com|project_name>')
         expect(subject.attachments).to be_empty
       end
     end
   end
 
-  context 'approval' do
-    before do
-      args[:object_attributes][:action] = 'approved'
-    end
-
-    it 'returns a message regarding approval of merge requests' do
-      expect(subject.pretext).to eq(
-        'Test User (test.user) approved <http://somewhere.com/merge_requests/100|!100 *Merge Request title*> '\
-        'in <http://somewhere.com|project_name>: *Merge Request title*')
-      expect(subject.attachments).to be_empty
-    end
-  end
-
   context 'with markdown' do
     before do
       args[:markdown] = true
@@ -71,7 +58,7 @@ describe ChatMessage::MergeMessage do
     context 'open' do
       it 'returns a message regarding opening of merge requests' do
         expect(subject.pretext).to eq(
-          'Test User (test.user) opened [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com): *Merge Request title*')
+          'Test User (test.user) opened [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com)')
         expect(subject.attachments).to be_empty
         expect(subject.activity).to eq({
           title: 'Merge Request opened by Test User (test.user)',
@@ -89,7 +76,7 @@ describe ChatMessage::MergeMessage do
 
       it 'returns a message regarding closing of merge requests' do
         expect(subject.pretext).to eq(
-          'Test User (test.user) closed [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com): *Merge Request title*')
+          'Test User (test.user) closed [!100 *Merge Request title*](http://somewhere.com/merge_requests/100) in [project_name](http://somewhere.com)')
         expect(subject.attachments).to be_empty
         expect(subject.activity).to eq({
           title: 'Merge Request closed by Test User (test.user)',