Merge branch '276444-tidy-up-merge-request-' into 'master'

Tidy up merge request code

See merge request gitlab-org/gitlab!48945

app/assets/javascripts/merge_request.js

@@ -102,14 +102,6 @@ MergeRequest.prototype.initMRBtnListeners = function() {
   return $('.btn-close, .btn-reopen').on('click', function(e) {
     const $this = $(this);
     const shouldSubmit = $this.hasClass('btn-comment');
-    if ($this.hasClass('js-btn-issue-action')) {
-      const url = $this.data('endpoint');
-      return axios
-        .put(url)
-        .then(() => window.location.reload())
-        .catch(() => createFlash(__('Something went wrong.')));
-    }
-
     if (shouldSubmit && $this.data('submitted')) {
       return;
     }
@@ -171,10 +163,6 @@ MergeRequest.decreaseCounter = function(by = 1) {
 
 MergeRequest.hideCloseButton = function() {
   const el = document.querySelector('.merge-request .js-issuable-actions');
-  const closeDropdownItem = el.querySelector('li.close-item');
-  if (closeDropdownItem) {
-    closeDropdownItem.classList.add('hidden');
-  }
   // Dropdown for mobile screen
   el.querySelector('li.js-close-item').classList.add('hidden');
 };

app/assets/stylesheets/pages/detail_page.scss

@@ -41,12 +41,6 @@
   @include media-breakpoint-down(xs) {
     width: 100%;
     margin-top: 10px;
-
-    > .issue-btn-group {
-      > .btn {
-        width: 100%;
-      }
-    }
   }
 }
 

app/assets/stylesheets/pages/issuable.scss

@@ -890,24 +890,6 @@
   }
 }
 
-.issuable-close-dropdown {
-  .dropdown-menu {
-    min-width: 270px;
-    left: auto;
-    right: 0;
-  }
-
-  .description {
-    .text {
-      margin: 0;
-    }
-  }
-
-  .dropdown-toggle > .icon {
-    margin: 0 3px;
-  }
-}
-
 /*
  * Following overrides are done to prevent
  * legacy dropdown styles from influencing

app/helpers/issuables_helper.rb

@@ -319,12 +319,6 @@ module IssuablesHelper
     issuable_path(issuable, close_reopen_params(issuable, :reopen))
   end
 
-  def toggle_draft_issuable_path(issuable)
-    wip_event = issuable.work_in_progress? ? 'unwip' : 'wip'
-
-    issuable_path(issuable, { merge_request: { wip_event: wip_event } })
-  end
-
   def issuable_path(issuable, *options)
     polymorphic_path(issuable, *options)
   end

app/helpers/merge_requests_helper.rb

@@ -39,19 +39,6 @@ module MergeRequestsHelper
     end
   end
 
-  def ci_build_details_path(merge_request)
-    build_url = merge_request.source_project.ci_service.build_page(merge_request.diff_head_sha, merge_request.source_branch)
-    return unless build_url
-
-    parsed_url = URI.parse(build_url)
-
-    unless parsed_url.userinfo.blank?
-      parsed_url.userinfo = ''
-    end
-
-    parsed_url.to_s
-  end
-
   def merge_path_description(merge_request, separator)
     if merge_request.for_fork?
       "Project:Branches: #{@merge_request.source_project_path}:#{@merge_request.source_branch} #{separator} #{@merge_request.target_project.full_path}:#{@merge_request.target_branch}"
@@ -166,6 +153,12 @@ module MergeRequestsHelper
       current_user.fork_of(project)
     end
   end
+
+  def toggle_draft_merge_request_path(issuable)
+    wip_event = issuable.work_in_progress? ? 'unwip' : 'wip'
+
+    issuable_path(issuable, { merge_request: { wip_event: wip_event } })
+  end
 end
 
 MergeRequestsHelper.prepend_if_ee('EE::MergeRequestsHelper')

app/views/shared/issuable/_close_reopen_draft_report_toggle.html.haml → app/views/projects/merge_requests/_close_reopen_draft_report_toggle.html.haml

-- display_issuable_type = issuable_display_type(issuable)
-- button_action_class = issuable.closed? ? 'btn-default' : 'btn-warning btn-warning-secondary'
-- button_class = "btn gl-button #{!issuable.closed? && 'js-draft-toggle-button'}"
+- display_issuable_type = issuable_display_type(@merge_request)
+- button_action_class = @merge_request.closed? ? 'btn-default' : 'btn-warning btn-warning-secondary'
+- button_class = "btn gl-button #{!@merge_request.closed? && 'js-draft-toggle-button'}"
 - toggle_class = "btn gl-button dropdown-toggle"
 
-.float-left.btn-group.gl-ml-3.issuable-close-dropdown.d-none.d-md-inline-flex.js-issuable-close-dropdown
-  = link_to issuable.closed? ? reopen_issuable_path(issuable) : toggle_draft_issuable_path(issuable), method: :put, class: "#{button_class} #{button_action_class}" do
-    - if issuable.closed?
+.float-left.btn-group.gl-ml-3.gl-display-none.gl-display-md-flex
+  = link_to @merge_request.closed? ? reopen_issuable_path(@merge_request) : toggle_draft_merge_request_path(@merge_request), method: :put, class: "#{button_class} #{button_action_class}" do
+    - if @merge_request.closed?
       = _('Reopen')
       = display_issuable_type
     - else
-      = issuable.work_in_progress? ? _('Mark as ready') : _('Mark as draft')
+      = @merge_request.work_in_progress? ? _('Mark as ready') : _('Mark as draft')
 
-  - if !issuable.closed? || !issuable_author_is_current_user(issuable)
+  - if !@merge_request.closed? || !issuable_author_is_current_user(@merge_request)
     = button_tag type: 'button', class: "#{toggle_class} #{button_action_class}", data: { 'toggle' => 'dropdown' } do
-      %span.sr-only= _('Toggle dropdown')
+      %span.gl-sr-only= _('Toggle dropdown')
       = sprite_icon "angle-down", size: 12
 
-    %ul.js-issuable-close-menu.dropdown-menu.dropdown-menu-right
-      - if issuable.open?
+    %ul.dropdown-menu.dropdown-menu-right
+      - if @merge_request.open?
         %li
-          = link_to close_issuable_path(issuable), method: :put do
+          = link_to close_issuable_path(@merge_request), method: :put do
             .description
               %strong.title
                 = _('Close')
                 = display_issuable_type
 
-      - unless issuable_author_is_current_user(issuable)
-        - unless issuable.closed?
+      - unless issuable_author_is_current_user(@merge_request)
+        - unless @merge_request.closed?
           %li.divider.droplab-item-ignore
 
-        %li.report-item
-          %a.report-abuse-link{ href: new_abuse_report_path(user_id: issuable.author.id, ref_url: merge_request_url(issuable)) }
+        %li
+          %a{ href: new_abuse_report_path(user_id: @merge_request.author.id, ref_url: merge_request_url(@merge_request)) }
             .description
               %strong.title= _('Report abuse')
-              %p.text
+              %p.text.gl-mb-0
                 = _('Report %{display_issuable_type} that are abusive, inappropriate or spam.') % { display_issuable_type: display_issuable_type.pluralize }

app/views/projects/merge_requests/_mr_title.html.haml

@@ -25,8 +25,8 @@
       = sprite_icon('chevron-double-lg-left')
 
   .detail-page-header-actions.js-issuable-actions
-    .clearfix.issue-btn-group.dropdown
-      %button.gl-button.btn.btn-default.float-left.gl-display-md-none{ type: "button", data: { toggle: "dropdown" } }
+    .clearfix.dropdown
+      %button.gl-button.btn.btn-default.float-left.gl-display-md-none.gl-w-full{ type: "button", data: { toggle: "dropdown" } }
         Options
         = sprite_icon('chevron-down', css_class: 'gl-text-gray-500')
       .dropdown-menu.dropdown-menu-right
@@ -35,12 +35,12 @@
             %li= link_to 'Edit', edit_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
             - if @merge_request.opened?
               %li
-                = link_to @merge_request.work_in_progress? ? _('Mark as ready') : _('Mark as draft'), toggle_draft_issuable_path(@merge_request), method: :put, class: "js-draft-toggle-button"
+                = link_to @merge_request.work_in_progress? ? _('Mark as ready') : _('Mark as draft'), toggle_draft_merge_request_path(@merge_request), method: :put, class: "js-draft-toggle-button"
             %li{ class: [merge_request_button_visibility(@merge_request, true), 'js-close-item'] }
               = link_to 'Close', merge_request_path(@merge_request, merge_request: { state_event: :close }), method: :put, title: 'Close merge request'
           - if can_reopen_merge_request
             %li{ class: merge_request_button_visibility(@merge_request, false) }
-              = link_to 'Reopen', merge_request_path(@merge_request, merge_request: { state_event: :reopen }), method: :put, class: 'reopen-mr-link', title: 'Reopen merge request'
+              = link_to 'Reopen', merge_request_path(@merge_request, merge_request: { state_event: :reopen }), method: :put, title: 'Reopen merge request'
           - unless @merge_request.merged? || current_user == @merge_request.author
             %li= link_to 'Report abuse', new_abuse_report_path(user_id: @merge_request.author.id, ref_url: merge_request_url(@merge_request))
 
@@ -48,6 +48,6 @@
         = link_to 'Edit', edit_project_merge_request_path(@project, @merge_request), class: "d-none d-md-block btn gl-button btn-grouped js-issuable-edit qa-edit-button"
 
       - if can_update_merge_request && !are_close_and_open_buttons_hidden
-        = render 'shared/issuable/close_reopen_draft_report_toggle', issuable: @merge_request
+        = render 'projects/merge_requests/close_reopen_draft_report_toggle'
       - elsif !@merge_request.merged?
         = link_to _('Report abuse'), new_abuse_report_path(user_id: @merge_request.author.id, ref_url: merge_request_url(@merge_request)), class: 'gl-display-none gl-display-md-block gl-button btn btn-warning-secondary float-right gl-ml-3', title: _('Report abuse')

locale/gitlab.pot

@@ -25646,9 +25646,6 @@ msgstr ""
 msgid "Something went wrong, unable to search projects"
 msgstr ""
 
-msgid "Something went wrong."
-msgstr ""
-
 msgid "Something went wrong. Please try again."
 msgstr ""
 

spec/features/issuables/close_reopen_report_toggle_spec.rb → spec/features/merge_request/close_reopen_report_toggle_spec.rb

@@ -7,44 +7,6 @@ RSpec.describe 'Issuables Close/Reopen/Report toggle' do
 
   let(:user) { create(:user) }
 
-  shared_examples 'an issuable close/reopen/report toggle' do
-    let(:container) { find('.issuable-close-dropdown') }
-    let(:human_model_name) { issuable.model_name.human.downcase }
-
-    it 'shows toggle' do
-      expect(page).to have_button("Close #{human_model_name}")
-      expect(page).to have_selector('.issuable-close-dropdown')
-    end
-
-    it 'opens a dropdown when toggle is clicked' do
-      container.find('.dropdown-toggle').click
-
-      expect(container).to have_selector('.dropdown-menu')
-      expect(container).to have_content("Close #{human_model_name}")
-      expect(container).to have_content('Report abuse')
-      expect(container).to have_content("Report #{human_model_name.pluralize} that are abusive, inappropriate or spam.")
-
-      if issuable.is_a?(MergeRequest)
-        page.within('.js-issuable-close-dropdown') do
-          expect(page).to have_link('Close merge request')
-        end
-      else
-        expect(container).to have_selector('.close-item.droplab-item-selected')
-      end
-
-      expect(container).to have_selector('.report-item')
-      expect(container).not_to have_selector('.report-item.droplab-item-selected')
-      expect(container).not_to have_selector('.reopen-item')
-    end
-
-    it 'links to Report Abuse' do
-      container.find('.dropdown-toggle').click
-      container.find('.report-abuse-link').click
-
-      expect(page).to have_content('Report abuse to admin')
-    end
-  end
-
   context 'on a merge request' do
     let(:container) { find('.detail-page-header-actions') }
     let(:project) { create(:project, :repository) }
@@ -60,7 +22,22 @@ RSpec.describe 'Issuables Close/Reopen/Report toggle' do
         visit project_merge_request_path(project, issuable)
       end
 
-      it_behaves_like 'an issuable close/reopen/report toggle'
+      context 'close/reopen/report toggle' do
+        it 'opens a dropdown when toggle is clicked' do
+          click_button 'Toggle dropdown'
+
+          expect(container).to have_link("Close merge request")
+          expect(container).to have_link('Report abuse')
+          expect(container).to have_text("Report merge requests that are abusive, inappropriate or spam.")
+        end
+
+        it 'links to Report Abuse' do
+          click_button 'Toggle dropdown'
+          click_link 'Report abuse'
+
+          expect(page).to have_content('Report abuse to admin')
+        end
+      end
 
       context 'when the merge request is open' do
         let(:issuable) { create(:merge_request, :opened, source_project: project) }

spec/features/merge_request/user_reopens_merge_request_spec.rb

@@ -15,7 +15,7 @@ RSpec.describe 'User reopens a merge requests', :js do
   end
 
   it 'reopens a merge request' do
-    find('.js-issuable-close-dropdown .dropdown-toggle').click
+    find('.detail-page-header .dropdown-toggle').click
 
     click_link('Reopen merge request', match: :first)
 

spec/helpers/merge_requests_helper_spec.rb

@@ -6,26 +6,6 @@ RSpec.describe MergeRequestsHelper do
   include ActionView::Helpers::UrlHelper
   include ProjectForksHelper
 
-  describe 'ci_build_details_path' do
-    let(:project) { create(:project) }
-    let(:merge_request) { MergeRequest.new }
-    let(:ci_service) { CiService.new }
-    let(:last_commit) { Ci::Pipeline.new({}) }
-
-    before do
-      allow(merge_request).to receive(:source_project).and_return(project)
-      allow(merge_request).to receive(:last_commit).and_return(last_commit)
-      allow(project).to receive(:ci_service).and_return(ci_service)
-      allow(last_commit).to receive(:sha).and_return('12d65c')
-    end
-
-    it 'does not include api credentials in a link' do
-      allow(ci_service)
-        .to receive(:build_page).and_return("http://secretuser:secretpass@jenkins.example.com:8888/job/test1/scm/bySHA1/12d65c")
-      expect(helper.ci_build_details_path(merge_request)).not_to match("secret")
-    end
-  end
-
   describe '#state_name_with_icon' do
     using RSpec::Parameterized::TableSyntax