Commit b58f74f0 authored by Takuya Noguchi's avatar Takuya Noguchi Committed by Sean McGivern

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

Adds and refactors the rspec code around static frontend tests.
Signed-off-by: default avatarTakuya Noguchi <takninnovationresearch@gmail.com>
parent 3f577ec0
...@@ -32,8 +32,7 @@ ...@@ -32,8 +32,7 @@
%ul %ul
- if can_update_merge_request - if can_update_merge_request
%li= link_to 'Edit', edit_namespace_project_merge_request_path(@project.namespace, @project, @merge_request) %li= link_to 'Edit', edit_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
- if can_update_merge_request - if @merge_request.opened?
- unless @merge_request.closed?
%li %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_issuable_path(@merge_request), method: :put, class: "js-draft-toggle-button"
%li{ class: [merge_request_button_visibility(@merge_request, true), 'js-close-item'] } %li{ class: [merge_request_button_visibility(@merge_request, true), 'js-close-item'] }
......
---
title: Hide Mark as draft button in a merged MR even on mobile
merge_request: 47678
author: Takuya Noguchi
type: fixed
...@@ -7,7 +7,7 @@ RSpec.describe 'projects/merge_requests/show.html.haml' do ...@@ -7,7 +7,7 @@ RSpec.describe 'projects/merge_requests/show.html.haml' do
allow(view).to receive(:experiment_enabled?).and_return(false) allow(view).to receive(:experiment_enabled?).and_return(false)
end 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 context 'when merge request is created by a GitLab team member' do
let(:user) { create(:user) } let(:user) { create(:user) }
......
...@@ -121,6 +121,18 @@ RSpec.describe 'Issuables Close/Reopen/Report toggle' do ...@@ -121,6 +121,18 @@ RSpec.describe 'Issuables Close/Reopen/Report toggle' do
it_behaves_like 'an issuable close/reopen/report toggle' 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 context 'when the merge request is closed' do
let(:issuable) { create(:merge_request, :closed, source_project: project) } let(:issuable) { create(:merge_request, :closed, source_project: project) }
......
# 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
# frozen_string_literal: true # 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 Devise::Test::ControllerHelpers
include ProjectForksHelper include ProjectForksHelper
include Spec::Support::Helpers::Features::MergeRequestHelpers
let(:user) { create(:user) } let(:user) { create(:user) }
let(:project) { create(:project, :public, :repository) } let(:project) { create(:project, :public, :repository) }
...@@ -17,13 +44,6 @@ RSpec.shared_context 'merge request show action' do ...@@ -17,13 +44,6 @@ RSpec.shared_context 'merge request show action' do
author: user) author: user)
end 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 before do
assign(:project, project) assign(:project, project)
assign(:merge_request, closed_merge_request) assign(:merge_request, closed_merge_request)
...@@ -34,16 +54,10 @@ RSpec.shared_context 'merge request show action' do ...@@ -34,16 +54,10 @@ RSpec.shared_context 'merge request show action' do
assign(:pipelines, Ci::Pipeline.none) assign(:pipelines, Ci::Pipeline.none)
assign(:issuable_sidebar, serialize_issuable_sidebar(user, project, closed_merge_request)) 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, allow(view).to receive_messages(current_user: user,
can?: true, can?: true,
current_application_settings: Gitlab::CurrentSettings.current_application_settings) current_application_settings: Gitlab::CurrentSettings.current_application_settings)
end end
def serialize_issuable_sidebar(user, project, merge_request)
MergeRequestSerializer
.new(current_user: user, project: project)
.represent(closed_merge_request, serializer: 'sidebar')
end
end end
...@@ -3,30 +3,45 @@ ...@@ -3,30 +3,45 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe 'projects/merge_requests/show.html.haml' do RSpec.describe 'projects/merge_requests/show.html.haml' do
include Spec::Support::Helpers::Features::MergeRequestHelpers
before do before do
allow(view).to receive(:experiment_enabled?).and_return(false) allow(view).to receive(:experiment_enabled?).and_return(false)
end end
include_context 'merge request show action' context 'when the merge request is open' do
include_context 'open 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 it 'shows the "Mark as draft" button' do
render
expect(rendered).not_to have_css('.merge-icon') expect(rendered).to have_css('a', visible: true, text: 'Mark as draft')
end expect(rendered).to have_css('a', visible: false, text: 'Reopen')
expect(rendered).to have_css('a', visible: true, text: 'Close')
end end
end end
context 'when the merge request is closed' do 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 it 'shows the "Reopen" button' do
render 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: true, text: 'Reopen')
expect(rendered).to have_css('a', visible: false, text: 'Close') expect(rendered).to have_css('a', visible: false, text: 'Close')
end end
...@@ -34,7 +49,7 @@ RSpec.describe 'projects/merge_requests/show.html.haml' do ...@@ -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 it 'does not show the "Reopen" button when the source project does not exist' do
unlink_project.execute unlink_project.execute
closed_merge_request.reload closed_merge_request.reload
preload_view_requirements preload_view_requirements(closed_merge_request, note)
render render
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment