Merge branch 'ce-to-ee-2018-08-21' into 'master'

CE upstream - 2018-08-21 21:21 UTC

Closes gitlab-ce#40550

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

app/assets/javascripts/pipelines/components/pipelines_table_row.vue

@@ -330,7 +330,7 @@ export default {
         <pipelines-artifacts-component
           v-if="pipeline.details.artifacts.length"
           :artifacts="pipeline.details.artifacts"
-          class="d-none d-sm-none d-md-block"
+          class="d-md-block"
         />
 
         <loading-button

app/controllers/concerns/issuable_collections.rb

@@ -108,11 +108,15 @@ module IssuableCollections
   end
 
   def set_sort_order_from_cookie
-    key = 'issuable_sort'
+    cookies[remember_sorting_key] = params[:sort] if params[:sort].present?
+    # fallback to legacy cookie value for backward compatibility
+    cookies[remember_sorting_key] ||= cookies['issuable_sort']
+    cookies[remember_sorting_key] = update_cookie_value(cookies[remember_sorting_key])
+    params[:sort] = cookies[remember_sorting_key]
+  end
 
-    cookies[key] = params[:sort] if params[:sort].present?
-    cookies[key] = update_cookie_value(cookies[key])
-    params[:sort] = cookies[key]
+  def remember_sorting_key
+    @remember_sorting_key ||= "#{collection_type.downcase}_sort"
   end
 
   def default_sort_order
@@ -141,16 +145,14 @@ module IssuableCollections
   end
 
   def finder
-    strong_memoize(:finder) do
-      issuable_finder_for(finder_type)
-    end
+    @finder ||= issuable_finder_for(finder_type)
   end
 
   def collection_type
-    @collection_type ||= case finder
-                         when IssuesFinder
+    @collection_type ||= case finder_type.name
+                         when 'IssuesFinder'
                            'Issue'
-                         when MergeRequestsFinder
+                         when 'MergeRequestsFinder'
                            'MergeRequest'
                          end
   end

changelogs/unreleased/48869-wiki-slugs-with-spaces.yml

+---
+title: Allow spaces in wiki markdown links when using CommonMark
+merge_request: 20417
+author:
+type: fixed

changelogs/unreleased/49993-fix-remember-sorting-issue-mr.yml

+---
+title: Split remembering sorting for issues and merge requests
+merge_request: 21153
+author: Jacopo Beschi @jacopo-beschi
+type: fixed

changelogs/unreleased/50524-artifacts-sm.yml

+---
+title: Shows download artifacts button for pipelines on small screens
+merge_request:
+author:
+type: changed

changelogs/unreleased/arguments-keyword-sast.yml

+---
+title: Don't use arguments keyword in gettext script
+merge_request: 21296
+author: gfyoung
+type: fixed

lib/banzai/filter/spaced_link_filter.rb

+# frozen_string_literal: true
+
+require 'uri'
+
+module Banzai
+  module Filter
+    # HTML Filter for markdown links with spaces in the URLs
+    #
+    # Based on Banzai::Filter::AutolinkFilter
+    #
+    # CommonMark does not allow spaces in the url portion of a link.
+    # For example, `[example](page slug)` is not valid.  However,
+    # in our wikis, we support (via RedCarpet) this type of link, allowing
+    # wiki pages to be easily linked by their title.  This filter adds that functionality.
+    # The intent is for this to only be used in Wikis - in general, we want
+    # to adhere to CommonMark's spec.
+    #
+    class SpacedLinkFilter < HTML::Pipeline::Filter
+      include ActionView::Helpers::TagHelper
+
+      # Pattern to match a standard markdown link
+      #
+      # Rubular: http://rubular.com/r/z9EAHxYmKI
+      LINK_PATTERN = /\[([^\]]+)\]\(([^)"]+)(?: \"([^\"]+)\")?\)/
+
+      # Text matching LINK_PATTERN inside these elements will not be linked
+      IGNORE_PARENTS = %w(a code kbd pre script style).to_set
+
+      # The XPath query to use for finding text nodes to parse.
+      TEXT_QUERY = %Q(descendant-or-self::text()[
+        not(#{IGNORE_PARENTS.map { |p| "ancestor::#{p}" }.join(' or ')})
+        and contains(., ']\(')
+      ]).freeze
+
+      def call
+        return doc if context[:markdown_engine] == :redcarpet
+
+        doc.xpath(TEXT_QUERY).each do |node|
+          content = node.to_html
+
+          next unless content.match(LINK_PATTERN)
+
+          html = spaced_link_filter(content)
+
+          next if html == content
+
+          node.replace(html)
+        end
+
+        doc
+      end
+
+      private
+
+      def spaced_link_match(link)
+        match = LINK_PATTERN.match(link)
+        return link unless match && match[1] && match[2]
+
+        # escape the spaces in the url so that it's a valid markdown link,
+        # then run it through the markdown processor again, let it do its magic
+        text     = match[1]
+        new_link = match[2].gsub(' ', '%20')
+        title    = match[3] ? " \"#{match[3]}\"" : ''
+        html     = Banzai::Filter::MarkdownFilter.call("[#{text}](#{new_link}#{title})", context)
+
+        # link is wrapped in a <p>, so strip that off
+        html.sub('<p>', '').chomp('</p>')
+      end
+
+      def spaced_link_filter(text)
+        Gitlab::StringRegexMarker.new(CGI.unescapeHTML(text), text.html_safe).mark(LINK_PATTERN) do |link, left:, right:|
+          spaced_link_match(link)
+        end
+      end
+    end
+  end
+end

lib/banzai/pipeline/wiki_pipeline.rb

@@ -5,6 +5,7 @@ module Banzai
         @filters ||= begin
           super.insert_after(Filter::TableOfContentsFilter, Filter::GollumTagsFilter)
                .insert_before(Filter::TaskListFilter, Filter::WikiLinkFilter)
+               .insert_before(Filter::WikiLinkFilter, Filter::SpacedLinkFilter)
         end
       end
     end

qa/qa/page/project/settings/secret_variables.rb

@@ -23,7 +23,13 @@ module QA
             # After we fill the key, JS would generate another field so
             # we need to use the same index to find the corresponding one.
             keys[index].set(key)
-            all_elements(:ci_variable_input_value)[index].set(value)
+            node = all_elements(:ci_variable_input_value)[index]
+
+            # Simply run `node.set(value)` is too slow for long text here,
+            # so we need to run JavaScript directly to set the value.
+            # The code was inspired from:
+            # https://github.com/teamcapybara/capybara/blob/679548cea10773d45e32808f4d964377cfe5e892/lib/capybara/selenium/node.rb#L217
+            execute_script("arguments[0].value = #{value.to_json}", node)
           end
 
           def save_variables

scripts/frontend/extract_gettext_all.js

@@ -7,7 +7,7 @@ const {
 } = require('gettext-extractor-vue');
 const ensureSingleLine = require('../../app/assets/javascripts/locale/ensure_single_line.js');
 
-const arguments = argumentsParser
+const args = argumentsParser
   .option('-f, --file <file>', 'Extract message from one single file')
   .option('-a, --all', 'Extract message from all js/vue files')
   .parse(process.argv);
@@ -61,12 +61,12 @@ function printJson() {
   console.log(JSON.stringify(messages));
 }
 
-if (arguments.file) {
-  vueParser.parseFile(arguments.file).then(() => printJson());
-} else if (arguments.all) {
+if (args.file) {
+  vueParser.parseFile(args.file).then(() => printJson());
+} else if (args.all) {
   vueParser.parseFilesGlob('{ee/app,app}/assets/javascripts/**/*.{js,vue}').then(() => printJson());
 } else {
   console.warn('ERROR: Please use the script correctly:');
-  arguments.outputHelp();
+  args.outputHelp();
   process.exit(1);
 }

spec/features/projects/issues/user_sorts_issues_spec.rb

 require "spec_helper"
 
 describe "User sorts issues" do
-  set(:project) { create(:project_empty_repo, :public) }
+  set(:user) { create(:user) }
+  set(:group) { create(:group) }
+  set(:project) { create(:project_empty_repo, :public, group: group) }
   set(:issue1) { create(:issue, project: project) }
   set(:issue2) { create(:issue, project: project) }
   set(:issue3) { create(:issue, project: project) }
@@ -12,7 +14,29 @@ describe "User sorts issues" do
     create(:award_emoji, :downvote, awardable: issue1)
     create(:award_emoji, :upvote, awardable: issue2)
 
+    sign_in(user)
+
+    visit(project_issues_path(project))
+  end
+
+  it 'keeps the sort option' do
+    find('button.dropdown-toggle').click
+
+    page.within('.content ul.dropdown-menu.dropdown-menu-right li') do
+      click_link('Milestone')
+    end
+
+    visit(issues_dashboard_path(assignee_id: user.id))
+
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+
     visit(project_issues_path(project))
+
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+
+    visit(issues_group_path(group))
+
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
   end
 
   it "sorts by popularity" do

spec/features/projects/merge_requests/user_sorts_merge_requests_spec.rb

 require 'spec_helper'
 
 describe 'User sorts merge requests' do
+  include CookieHelper
+
   let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
   let!(:merge_request2) do
     create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test')
   end
-  let(:project) { create(:project, :public, :repository) }
-  let(:user) { create(:user) }
+  set(:user) { create(:user) }
+  set(:group) { create(:group) }
+  set(:group_member) { create(:group_member, :maintainer, user: user, group: group) }
+  set(:project) { create(:project, :public, group: group) }
 
   before do
-    project.add_maintainer(user)
     sign_in(user)
 
     visit(project_merge_requests_path(project))
@@ -19,16 +22,42 @@ describe 'User sorts merge requests' do
     find('button.dropdown-toggle').click
 
     page.within('.content ul.dropdown-menu.dropdown-menu-right li') do
-      click_link('Last updated')
+      click_link('Milestone')
     end
 
     visit(merge_requests_dashboard_path(assignee_id: user.id))
 
-    expect(find('.issues-filters')).to have_content('Last updated')
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
 
     visit(project_merge_requests_path(project))
 
-    expect(find('.issues-filters')).to have_content('Last updated')
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+
+    visit(merge_requests_group_path(group))
+
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+  end
+
+  it 'fallbacks to issuable_sort cookie key when remembering the sorting option' do
+    set_cookie('issuable_sort', 'milestone')
+
+    visit(merge_requests_dashboard_path(assignee_id: user.id))
+
+    expect(find('.issues-filters a.is-active')).to have_content('Milestone')
+  end
+
+  it 'separates remember sorting with issues' do
+    create(:issue, project: project)
+
+    find('button.dropdown-toggle').click
+
+    page.within('.content ul.dropdown-menu.dropdown-menu-right li') do
+      click_link('Milestone')
+    end
+
+    visit(project_issues_path(project))
+
+    expect(find('.issues-filters a.is-active')).not_to have_content('Milestone')
   end
 
   context 'when merge requests have awards' do

spec/javascripts/jobs/artifacts_block_spec.js → spec/javascripts/jobs/components/artifacts_block_spec.js

 import Vue from 'vue';
 import { getTimeago } from '~/lib/utils/datetime_utility';
 import component from '~/jobs/components/artifacts_block.vue';
-import mountComponent from '../helpers/vue_mount_component_helper';
+import mountComponent from '../../helpers/vue_mount_component_helper';
 
 describe('Artifacts block', () => {
   const Component = Vue.extend(component);

spec/javascripts/jobs/commit_block_spec.js → spec/javascripts/jobs/components/commit_block_spec.js

 import Vue from 'vue';
 import component from '~/jobs/components/commit_block.vue';
-import mountComponent from '../helpers/vue_mount_component_helper';
+import mountComponent from '../../helpers/vue_mount_component_helper';
 
 describe('Commit block', () => {
   const Component = Vue.extend(component);

spec/javascripts/jobs/empty_state_spec.js → spec/javascripts/jobs/components/empty_state_spec.js

 import Vue from 'vue';
 import component from '~/jobs/components/empty_state.vue';
-import mountComponent from '../helpers/vue_mount_component_helper';
+import mountComponent from '../../helpers/vue_mount_component_helper';
 
 describe('Empty State', () => {
   const Component = Vue.extend(component);

spec/javascripts/jobs/erased_block_spec.js → spec/javascripts/jobs/components/erased_block_spec.js

 import Vue from 'vue';
 import { getTimeago } from '~/lib/utils/datetime_utility';
 import component from '~/jobs/components/erased_block.vue';
-import mountComponent from '../helpers/vue_mount_component_helper';
+import mountComponent from '../../helpers/vue_mount_component_helper';
 
 describe('Erased block', () => {
   const Component = Vue.extend(component);

spec/javascripts/jobs/job_details_mediator_spec.js → spec/javascripts/jobs/components/job_details_mediator_spec.js

 import MockAdapter from 'axios-mock-adapter';
 import axios from '~/lib/utils/axios_utils';
 import JobMediator from '~/jobs/job_details_mediator';
-import job from './mock_data';
+import job from '../mock_data';
 
 describe('JobMediator', () => {
   let mediator;

spec/javascripts/jobs/job_log_spec.js → spec/javascripts/jobs/components/job_log_spec.js

 import Vue from 'vue';
 import component from '~/jobs/components/job_log.vue';
-import mountComponent from '../helpers/vue_mount_component_helper';
+import mountComponent from '../../helpers/vue_mount_component_helper';
 
 describe('Job Log', () => {
   const Component = Vue.extend(component);

spec/javascripts/jobs/job_store_spec.js → spec/javascripts/jobs/components/job_store_spec.js

 import JobStore from '~/jobs/stores/job_store';
-import job from './mock_data';
+import job from '../mock_data';
 
 describe('Job Store', () => {
   let store;

spec/javascripts/jobs/jobs_container_spec.js → spec/javascripts/jobs/components/jobs_container_spec.js

 import Vue from 'vue';
 import component from '~/jobs/components/jobs_container.vue';
-import mountComponent from '../helpers/vue_mount_component_helper';
+import mountComponent from '../../helpers/vue_mount_component_helper';
 
 describe('Artifacts block', () => {
   const Component = Vue.extend(component);

spec/javascripts/jobs/sidebar_details_block_spec.js → spec/javascripts/jobs/components/sidebar_details_block_spec.js

 import Vue from 'vue';
 import sidebarDetailsBlock from '~/jobs/components/sidebar_details_block.vue';
-import job from './mock_data';
-import mountComponent from '../helpers/vue_mount_component_helper';
+import job from '../mock_data';
+import mountComponent from '../../helpers/vue_mount_component_helper';
 
 describe('Sidebar details block', () => {
   let SidebarComponent;

spec/javascripts/jobs/stages_dropdown_spec.js → spec/javascripts/jobs/components/stages_dropdown_spec.js

 import Vue from 'vue';
 import component from '~/jobs/components/stages_dropdown.vue';
-import mountComponent from '../helpers/vue_mount_component_helper';
+import mountComponent from '../../helpers/vue_mount_component_helper';
 
 describe('Artifacts block', () => {
   const Component = Vue.extend(component);

spec/javascripts/jobs/stuck_block_spec.js → spec/javascripts/jobs/components/stuck_block_spec.js

 import Vue from 'vue';
 import component from '~/jobs/components/stuck_block.vue';
-import mountComponent from '../helpers/vue_mount_component_helper';
+import mountComponent from '../../helpers/vue_mount_component_helper';
 
 describe('Stuck Block Job component', () => {
   const Component = Vue.extend(component);

spec/javascripts/jobs/trigger_value_spec.js → spec/javascripts/jobs/components/trigger_value_spec.js

 import Vue from 'vue';
 import component from '~/jobs/components/trigger_block.vue';
-import mountComponent from '../helpers/vue_mount_component_helper';
+import mountComponent from '../../helpers/vue_mount_component_helper';
 
 describe('Trigger block', () => {
   const Component = Vue.extend(component);

spec/lib/banzai/filter/spaced_link_filter_spec.rb

+require 'spec_helper'
+
+describe Banzai::Filter::SpacedLinkFilter do
+  include FilterSpecHelper
+
+  let(:link) { '[example](page slug)' }
+
+  it 'converts slug with spaces to a link' do
+    doc = filter("See #{link}")
+
+    expect(doc.at_css('a').text).to eq 'example'
+    expect(doc.at_css('a')['href']).to eq 'page%20slug'
+    expect(doc.at_css('p')).to eq nil
+  end
+
+  it 'converts slug with spaces and a title to a link' do
+    link = '[example](page slug "title")'
+    doc  = filter("See #{link}")
+
+    expect(doc.at_css('a').text).to eq 'example'
+    expect(doc.at_css('a')['href']).to eq 'page%20slug'
+    expect(doc.at_css('a')['title']).to eq 'title'
+    expect(doc.at_css('p')).to eq nil
+  end
+
+  it 'does nothing when markdown_engine is redcarpet' do
+    exp = act = link
+    expect(filter(act, markdown_engine: :redcarpet).to_html).to eq exp
+  end
+
+  it 'does nothing with empty text' do
+    link = '[](page slug)'
+    doc  = filter("See #{link}")
+
+    expect(doc.at_css('a')).to eq nil
+  end
+
+  it 'does nothing with an empty slug' do
+    link = '[example]()'
+    doc  = filter("See #{link}")
+
+    expect(doc.at_css('a')).to eq nil
+  end
+
+  it 'converts multiple URLs' do
+    link1 = '[first](slug one)'
+    link2 = '[second](http://example.com/slug two)'
+    doc   = filter("See #{link1} and #{link2}")
+
+    found_links = doc.css('a')
+
+    expect(found_links.size).to eq(2)
+    expect(found_links[0].text).to eq 'first'
+    expect(found_links[0]['href']).to eq 'slug%20one'
+    expect(found_links[1].text).to eq 'second'
+    expect(found_links[1]['href']).to eq 'http://example.com/slug%20two'
+  end
+
+  described_class::IGNORE_PARENTS.each do |elem|
+    it "ignores valid links contained inside '#{elem}' element" do
+      exp = act = "<#{elem}>See #{link}</#{elem}>"
+
+      expect(filter(act).to_html).to eq exp
+    end
+  end
+end

spec/models/project_services/jira_service_spec.rb

@@ -27,13 +27,13 @@ describe JiraService do
     end
   end
 
-  describe "Associations" do
+  describe 'Associations' do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
     it { is_expected.to allow_value(nil).for(:jira_issue_transition_id) }
-    it { is_expected.to allow_value("1,2,3").for(:jira_issue_transition_id) }
-    it { is_expected.to allow_value("1;2;3").for(:jira_issue_transition_id) }
-    it { is_expected.not_to allow_value("a,b,cd").for(:jira_issue_transition_id) }
+    it { is_expected.to allow_value('1,2,3').for(:jira_issue_transition_id) }
+    it { is_expected.to allow_value('1;2;3').for(:jira_issue_transition_id) }
+    it { is_expected.not_to allow_value('a,b,cd').for(:jira_issue_transition_id) }
   end
 
   describe 'Validations' do
@@ -116,143 +116,142 @@ describe JiraService do
   describe '#close_issue' do
     let(:custom_base_url) { 'http://custom_url' }
     let(:user)    { create(:user) }
-    let(:project) { create(:project) }
-    let(:merge_request) { create(:merge_request) }
-
-    before do
-      @jira_service = described_class.new
-      allow(@jira_service).to receive_messages(
-        project_id: project.id,
-        project: project,
-        service_hook: true,
-        url: 'http://jira.example.com',
-        username: 'gitlab_jira_username',
-        password: 'gitlab_jira_password',
-        jira_issue_transition_id: "999"
-      )
+    let(:project) { create(:project, :repository) }
 
-      # These stubs are needed to test JiraService#close_issue.
-      # We close the issue then do another request to API to check if it got closed.
-      # Here is stubbed the API return with a closed and an opened issues.
-      open_issue   = JIRA::Resource::Issue.new(@jira_service.client, attrs: { "id" => "JIRA-123" })
-      closed_issue = open_issue.dup
-      allow(open_issue).to receive(:resolution).and_return(false)
-      allow(closed_issue).to receive(:resolution).and_return(true)
-      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
-
-      project_issues_url = 'http://jira.example.com/rest/api/2/issue/JIRA-123'
-      @transitions_url   = 'http://jira.example.com/rest/api/2/issue/JIRA-123/transitions'
-      @comment_url       = 'http://jira.example.com/rest/api/2/issue/JIRA-123/comment'
-      @remote_link_url   = 'http://jira.example.com/rest/api/2/issue/JIRA-123/remotelink'
-
-      WebMock.stub_request(:get, project_issues_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password))
-      WebMock.stub_request(:post, @transitions_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password))
-      WebMock.stub_request(:post, @comment_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password))
-      WebMock.stub_request(:post, @remote_link_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password))
-    end
+    shared_examples 'close_issue' do
+      before do
+        @jira_service = described_class.new
+        allow(@jira_service).to receive_messages(
+          project_id: project.id,
+          project: project,
+          service_hook: true,
+          url: 'http://jira.example.com',
+          username: 'gitlab_jira_username',
+          password: 'gitlab_jira_password',
+          jira_issue_transition_id: '999'
+        )
 
-    it "calls JIRA API" do
-      @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
+        # These stubs are needed to test JiraService#close_issue.
+        # We close the issue then do another request to API to check if it got closed.
+        # Here is stubbed the API return with a closed and an opened issues.
+        open_issue   = JIRA::Resource::Issue.new(@jira_service.client, attrs: { 'id' => 'JIRA-123' })
+        closed_issue = open_issue.dup
+        allow(open_issue).to receive(:resolution).and_return(false)
+        allow(closed_issue).to receive(:resolution).and_return(true)
+        allow(JIRA::Resource::Issue).to receive(:find).and_return(open_issue, closed_issue)
 
-      expect(WebMock).to have_requested(:post, @comment_url).with(
-        body: /Issue solved with/
-      ).once
-    end
+        allow_any_instance_of(JIRA::Resource::Issue).to receive(:key).and_return('JIRA-123')
+        allow(JIRA::Resource::Remotelink).to receive(:all).and_return([])
 
-    # Check https://developer.atlassian.com/jiradev/jira-platform/guides/other/guide-jira-remote-issue-links/fields-in-remote-issue-links
-    # for more information
-    it "creates Remote Link reference in JIRA for comment" do
-      @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
-
-      favicon_path = "http://localhost/assets/#{find_asset('favicon.png').digest_path}"
-
-      # Creates comment
-      expect(WebMock).to have_requested(:post, @comment_url)
-      # Creates Remote Link in JIRA issue fields
-      expect(WebMock).to have_requested(:post, @remote_link_url).with(
-        body: hash_including(
-          GlobalID: "GitLab",
-          object: {
-            url: "#{Gitlab.config.gitlab.url}/#{project.full_path}/commit/#{merge_request.diff_head_sha}",
-            title: "GitLab: Solved by commit #{merge_request.diff_head_sha}.",
-            icon: { title: "GitLab", url16x16: favicon_path },
-            status: { resolved: true }
-          }
-        )
-      ).once
-    end
+        @jira_service.save
 
-    it "does not send comment or remote links to issues already closed" do
-      allow_any_instance_of(JIRA::Resource::Issue).to receive(:resolution).and_return(true)
+        project_issues_url = 'http://jira.example.com/rest/api/2/issue/JIRA-123'
+        @transitions_url   = 'http://jira.example.com/rest/api/2/issue/JIRA-123/transitions'
+        @comment_url       = 'http://jira.example.com/rest/api/2/issue/JIRA-123/comment'
+        @remote_link_url   = 'http://jira.example.com/rest/api/2/issue/JIRA-123/remotelink'
 
-      @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
+        WebMock.stub_request(:get, project_issues_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password))
+        WebMock.stub_request(:post, @transitions_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password))
+        WebMock.stub_request(:post, @comment_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password))
+        WebMock.stub_request(:post, @remote_link_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password))
+      end
 
-      expect(WebMock).not_to have_requested(:post, @comment_url)
-      expect(WebMock).not_to have_requested(:post, @remote_link_url)
-    end
+      it 'calls JIRA API' do
+        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
 
-    it "does not send comment or remote links to issues with unknown resolution" do
-      allow_any_instance_of(JIRA::Resource::Issue).to receive(:respond_to?).with(:resolution).and_return(false)
+        expect(WebMock).to have_requested(:post, @comment_url).with(
+          body: /Issue solved with/
+        ).once
+      end
 
-      @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
+      # Check https://developer.atlassian.com/jiradev/jira-platform/guides/other/guide-jira-remote-issue-links/fields-in-remote-issue-links
+      # for more information
+      it 'creates Remote Link reference in JIRA for comment' do
+        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
+
+        favicon_path = "http://localhost/assets/#{find_asset('favicon.png').digest_path}"
+
+        # Creates comment
+        expect(WebMock).to have_requested(:post, @comment_url)
+        # Creates Remote Link in JIRA issue fields
+        expect(WebMock).to have_requested(:post, @remote_link_url).with(
+          body: hash_including(
+            GlobalID: 'GitLab',
+            object: {
+              url: "#{Gitlab.config.gitlab.url}/#{project.full_path}/commit/#{commit_id}",
+              title: "GitLab: Solved by commit #{commit_id}.",
+              icon: { title: 'GitLab', url16x16: favicon_path },
+              status: { resolved: true }
+            }
+          )
+        ).once
+      end
 
-      expect(WebMock).not_to have_requested(:post, @comment_url)
-      expect(WebMock).not_to have_requested(:post, @remote_link_url)
-    end
+      it 'does not send comment or remote links to issues already closed' do
+        allow_any_instance_of(JIRA::Resource::Issue).to receive(:resolution).and_return(true)
 
-    it "references the GitLab commit/merge request" do
-      stub_config_setting(base_url: custom_base_url)
+        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
 
-      @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
+        expect(WebMock).not_to have_requested(:post, @comment_url)
+        expect(WebMock).not_to have_requested(:post, @remote_link_url)
+      end
 
-      expect(WebMock).to have_requested(:post, @comment_url).with(
-        body: %r{#{custom_base_url}/#{project.full_path}/commit/#{merge_request.diff_head_sha}}
-      ).once
-    end
+      it 'does not send comment or remote links to issues with unknown resolution' do
+        allow_any_instance_of(JIRA::Resource::Issue).to receive(:respond_to?).with(:resolution).and_return(false)
 
-    it "references the GitLab commit/merge request (relative URL)" do
-      stub_config_setting(relative_url_root: '/gitlab')
-      stub_config_setting(url: Settings.send(:build_gitlab_url))
+        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
 
-      allow(described_class).to receive(:default_url_options) do
-        { script_name: '/gitlab' }
+        expect(WebMock).not_to have_requested(:post, @comment_url)
+        expect(WebMock).not_to have_requested(:post, @remote_link_url)
       end
 
-      @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
+      it 'references the GitLab commit' do
+        stub_config_setting(base_url: custom_base_url)
 
-      expect(WebMock).to have_requested(:post, @comment_url).with(
-        body: %r{#{Gitlab.config.gitlab.url}/#{project.full_path}/commit/#{merge_request.diff_head_sha}}
-      ).once
-    end
+        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
+
+        expect(WebMock).to have_requested(:post, @comment_url).with(
+          body: %r{#{custom_base_url}/#{project.full_path}/commit/#{commit_id}}
+        ).once
+      end
+
+      it 'references the GitLab commit' do
+        stub_config_setting(relative_url_root: '/gitlab')
+        stub_config_setting(url: Settings.send(:build_gitlab_url))
+
+        allow(described_class).to receive(:default_url_options) do
+          { script_name: '/gitlab' }
+        end
+
+        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
 
-    context '#close_issue' do
-      it "logs exception when transition id is not valid" do
+        expect(WebMock).to have_requested(:post, @comment_url).with(
+          body: %r{#{Gitlab.config.gitlab.url}/#{project.full_path}/commit/#{commit_id}}
+        ).once
+      end
+
+      it 'logs exception when transition id is not valid' do
         allow(Rails.logger).to receive(:info)
-        WebMock.stub_request(:post, @transitions_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password)).and_raise("Bad Request")
+        WebMock.stub_request(:post, @transitions_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password)).and_raise('Bad Request')
 
-        @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
+        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
 
-        expect(Rails.logger).to have_received(:info).with("JiraService Issue Transition failed message ERROR: http://jira.example.com - Bad Request")
+        expect(Rails.logger).to have_received(:info).with('JiraService Issue Transition failed message ERROR: http://jira.example.com - Bad Request')
       end
 
-      it "calls the api with jira_issue_transition_id" do
-        @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
+      it 'calls the api with jira_issue_transition_id' do
+        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
 
         expect(WebMock).to have_requested(:post, @transitions_url).with(
           body: /999/
         ).once
       end
 
-      context "when have multiple transition ids" do
-        it "calls the api with transition ids separated by comma" do
-          allow(@jira_service).to receive_messages(jira_issue_transition_id: "1,2,3")
+      context 'when have multiple transition ids' do
+        it 'calls the api with transition ids separated by comma' do
+          allow(@jira_service).to receive_messages(jira_issue_transition_id: '1,2,3')
 
-          @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
+          @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
 
           1.upto(3) do |transition_id|
             expect(WebMock).to have_requested(:post, @transitions_url).with(
@@ -261,10 +260,10 @@ describe JiraService do
           end
         end
 
-        it "calls the api with transition ids separated by semicolon" do
-          allow(@jira_service).to receive_messages(jira_issue_transition_id: "1;2;3")
+        it 'calls the api with transition ids separated by semicolon' do
+          allow(@jira_service).to receive_messages(jira_issue_transition_id: '1;2;3')
 
-          @jira_service.close_issue(merge_request, ExternalIssue.new("JIRA-123", project))
+          @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
 
           1.upto(3) do |transition_id|
             expect(WebMock).to have_requested(:post, @transitions_url).with(
@@ -274,6 +273,20 @@ describe JiraService do
         end
       end
     end
+
+    context 'when resource is a merge request' do
+      let(:resource) { create(:merge_request) }
+      let(:commit_id) { resource.diff_head_sha }
+
+      it_behaves_like 'close_issue'
+    end
+
+    context 'when resource is a commit' do
+      let(:resource) { project.commit('master') }
+      let(:commit_id) { resource.id }
+
+      it_behaves_like 'close_issue'
+    end
   end
 
   describe '#test_settings' do
@@ -321,17 +334,17 @@ describe JiraService do
     end
   end
 
-  describe "Stored password invalidation" do
+  describe 'Stored password invalidation' do
     let(:project) { create(:project) }
 
-    context "when a password was previously set" do
+    context 'when a password was previously set' do
       before do
         @jira_service = described_class.create!(
           project: project,
           properties: {
             url: 'http://jira.example.com/web',
             username: 'mic',
-            password: "password"
+            password: 'password'
           }
         )
       end
@@ -370,10 +383,10 @@ describe JiraService do
           @jira_service.url = 'http://jira_edited.example.com/rweb'
           @jira_service.save
 
-          expect(@jira_service.password).to eq("password")
+          expect(@jira_service.password).to eq('password')
         end
 
-        it 'reset password if api url set to ""' do
+        it 'reset password if api url set to empty' do
           @jira_service.api_url = ''
           @jira_service.save
 
@@ -440,7 +453,7 @@ describe JiraService do
 
       it 'is initialized' do
         expect(@service.title).to eq('JIRA')
-        expect(@service.description).to eq("Jira issue tracker")
+        expect(@service.description).to eq('Jira issue tracker')
       end
     end
 
@@ -454,7 +467,7 @@ describe JiraService do
         @service.destroy!
       end
 
-      it "is correct" do
+      it 'is correct' do
         expect(@service.title).to eq('Jira One')
         expect(@service.description).to eq('Jira One issue tracker')
       end
@@ -476,7 +489,7 @@ describe JiraService do
 
       it 'is initialized' do
         expect(@service.options[:use_cookies]).to eq(true)
-        expect(@service.options[:additional_cookies]).to eq(["OBBasicAuth=fromDialog"])
+        expect(@service.options[:additional_cookies]).to eq(['OBBasicAuth=fromDialog'])
       end
     end
   end