1 files changed, 208 insertions, 42 deletions

diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 5bb107fdd85..0e8adb68721 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe SystemNoteService, services: true do
+  include Gitlab::Routing.url_helpers
+
   let(:project)  { create(:project) }
   let(:author)   { create(:user) }
   let(:noteable) { create(:issue, project: project) }
@@ -48,7 +50,7 @@ describe SystemNoteService, services: true do
 
       context 'without existing commits' do
         it 'adds a message header' do
-          expect(note_lines[0]).to eq "Added #{new_commits.size} commits:"
+          expect(note_lines[0]).to eq "added #{new_commits.size} commits"
         end
 
         it 'adds a message line for each commit' do
@@ -118,7 +120,7 @@ describe SystemNoteService, services: true do
 
     context 'when assignee added' do
       it 'sets the note text' do
-        expect(subject.note).to eq "Reassigned to @#{assignee.username}"
+        expect(subject.note).to eq "assigned to @#{assignee.username}"
       end
     end
 
@@ -126,7 +128,7 @@ describe SystemNoteService, services: true do
       let(:assignee) { nil }
 
       it 'sets the note text' do
-        expect(subject.note).to eq 'Assignee removed'
+        expect(subject.note).to eq 'removed assignee'
       end
     end
   end
@@ -145,7 +147,7 @@ describe SystemNoteService, services: true do
       let(:removed) { [] }
 
       it 'sets the note text' do
-        expect(subject.note).to eq "Added ~#{labels[0].id} ~#{labels[1].id} labels"
+        expect(subject.note).to eq "added ~#{labels[0].id} ~#{labels[1].id} labels"
       end
     end
 
@@ -154,7 +156,7 @@ describe SystemNoteService, services: true do
       let(:removed) { labels }
 
       it 'sets the note text' do
-        expect(subject.note).to eq "Removed ~#{labels[0].id} ~#{labels[1].id} labels"
+        expect(subject.note).to eq "removed ~#{labels[0].id} ~#{labels[1].id} labels"
       end
     end
 
@@ -163,7 +165,7 @@ describe SystemNoteService, services: true do
       let(:removed) { [labels[1]] }
 
       it 'sets the note text' do
-        expect(subject.note).to eq "Added ~#{labels[0].id} and removed ~#{labels[1].id} labels"
+        expect(subject.note).to eq "added ~#{labels[0].id} and removed ~#{labels[1].id} labels"
       end
     end
   end
@@ -177,7 +179,7 @@ describe SystemNoteService, services: true do
 
     context 'when milestone added' do
       it 'sets the note text' do
-        expect(subject.note).to eq "Milestone changed to #{milestone.to_reference}"
+        expect(subject.note).to eq "changed milestone to #{milestone.to_reference}"
       end
     end
 
@@ -185,7 +187,7 @@ describe SystemNoteService, services: true do
       let(:milestone) { nil }
 
       it 'sets the note text' do
-        expect(subject.note).to eq 'Milestone removed'
+        expect(subject.note).to eq 'removed milestone'
       end
     end
   end
@@ -202,13 +204,13 @@ describe SystemNoteService, services: true do
       let(:source) { double('commit', gfm_reference: 'commit 123456') }
 
       it 'sets the note text' do
-        expect(subject.note).to eq "Status changed to #{status} by commit 123456"
+        expect(subject.note).to eq "#{status} via commit 123456"
       end
     end
 
     context 'without a source' do
       it 'sets the note text' do
-        expect(subject.note).to eq "Status changed to #{status}"
+        expect(subject.note).to eq status
       end
     end
   end
@@ -223,8 +225,8 @@ describe SystemNoteService, services: true do
 
     it_behaves_like 'a system note'
 
-    it "posts the Merge When Build Succeeds system note" do
-      expect(subject.note).to match  /Enabled an automatic merge when the build for (\w+\/\w+@)?[0-9a-f]{40} succeeds/
+    it "posts the 'merge when pipeline succeeds' system note" do
+      expect(subject.note).to match  /enabled an automatic merge when the pipeline for (\w+\/\w+@)?\h{40} succeeds/
     end
   end
 
@@ -237,8 +239,8 @@ describe SystemNoteService, services: true do
 
     it_behaves_like 'a system note'
 
-    it "posts the Merge When Build Succeeds system note" do
-      expect(subject.note).to eq  "Canceled the automatic merge"
+    it "posts the 'merge when pipeline succeeds' system note" do
+      expect(subject.note).to eq  "canceled the automatic merge"
     end
   end
 
@@ -250,7 +252,7 @@ describe SystemNoteService, services: true do
 
       it 'sets the note text' do
         expect(subject.note).
-          to eq "Changed title: **{-Old title-}** → **{+#{noteable.title}+}**"
+          to eq "changed title from **{-Old title-}** to **{+#{noteable.title}+}**"
       end
     end
   end
@@ -262,7 +264,7 @@ describe SystemNoteService, services: true do
       it_behaves_like 'a system note'
 
       it 'sets the note text' do
-        expect(subject.note).to eq 'Made the issue visible'
+        expect(subject.note).to eq 'made the issue visible to everyone'
       end
     end
   end
@@ -276,7 +278,7 @@ describe SystemNoteService, services: true do
 
     context 'when target branch name changed' do
       it 'sets the note text' do
-        expect(subject.note).to eq "Target branch changed from `#{old_branch}` to `#{new_branch}`"
+        expect(subject.note).to eq "changed target branch from `#{old_branch}` to `#{new_branch}`"
       end
     end
   end
@@ -288,7 +290,7 @@ describe SystemNoteService, services: true do
 
     context 'when source branch deleted' do
       it 'sets the note text' do
-        expect(subject.note).to eq "Deleted source branch `feature`"
+        expect(subject.note).to eq "deleted source branch `feature`"
       end
     end
   end
@@ -300,7 +302,7 @@ describe SystemNoteService, services: true do
 
     context 'when a branch is created from the new branch button' do
       it 'sets the note text' do
-        expect(subject.note).to match /\AStarted branch [`1-mepmep`]/
+        expect(subject.note).to match /\Acreated branch [`1-mepmep`]/
       end
     end
   end
@@ -336,13 +338,13 @@ describe SystemNoteService, services: true do
             let(:mentioner) { project2.repository.commit }
 
             it 'references the mentioning commit' do
-              expect(subject.note).to eq "Mentioned in commit #{mentioner.to_reference(project)}"
+              expect(subject.note).to eq "mentioned in commit #{mentioner.to_reference(project)}"
             end
           end
 
           context 'from non-Commit' do
             it 'references the mentioning object' do
-              expect(subject.note).to eq "Mentioned in issue #{mentioner.to_reference(project)}"
+              expect(subject.note).to eq "mentioned in issue #{mentioner.to_reference(project)}"
             end
           end
         end
@@ -352,13 +354,13 @@ describe SystemNoteService, services: true do
             let(:mentioner) { project.repository.commit }
 
             it 'references the mentioning commit' do
-              expect(subject.note).to eq "Mentioned in commit #{mentioner.to_reference}"
+              expect(subject.note).to eq "mentioned in commit #{mentioner.to_reference}"
             end
           end
 
           context 'from non-Commit' do
             it 'references the mentioning object' do
-              expect(subject.note).to eq "Mentioned in issue #{mentioner.to_reference}"
+              expect(subject.note).to eq "mentioned in issue #{mentioner.to_reference}"
             end
           end
         end
@@ -368,7 +370,11 @@ describe SystemNoteService, services: true do
 
   describe '.cross_reference?' do
     it 'is truthy when text begins with expected text' do
-      expect(described_class.cross_reference?('Mentioned in something')).to be_truthy
+      expect(described_class.cross_reference?('mentioned in something')).to be_truthy
+    end
+
+    it 'is truthy when text begins with legacy capitalized expected text' do
+      expect(described_class.cross_reference?('mentioned in something')).to be_truthy
     end
 
     it 'is falsey when text does not begin with expected text' do
@@ -431,6 +437,19 @@ describe SystemNoteService, services: true do
         expect(described_class.cross_reference_exists?(noteable, commit1)).
           to be_falsey
       end
+
+      context 'legacy capitalized cross reference' do
+        before do
+          # Mention issue (noteable) from commit0
+          system_note = described_class.cross_reference(noteable, commit0, author)
+          system_note.update(note: system_note.note.capitalize)
+        end
+
+        it 'is truthy when already mentioned' do
+          expect(described_class.cross_reference_exists?(noteable, commit0)).
+            to be_truthy
+        end
+      end
     end
 
     context 'commit from commit' do
@@ -448,6 +467,19 @@ describe SystemNoteService, services: true do
         expect(described_class.cross_reference_exists?(commit1, commit0)).
           to be_falsey
       end
+
+      context 'legacy capitalized cross reference' do
+        before do
+          # Mention commit1 from commit0
+          system_note = described_class.cross_reference(commit0, commit1, author)
+          system_note.update(note: system_note.note.capitalize)
+        end
+
+        it 'is truthy when already mentioned' do
+          expect(described_class.cross_reference_exists?(commit0, commit1)).
+            to be_truthy
+        end
+      end
     end
 
     context 'commit with cross-reference from fork' do
@@ -463,6 +495,18 @@ describe SystemNoteService, services: true do
         expect(described_class.cross_reference_exists?(noteable, commit2)).
             to be true
       end
+
+      context 'legacy capitalized cross reference' do
+        before do
+          system_note = described_class.cross_reference(noteable, commit0, author2)
+          system_note.update(note: system_note.note.capitalize)
+        end
+
+        it 'is true when a fork mentions an external issue' do
+          expect(described_class.cross_reference_exists?(noteable, commit2)).
+              to be true
+        end
+      end
     end
   end
 
@@ -486,7 +530,7 @@ describe SystemNoteService, services: true do
       end
 
       it 'mentions referenced project' do
-        expect(subject.note).to include new_project.to_reference
+        expect(subject.note).to include new_project.path_with_namespace
       end
     end
 
@@ -496,7 +540,7 @@ describe SystemNoteService, services: true do
       it_behaves_like 'cross project mentionable'
 
       it 'notifies about noteable being moved to' do
-        expect(subject.note).to match /Moved to/
+        expect(subject.note).to match /moved to/
       end
     end
 
@@ -506,7 +550,7 @@ describe SystemNoteService, services: true do
       it_behaves_like 'cross project mentionable'
 
       it 'notifies about noteable being moved from' do
-        expect(subject.note).to match /Moved from/
+        expect(subject.note).to match /moved from/
       end
     end
 
@@ -534,44 +578,166 @@ describe SystemNoteService, services: true do
     let(:project)         { create(:jira_project) }
     let(:author)          { create(:user) }
     let(:issue)           { create(:issue, project: project) }
-    let(:mergereq)        { create(:merge_request, :simple, target_project: project, source_project: project) }
+    let(:merge_request)        { create(:merge_request, :simple, target_project: project, source_project: project) }
     let(:jira_issue)      { ExternalIssue.new("JIRA-1", project)}
     let(:jira_tracker)    { project.jira_service }
     let(:commit)          { project.commit }
     let(:comment_url)     { jira_api_comment_url(jira_issue.id) }
     let(:success_message) { "JiraService SUCCESS: Successfully posted to http://jira.example.net." }
 
-    before { stub_jira_urls(jira_issue.id) }
+    before do
+      stub_jira_urls(jira_issue.id)
+      jira_service_settings
+    end
+
+    noteable_types = ["merge_requests", "commit"]
+
+    noteable_types.each do |type|
+      context "when noteable is a #{type}" do
+        it "blocks cross reference when #{type.underscore}_events is false" do
+          jira_tracker.update("#{type}_events" => false)
 
-    context 'in JIRA issue tracker' do
-      before { jira_service_settings }
+          noteable = type == "commit" ? commit : merge_request
+          result = described_class.cross_reference(jira_issue, noteable, author)
 
-      describe "new reference" do
-        subject { described_class.cross_reference(jira_issue, commit, author) }
+          expect(result).to eq("Events for #{noteable.class.to_s.underscore.humanize.pluralize.downcase} are disabled.")
+        end
+
+        it "blocks cross reference when #{type.underscore}_events is true" do
+          jira_tracker.update("#{type}_events" => true)
 
-        it { is_expected.to eq(success_message) }
+          noteable = type == "commit" ? commit : merge_request
+          result = described_class.cross_reference(jira_issue, noteable, author)
+
+          expect(result).to eq(success_message)
+        end
       end
     end
 
-    context 'issue from an issue' do
-      context 'in JIRA issue tracker' do
-        before { jira_service_settings }
+    describe "new reference" do
+      context 'for commits' do
+        it "creates comment" do
+          result = described_class.cross_reference(jira_issue, commit, author)
+
+          expect(result).to eq(success_message)
+        end
+
+        it "creates remote link" do
+          described_class.cross_reference(jira_issue, commit, author)
+
+          expect(WebMock).to have_requested(:post, jira_api_remote_link_url(jira_issue)).with(
+            body: hash_including(
+              GlobalID: "GitLab",
+              object: {
+                url: namespace_project_commit_url(project.namespace, project, commit),
+                title: "GitLab: Mentioned on commit - #{commit.title}",
+                icon: { title: "GitLab", url16x16: "https://gitlab.com/favicon.ico" },
+                status: { resolved: false }
+              }
+            )
+          ).once
+        end
+      end
+
+      context 'for issues' do
+        let(:issue)           { create(:issue, project: project) }
+
+        it "creates comment" do
+          result = described_class.cross_reference(jira_issue, issue, author)
+
+          expect(result).to eq(success_message)
+        end
+
+        it "creates remote link" do
+          described_class.cross_reference(jira_issue, issue, author)
+
+          expect(WebMock).to have_requested(:post, jira_api_remote_link_url(jira_issue)).with(
+            body: hash_including(
+              GlobalID: "GitLab",
+              object: {
+                url: namespace_project_issue_url(project.namespace, project, issue),
+                title: "GitLab: Mentioned on issue - #{issue.title}",
+                icon: { title: "GitLab", url16x16: "https://gitlab.com/favicon.ico" },
+                status: { resolved: false }
+              }
+            )
+          ).once
+        end
+      end
 
-        subject { described_class.cross_reference(jira_issue, issue, author) }
+      context 'for snippets' do
+        let(:snippet) { create(:snippet, project: project) }
 
-        it { is_expected.to eq(success_message) }
+        it "creates comment" do
+          result = described_class.cross_reference(jira_issue, snippet, author)
+
+          expect(result).to eq(success_message)
+        end
+
+        it "creates remote link" do
+          described_class.cross_reference(jira_issue, snippet, author)
+
+          expect(WebMock).to have_requested(:post, jira_api_remote_link_url(jira_issue)).with(
+            body: hash_including(
+              GlobalID: "GitLab",
+              object: {
+                url: namespace_project_snippet_url(project.namespace, project, snippet),
+                title: "GitLab: Mentioned on snippet - #{snippet.title}",
+                icon: { title: "GitLab", url16x16: "https://gitlab.com/favicon.ico" },
+                status: { resolved: false }
+              }
+            )
+          ).once
+        end
       end
     end
 
     describe "existing reference" do
       before do
-        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}'"
+        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
 
-      subject { described_class.cross_reference(jira_issue, commit, author) }
+      it "does not return success message" do
+        result = described_class.cross_reference(jira_issue, commit, author)
+
+        expect(result).not_to eq(success_message)
+      end
+
+      it 'does not try to create comment and remote link' do
+        subject
+
+        expect(WebMock).not_to have_requested(:post, jira_api_comment_url(jira_issue))
+        expect(WebMock).not_to have_requested(:post, jira_api_remote_link_url(jira_issue))
+      end
+    end
+  end
+
+  describe '.discussion_continued_in_issue' do
+    let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first }
+    let(:merge_request) { discussion.noteable }
+    let(:project) { merge_request.source_project }
+    let(:issue) { create(:issue, project: project) }
+    let(:user) { create(:user) }
+
+    def reloaded_merge_request
+      MergeRequest.find(merge_request.id)
+    end
+
+    before do
+      project.team << [user, :developer]
+    end
+
+    it 'creates a new note in the discussion' do
+      # we need to completely rebuild the merge request object, or the `@discussions` on the merge request are not reloaded.
+      expect { SystemNoteService.discussion_continued_in_issue(discussion, project, user, issue) }.
+        to change { reloaded_merge_request.discussions.first.notes.size }.by(1)
+    end
+
+    it 'mentions the created issue in the system note' do
+      note = SystemNoteService.discussion_continued_in_issue(discussion, project, user, issue)
 
-      it { is_expected.not_to eq(success_message) }
+      expect(note.note).to include(issue.to_reference)
     end
   end
 end