Merge branch '25373-jira-links' into 'master'

Don’t create comment on JIRA if link already exists

Closes #25373

See merge request !11485

app/models/project_services/jira_service.rb

@@ -239,17 +239,26 @@ class JiraService < IssueTrackerService
     return unless client_url.present?
 
     jira_request do
-      if issue.comments.build.save!(body: message)
-        remote_link = issue.remotelink.build
+      remote_link = find_remote_link(issue, remote_link_props[:object][:url])
+      if remote_link
         remote_link.save!(remote_link_props)
-        result_message = "#{self.class.name} SUCCESS: Successfully posted to #{client_url}."
+      elsif issue.comments.build.save!(body: message)
+        new_remote_link = issue.remotelink.build
+        new_remote_link.save!(remote_link_props)
       end
 
+      result_message = "#{self.class.name} SUCCESS: Successfully posted to #{client_url}."
       Rails.logger.info(result_message)
       result_message
     end
   end
 
+  def find_remote_link(issue, url)
+    links = jira_request { issue.remotelink.all }
+
+    links.find { |link| link.object["url"] == url }
+  end
+
   def build_remote_link_props(url:, title:, resolved: false)
     status = {
       resolved: resolved

changelogs/unreleased/25373-jira-links.yml

@@ -0,0 +1,4 @@
+---
+title: Don’t create comment on JIRA if it already exists for the entity
+merge_request:
+author:

spec/models/project_services/jira_service_spec.rb

@@ -133,6 +133,7 @@ describe JiraService, models: true do
       allow(JIRA::Resource::Issue).to receive(:find).and_return(open_issue, closed_issue)
 
       allow_any_instance_of(JIRA::Resource::Issue).to receive(:key).and_return("JIRA-123")
+      allow(JIRA::Resource::Remotelink).to receive(:all).and_return([])
 
       @jira_service.save
 

spec/services/git_push_service_spec.rb

@@ -436,6 +436,7 @@ describe GitPushService, services: true do
                                                     author_name: commit_author.name,
                                                     author_email: commit_author.email
                                                   })
+        allow(JIRA::Resource::Remotelink).to receive(:all).and_return([])
 
         allow(project.repository).to receive_messages(commits_between: [closing_commit])
       end

spec/services/system_note_service_spec.rb

@@ -733,6 +733,26 @@ describe SystemNoteService, services: true do
       jira_service_settings
     end
 
+    def cross_reference(type, link_exists = false)
+      noteable = type == 'commit' ? commit : merge_request
+
+      links = []
+      if link_exists
+        url = if type == 'commit'
+                "#{Settings.gitlab.base_url}/#{project.namespace.path}/#{project.path}/commit/#{commit.id}"
+              else
+                "#{Settings.gitlab.base_url}/#{project.namespace.path}/#{project.path}/merge_requests/#{merge_request.iid}"
+              end
+        link = double(object: { 'url' => url })
+        links << link
+        expect(link).to receive(:save!)
+      end
+
+      allow(JIRA::Resource::Remotelink).to receive(:all).and_return(links)
+
+      described_class.cross_reference(jira_issue, noteable, author)
+    end
+
     noteable_types = %w(merge_requests commit)
 
     noteable_types.each do |type|
@@ -740,24 +760,39 @@ describe SystemNoteService, services: true do
         it "blocks cross reference when #{type.underscore}_events is false" do
           jira_tracker.update("#{type}_events" => false)
 
-          noteable = type == "commit" ? commit : merge_request
-          result = described_class.cross_reference(jira_issue, noteable, author)
-
-          expect(result).to eq("Events for #{noteable.class.to_s.underscore.humanize.pluralize.downcase} are disabled.")
+          expect(cross_reference(type)).to eq("Events for #{type.pluralize.humanize.downcase} are disabled.")
         end
 
         it "blocks cross reference when #{type.underscore}_events is true" do
           jira_tracker.update("#{type}_events" => true)
 
-          noteable = type == "commit" ? commit : merge_request
-          result = described_class.cross_reference(jira_issue, noteable, author)
+          expect(cross_reference(type)).to eq(success_message)
+        end
+      end
 
-          expect(result).to eq(success_message)
+      context 'when a new cross reference is created' do
+        it 'creates a new comment and remote link' do
+          cross_reference(type)
+
+          expect(WebMock).to have_requested(:post, jira_api_comment_url(jira_issue))
+          expect(WebMock).to have_requested(:post, jira_api_remote_link_url(jira_issue))
+        end
+      end
+
+      context 'when a link exists' do
+        it 'updates a link but does not create a new comment' do
+          expect(WebMock).not_to have_requested(:post, jira_api_comment_url(jira_issue))
+
+          cross_reference(type, true)
         end
       end
     end
 
     describe "new reference" do
+      before do
+        allow(JIRA::Resource::Remotelink).to receive(:all).and_return([])
+      end
+
       context 'for commits' do
         it "creates comment" do
           result = described_class.cross_reference(jira_issue, commit, author)
@@ -837,6 +872,7 @@ describe SystemNoteService, services: true do
 
     describe "existing reference" do
       before do
+        allow(JIRA::Resource::Remotelink).to receive(:all).and_return([])
         message = "[#{author.name}|http://localhost/#{author.username}] mentioned this issue in [a commit of #{project.path_with_namespace}|http://localhost/#{project.path_with_namespace}/commit/#{commit.id}]:\n'#{commit.title.chomp}'"
         allow_any_instance_of(JIRA::Resource::Issue).to receive(:comments).and_return([OpenStruct.new(body: message)])
       end