Merge branch '282506-mark-as-draft' into 'master'

Hide Mark as draft button in a merged MR even on mobile

See merge request gitlab-org/gitlab!47678

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

@@ -32,8 +32,7 @@
         %ul
           - if can_update_merge_request
             %li= link_to 'Edit', edit_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
-          - if can_update_merge_request
-            - unless @merge_request.closed?
+            - 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"
             %li{ class: [merge_request_button_visibility(@merge_request, true), 'js-close-item'] }

changelogs/unreleased/282506-mark-as-draft.yml

+---
+title: Hide Mark as draft button in a merged MR even on mobile
+merge_request: 47678
+author: Takuya Noguchi
+type: fixed

ee/spec/views/projects/merge_requests/show.html.haml_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe 'projects/merge_requests/show.html.haml' do
     allow(view).to receive(:experiment_enabled?).and_return(false)
   end
 
-  include_context 'merge request show action'
+  include_context 'closed merge request show action'
 
   context 'when merge request is created by a GitLab team member' do
     let(:user) { create(:user) }

spec/features/issuables/close_reopen_report_toggle_spec.rb

@@ -121,6 +121,18 @@ RSpec.describe 'Issuables Close/Reopen/Report toggle' do
 
       it_behaves_like 'an issuable close/reopen/report toggle'
 
+      context 'when the merge request is open' do
+        let(:issuable) { create(:merge_request, :opened, source_project: project) }
+
+        it 'shows the `Edit` and `Mark as draft` buttons' do
+          expect(container).to have_link('Edit')
+          expect(container).to have_link('Mark as draft')
+          expect(container).not_to have_button('Report abuse')
+          expect(container).not_to have_button('Close merge request')
+          expect(container).not_to have_link('Reopen merge request')
+        end
+      end
+
       context 'when the merge request is closed' do
         let(:issuable) { create(:merge_request, :closed, source_project: project) }
 

spec/support/helpers/features/merge_request_helpers.rb

+# frozen_string_literal: true
+
+module Spec
+  module Support
+    module Helpers
+      module Features
+        module MergeRequestHelpers
+          def preload_view_requirements(merge_request, note)
+            # This will load the status fields of the author of the note and merge request
+            # to avoid queries in when rendering the view being tested.
+            merge_request.author.status
+            note.author.status
+          end
+
+          def serialize_issuable_sidebar(user, project, merge_request)
+            MergeRequestSerializer
+              .new(current_user: user, project: project)
+              .represent(merge_request, serializer: 'sidebar')
+          end
+        end
+      end
+    end
+  end
+end

spec/support/shared_contexts/issuable/merge_request_shared_context.rb

 # frozen_string_literal: true
 
-RSpec.shared_context 'merge request show action' do
+RSpec.shared_context 'open merge request show action' do
+  include Spec::Support::Helpers::Features::MergeRequestHelpers
+
+  let(:user) { create(:user) }
+  let(:project) { create(:project, :public, :repository) }
+  let(:note) { create(:note_on_merge_request, project: project, noteable: open_merge_request) }
+
+  let(:open_merge_request) do
+    create(:merge_request, :opened, source_project: project, author: user)
+  end
+
+  before do
+    assign(:project, project)
+    assign(:merge_request, open_merge_request)
+    assign(:note, note)
+    assign(:noteable, open_merge_request)
+    assign(:notes, [])
+    assign(:pipelines, Ci::Pipeline.none)
+    assign(:issuable_sidebar, serialize_issuable_sidebar(user, project, open_merge_request))
+
+    preload_view_requirements(open_merge_request, note)
+
+    sign_in(user)
+  end
+end
+
+RSpec.shared_context 'closed merge request show action' do
   include Devise::Test::ControllerHelpers
   include ProjectForksHelper
+  include Spec::Support::Helpers::Features::MergeRequestHelpers
 
   let(:user) { create(:user) }
   let(:project) { create(:project, :public, :repository) }
@@ -17,13 +44,6 @@ RSpec.shared_context 'merge request show action' do
            author: user)
   end
 
-  def preload_view_requirements
-    # This will load the status fields of the author of the note and merge request
-    # to avoid queries in when rendering the view being tested.
-    closed_merge_request.author.status
-    note.author.status
-  end
-
   before do
     assign(:project, project)
     assign(:merge_request, closed_merge_request)
@@ -34,16 +54,10 @@ RSpec.shared_context 'merge request show action' do
     assign(:pipelines, Ci::Pipeline.none)
     assign(:issuable_sidebar, serialize_issuable_sidebar(user, project, closed_merge_request))
 
-    preload_view_requirements
+    preload_view_requirements(closed_merge_request, note)
 
     allow(view).to receive_messages(current_user: user,
                                     can?: true,
                                     current_application_settings: Gitlab::CurrentSettings.current_application_settings)
   end
-
-  def serialize_issuable_sidebar(user, project, merge_request)
-    MergeRequestSerializer
-      .new(current_user: user, project: project)
-      .represent(closed_merge_request, serializer: 'sidebar')
-  end
 end

spec/views/projects/merge_requests/show.html.haml_spec.rb

@@ -3,30 +3,45 @@
 require 'spec_helper'
 
 RSpec.describe 'projects/merge_requests/show.html.haml' do
+  include Spec::Support::Helpers::Features::MergeRequestHelpers
+
   before do
     allow(view).to receive(:experiment_enabled?).and_return(false)
   end
 
-  include_context 'merge request show action'
-
-  describe 'merge request assignee sidebar' do
-    context 'when assignee is allowed to merge' do
-      it 'does not show a warning icon' do
-        closed_merge_request.update!(assignee_id: user.id)
-        project.add_maintainer(user)
-        assign(:issuable_sidebar, serialize_issuable_sidebar(user, project, closed_merge_request))
+  context 'when the merge request is open' do
+    include_context 'open merge request show action'
 
-        render
+    it 'shows the "Mark as draft" button' do
+      render
 
-        expect(rendered).not_to have_css('.merge-icon')
-      end
+      expect(rendered).to have_css('a', visible: true, text: 'Mark as draft')
+      expect(rendered).to have_css('a', visible: false, text: 'Reopen')
+      expect(rendered).to have_css('a', visible: true, text: 'Close')
     end
   end
 
   context 'when the merge request is closed' do
+    include_context 'closed merge request show action'
+
+    describe 'merge request assignee sidebar' do
+      context 'when assignee is allowed to merge' do
+        it 'does not show a warning icon' do
+          closed_merge_request.update!(assignee_id: user.id)
+          project.add_maintainer(user)
+          assign(:issuable_sidebar, serialize_issuable_sidebar(user, project, closed_merge_request))
+
+          render
+
+          expect(rendered).not_to have_css('.merge-icon')
+        end
+      end
+    end
+
     it 'shows the "Reopen" button' do
       render
 
+      expect(rendered).not_to have_css('a', visible: true, text: 'Mark as draft')
       expect(rendered).to have_css('a', visible: true, text: 'Reopen')
       expect(rendered).to have_css('a', visible: false, text: 'Close')
     end
@@ -34,7 +49,7 @@ RSpec.describe 'projects/merge_requests/show.html.haml' do
     it 'does not show the "Reopen" button when the source project does not exist' do
       unlink_project.execute
       closed_merge_request.reload
-      preload_view_requirements
+      preload_view_requirements(closed_merge_request, note)
 
       render