Commit 77f4d2af authored by Patrick Bajao's avatar Patrick Bajao

Revert "Merge branch...

Revert "Merge branch '198562-merge-request-user-interface-encourages-accidentally-closing-the-request' into 'master'"

This reverts commit 85df3315, reversing
changes made to 0b9a6dc1.
parent 67dc94f1
...@@ -180,9 +180,6 @@ export default { ...@@ -180,9 +180,6 @@ export default {
trackingLabel() { trackingLabel() {
return slugifyWithUnderscore(`${this.commentButtonTitle} button`); return slugifyWithUnderscore(`${this.commentButtonTitle} button`);
}, },
hasCloseAndCommentButton() {
return !this.glFeatures.removeCommentCloseReopen;
},
confidentialNotesEnabled() { confidentialNotesEnabled() {
return Boolean(this.glFeatures.confidentialNotes); return Boolean(this.glFeatures.confidentialNotes);
}, },
...@@ -426,7 +423,7 @@ export default { ...@@ -426,7 +423,7 @@ export default {
</gl-dropdown-item> </gl-dropdown-item>
</gl-dropdown> </gl-dropdown>
<gl-button <gl-button
v-if="hasCloseAndCommentButton && canToggleIssueState" v-if="canToggleIssueState"
:loading="isToggleStateButtonLoading" :loading="isToggleStateButtonLoading"
category="secondary" category="secondary"
:variant="buttonVariant" :variant="buttonVariant"
......
# frozen_string_literal: true
module CommentAndCloseFlag
extend ActiveSupport::Concern
included do
before_action do
push_frontend_feature_flag(:remove_comment_close_reopen, @group)
end
end
end
...@@ -9,7 +9,6 @@ class Projects::IssuesController < Projects::ApplicationController ...@@ -9,7 +9,6 @@ class Projects::IssuesController < Projects::ApplicationController
include IssuesCalendar include IssuesCalendar
include SpammableActions include SpammableActions
include RecordUserLastActivity include RecordUserLastActivity
include CommentAndCloseFlag
ISSUES_EXCEPT_ACTIONS = %i[index calendar new create bulk_update import_csv export_csv service_desk].freeze ISSUES_EXCEPT_ACTIONS = %i[index calendar new create bulk_update import_csv export_csv service_desk].freeze
SET_ISSUEABLES_INDEX_ONLY_ACTIONS = %i[index calendar service_desk].freeze SET_ISSUEABLES_INDEX_ONLY_ACTIONS = %i[index calendar service_desk].freeze
......
...@@ -11,7 +11,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -11,7 +11,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
include RecordUserLastActivity include RecordUserLastActivity
include SourcegraphDecorator include SourcegraphDecorator
include DiffHelper include DiffHelper
include CommentAndCloseFlag
skip_before_action :merge_request, only: [:index, :bulk_update, :export_csv] skip_before_action :merge_request, only: [:index, :bulk_update, :export_csv]
before_action :apply_diff_view_cookie!, only: [:show] before_action :apply_diff_view_cookie!, only: [:show]
......
---
name: remove_comment_close_reopen
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49614
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/198562
milestone: '13.9'
type: development
group: group::code review
default_enabled: false
...@@ -8,7 +8,6 @@ class Groups::EpicsController < Groups::ApplicationController ...@@ -8,7 +8,6 @@ class Groups::EpicsController < Groups::ApplicationController
include RendersNotes include RendersNotes
include EpicsActions include EpicsActions
include DescriptionDiffActions include DescriptionDiffActions
include CommentAndCloseFlag
before_action :check_epics_available! before_action :check_epics_available!
before_action :epic, except: [:index, :create, :new, :bulk_update] before_action :epic, except: [:index, :create, :new, :bulk_update]
......
...@@ -28,8 +28,6 @@ RSpec.describe 'Epic show', :js do ...@@ -28,8 +28,6 @@ RSpec.describe 'Epic show', :js do
let_it_be(:child_issue_a) { create(:epic_issue, epic: epic, issue: public_issue, relative_position: 1) } let_it_be(:child_issue_a) { create(:epic_issue, epic: epic, issue: public_issue, relative_position: 1) }
before do before do
stub_feature_flags(remove_comment_close_reopen: false)
group.add_developer(user) group.add_developer(user)
stub_licensed_features(epics: true, subepics: true) stub_licensed_features(epics: true, subepics: true)
sign_in(user) sign_in(user)
...@@ -294,15 +292,9 @@ RSpec.describe 'Epic show', :js do ...@@ -294,15 +292,9 @@ RSpec.describe 'Epic show', :js do
end end
describe 'when open' do describe 'when open' do
let(:open_epic) { create(:epic, group: group) }
it_behaves_like 'page with comment and close button', 'Close epic' do
def setup
visit group_epic_path(group, open_epic)
end
end
context 'when clicking the top `Close epic` button', :aggregate_failures do context 'when clicking the top `Close epic` button', :aggregate_failures do
let(:open_epic) { create(:epic, group: group) }
before do before do
visit group_epic_path(group, open_epic) visit group_epic_path(group, open_epic)
end end
...@@ -311,6 +303,8 @@ RSpec.describe 'Epic show', :js do ...@@ -311,6 +303,8 @@ RSpec.describe 'Epic show', :js do
end end
context 'when clicking the bottom `Close epic` button', :aggregate_failures do context 'when clicking the bottom `Close epic` button', :aggregate_failures do
let(:open_epic) { create(:epic, group: group) }
before do before do
visit group_epic_path(group, open_epic) visit group_epic_path(group, open_epic)
end end
...@@ -320,15 +314,9 @@ RSpec.describe 'Epic show', :js do ...@@ -320,15 +314,9 @@ RSpec.describe 'Epic show', :js do
end end
describe 'when closed' do describe 'when closed' do
let(:closed_epic) { create(:epic, group: group, state: 'closed') }
it_behaves_like 'page with comment and close button', 'Reopen epic' do
def setup
visit group_epic_path(group, closed_epic)
end
end
context 'when clicking the top `Reopen epic` button', :aggregate_failures do context 'when clicking the top `Reopen epic` button', :aggregate_failures do
let(:closed_epic) { create(:epic, group: group, state: 'closed') }
before do before do
visit group_epic_path(group, closed_epic) visit group_epic_path(group, closed_epic)
end end
...@@ -337,6 +325,8 @@ RSpec.describe 'Epic show', :js do ...@@ -337,6 +325,8 @@ RSpec.describe 'Epic show', :js do
end end
context 'when clicking the bottom `Reopen epic` button', :aggregate_failures do context 'when clicking the bottom `Reopen epic` button', :aggregate_failures do
let(:closed_epic) { create(:epic, group: group, state: 'closed') }
before do before do
visit group_epic_path(group, closed_epic) visit group_epic_path(group, closed_epic)
end end
......
...@@ -123,23 +123,11 @@ RSpec.describe 'Related issues', :js do ...@@ -123,23 +123,11 @@ RSpec.describe 'Related issues', :js do
expect(find('.js-related-issues-header-issue-count')).to have_content('1') expect(find('.js-related-issues-header-issue-count')).to have_content('1')
end end
it_behaves_like 'page with comment and close button', 'Close issue' do
def setup
visit project_issue_path(project, issue_a)
wait_for_requests
end
end
context 'when clicking the top `Close issue` button in the issue header', :aggregate_failures do context 'when clicking the top `Close issue` button in the issue header', :aggregate_failures do
it_behaves_like 'issue closed by modal', '.detail-page-header' it_behaves_like 'issue closed by modal', '.detail-page-header'
end end
context 'when clicking the bottom `Close issue` button below the comment textarea', :aggregate_failures do context 'when clicking the bottom `Close issue` button below the comment textarea', :aggregate_failures do
before do
stub_feature_flags(remove_comment_close_reopen: false)
end
it_behaves_like 'issue closed by modal', '.new-note' it_behaves_like 'issue closed by modal', '.new-note'
end end
end end
......
...@@ -20,12 +20,15 @@ RSpec.describe 'Merge request > User selects branches for new MR', :js do ...@@ -20,12 +20,15 @@ RSpec.describe 'Merge request > User selects branches for new MR', :js do
context 'create a merge request for the selected branches' do context 'create a merge request for the selected branches' do
before do before do
visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'feature_conflict' }) visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'feature_conflict' })
fill_in 'merge_request_title', with: 'A Test MR'
click_button 'Submit merge request'
end end
it 'shows the saved MR' do context 'saving the MR' do
expect(page).to have_content('A Test MR') it 'shows the saved MR' do
fill_in 'merge_request_title', with: 'Test'
click_button 'Submit merge request'
expect(page).to have_button('Close merge request')
end
end end
end end
end end
...@@ -8,8 +8,6 @@ RSpec.describe 'Thread Comments Issue', :js do ...@@ -8,8 +8,6 @@ RSpec.describe 'Thread Comments Issue', :js do
let(:issue) { create(:issue, project: project) } let(:issue) { create(:issue, project: project) }
before do before do
stub_feature_flags(remove_comment_close_reopen: false)
project.add_maintainer(user) project.add_maintainer(user)
sign_in(user) sign_in(user)
......
...@@ -9,7 +9,6 @@ RSpec.describe 'Thread Comments Merge Request', :js do ...@@ -9,7 +9,6 @@ RSpec.describe 'Thread Comments Merge Request', :js do
before do before do
stub_feature_flags(remove_resolve_note: false) stub_feature_flags(remove_resolve_note: false)
stub_feature_flags(remove_comment_close_reopen: false)
project.add_maintainer(user) project.add_maintainer(user)
sign_in(user) sign_in(user)
......
...@@ -42,15 +42,9 @@ RSpec.describe 'issue state', :js do ...@@ -42,15 +42,9 @@ RSpec.describe 'issue state', :js do
end end
describe 'when open', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/297348' do describe 'when open', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/297348' do
let(:open_issue) { create(:issue, project: project) }
it_behaves_like 'page with comment and close button', 'Close issue' do
def setup
visit project_issue_path(project, open_issue)
end
end
context 'when clicking the top `Close issue` button', :aggregate_failures do context 'when clicking the top `Close issue` button', :aggregate_failures do
let(:open_issue) { create(:issue, project: project) }
before do before do
visit project_issue_path(project, open_issue) visit project_issue_path(project, open_issue)
end end
...@@ -59,8 +53,9 @@ RSpec.describe 'issue state', :js do ...@@ -59,8 +53,9 @@ RSpec.describe 'issue state', :js do
end end
context 'when clicking the bottom `Close issue` button', :aggregate_failures do context 'when clicking the bottom `Close issue` button', :aggregate_failures do
let(:open_issue) { create(:issue, project: project) }
before do before do
stub_feature_flags(remove_comment_close_reopen: false)
visit project_issue_path(project, open_issue) visit project_issue_path(project, open_issue)
end end
...@@ -69,15 +64,9 @@ RSpec.describe 'issue state', :js do ...@@ -69,15 +64,9 @@ RSpec.describe 'issue state', :js do
end end
describe 'when closed', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/297201' do describe 'when closed', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/297201' do
let(:closed_issue) { create(:issue, project: project, state: 'closed') }
it_behaves_like 'page with comment and close button', 'Reopen issue' do
def setup
visit project_issue_path(project, closed_issue)
end
end
context 'when clicking the top `Reopen issue` button', :aggregate_failures do context 'when clicking the top `Reopen issue` button', :aggregate_failures do
let(:closed_issue) { create(:issue, project: project, state: 'closed') }
before do before do
visit project_issue_path(project, closed_issue) visit project_issue_path(project, closed_issue)
end end
...@@ -86,8 +75,9 @@ RSpec.describe 'issue state', :js do ...@@ -86,8 +75,9 @@ RSpec.describe 'issue state', :js do
end end
context 'when clicking the bottom `Reopen issue` button', :aggregate_failures do context 'when clicking the bottom `Reopen issue` button', :aggregate_failures do
let(:closed_issue) { create(:issue, project: project, state: 'closed') }
before do before do
stub_feature_flags(remove_comment_close_reopen: false)
visit project_issue_path(project, closed_issue) visit project_issue_path(project, closed_issue)
end end
......
...@@ -12,15 +12,9 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https:// ...@@ -12,15 +12,9 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://
end end
describe 'when open' do describe 'when open' do
let(:open_merge_request) { create(:merge_request, source_project: project, target_project: project) }
it_behaves_like 'page with comment and close button', 'Close merge request' do
def setup
visit merge_request_path(open_merge_request)
end
end
context 'when clicking the top `Close merge request` link', :aggregate_failures do context 'when clicking the top `Close merge request` link', :aggregate_failures do
let(:open_merge_request) { create(:merge_request, source_project: project, target_project: project) }
before do before do
visit merge_request_path(open_merge_request) visit merge_request_path(open_merge_request)
end end
...@@ -40,8 +34,9 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https:// ...@@ -40,8 +34,9 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://
end end
context 'when clicking the bottom `Close merge request` button', :aggregate_failures do context 'when clicking the bottom `Close merge request` button', :aggregate_failures do
let(:open_merge_request) { create(:merge_request, source_project: project, target_project: project) }
before do before do
stub_feature_flags(remove_comment_close_reopen: false)
visit merge_request_path(open_merge_request) visit merge_request_path(open_merge_request)
end end
...@@ -61,22 +56,9 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https:// ...@@ -61,22 +56,9 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://
end end
describe 'when closed' do describe 'when closed' do
let(:closed_merge_request) { create(:merge_request, source_project: project, target_project: project, state: 'closed') }
it_behaves_like 'page with comment and close button', 'Close merge request' do
def setup
visit merge_request_path(closed_merge_request)
within '.detail-page-header' do
click_button 'Toggle dropdown'
click_link 'Reopen merge request'
end
wait_for_requests
end
end
context 'when clicking the top `Reopen merge request` link', :aggregate_failures do context 'when clicking the top `Reopen merge request` link', :aggregate_failures do
let(:closed_merge_request) { create(:merge_request, source_project: project, target_project: project, state: 'closed') }
before do before do
visit merge_request_path(closed_merge_request) visit merge_request_path(closed_merge_request)
end end
...@@ -96,8 +78,9 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https:// ...@@ -96,8 +78,9 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://
end end
context 'when clicking the bottom `Reopen merge request` button', :aggregate_failures do context 'when clicking the bottom `Reopen merge request` button', :aggregate_failures do
let(:closed_merge_request) { create(:merge_request, source_project: project, target_project: project, state: 'closed') }
before do before do
stub_feature_flags(remove_comment_close_reopen: false)
visit merge_request_path(closed_merge_request) visit merge_request_path(closed_merge_request)
end end
......
...@@ -69,13 +69,7 @@ RSpec.describe 'Task Lists', :js do ...@@ -69,13 +69,7 @@ RSpec.describe 'Task Lists', :js do
wait_for_requests wait_for_requests
expect(page).to have_selector(".md .task-list .task-list-item .task-list-item-checkbox") expect(page).to have_selector(".md .task-list .task-list-item .task-list-item-checkbox")
end expect(page).to have_selector('.btn-close')
it_behaves_like 'page with comment and close button', 'Close issue' do
def setup
visit_issue(project, issue)
wait_for_requests
end
end end
it 'is only editable by author' do it 'is only editable by author' do
......
# frozen_string_literal: true
RSpec.shared_examples 'page with comment and close button' do |button_text|
context 'when remove_comment_close_reopen feature flag is enabled' do
before do
stub_feature_flags(remove_comment_close_reopen: true)
setup
end
it "does not show #{button_text} button" do
within '.note-form-actions' do
expect(page).not_to have_button(button_text)
end
end
end
context 'when remove_comment_close_reopen feature flag is disabled' do
before do
stub_feature_flags(remove_comment_close_reopen: false)
setup
end
it "shows #{button_text} button" do
within '.note-form-actions' do
expect(page).to have_button(button_text)
end
end
end
end
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