Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
1
Merge Requests
1
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
nexedi
gitlab-ce
Commits
2bf5e864
Commit
2bf5e864
authored
Feb 03, 2021
by
Marcel van Remmerden
Committed by
Rémy Coutable
Feb 03, 2021
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Remove comment and close button behind flag
Fixes problem of accidental clicks on "Comment & close/reopen" button
parent
0224e857
Changes
15
Hide whitespace changes
Inline
Side-by-side
Showing
15 changed files
with
142 additions
and
33 deletions
+142
-33
app/assets/javascripts/notes/components/comment_form.vue
app/assets/javascripts/notes/components/comment_form.vue
+4
-1
app/controllers/concerns/comment_and_close_flag.rb
app/controllers/concerns/comment_and_close_flag.rb
+11
-0
app/controllers/projects/issues_controller.rb
app/controllers/projects/issues_controller.rb
+1
-0
app/controllers/projects/merge_requests_controller.rb
app/controllers/projects/merge_requests_controller.rb
+1
-0
config/feature_flags/development/remove_comment_close_reopen.yml
...feature_flags/development/remove_comment_close_reopen.yml
+8
-0
ee/app/controllers/groups/epics_controller.rb
ee/app/controllers/groups/epics_controller.rb
+1
-0
ee/spec/features/epics/epic_show_spec.rb
ee/spec/features/epics/epic_show_spec.rb
+18
-8
ee/spec/features/issues/related_issues_spec.rb
ee/spec/features/issues/related_issues_spec.rb
+12
-0
ee/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb
...es/merge_request/user_selects_branches_for_new_mr_spec.rb
+4
-7
spec/features/discussion_comments/issue_spec.rb
spec/features/discussion_comments/issue_spec.rb
+2
-0
spec/features/discussion_comments/merge_request_spec.rb
spec/features/discussion_comments/merge_request_spec.rb
+1
-0
spec/features/issues/issue_state_spec.rb
spec/features/issues/issue_state_spec.rb
+18
-8
spec/features/merge_request/user_closes_reopens_merge_request_state_spec.rb
...e_request/user_closes_reopens_merge_request_state_spec.rb
+25
-8
spec/features/task_lists_spec.rb
spec/features/task_lists_spec.rb
+7
-1
spec/support/shared_examples/features/comment_and_close_button_shared_examples.rb
...ples/features/comment_and_close_button_shared_examples.rb
+29
-0
No files found.
app/assets/javascripts/notes/components/comment_form.vue
View file @
2bf5e864
...
@@ -143,6 +143,9 @@ export default {
...
@@ -143,6 +143,9 @@ export default {
trackingLabel
()
{
trackingLabel
()
{
return
slugifyWithUnderscore
(
`
${
this
.
commentButtonTitle
}
button`
);
return
slugifyWithUnderscore
(
`
${
this
.
commentButtonTitle
}
button`
);
},
},
hasCloseAndCommentButton
()
{
return
!
this
.
glFeatures
.
removeCommentCloseReopen
;
},
},
},
watch
:
{
watch
:
{
note
(
newNote
)
{
note
(
newNote
)
{
...
@@ -405,7 +408,7 @@ export default {
...
@@ -405,7 +408,7 @@ export default {
</div>
</div>
<gl-button
<gl-button
v-if=
"canToggleIssueState"
v-if=
"
hasCloseAndCommentButton &&
canToggleIssueState"
:loading=
"isToggleStateButtonLoading"
:loading=
"isToggleStateButtonLoading"
category=
"secondary"
category=
"secondary"
:variant=
"buttonVariant"
:variant=
"buttonVariant"
...
...
app/controllers/concerns/comment_and_close_flag.rb
0 → 100644
View file @
2bf5e864
# 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
app/controllers/projects/issues_controller.rb
View file @
2bf5e864
...
@@ -9,6 +9,7 @@ class Projects::IssuesController < Projects::ApplicationController
...
@@ -9,6 +9,7 @@ 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
...
...
app/controllers/projects/merge_requests_controller.rb
View file @
2bf5e864
...
@@ -11,6 +11,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
...
@@ -11,6 +11,7 @@ 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
]
...
...
config/feature_flags/development/remove_comment_close_reopen.yml
0 → 100644
View file @
2bf5e864
---
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
ee/app/controllers/groups/epics_controller.rb
View file @
2bf5e864
...
@@ -8,6 +8,7 @@ class Groups::EpicsController < Groups::ApplicationController
...
@@ -8,6 +8,7 @@ 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
]
...
...
ee/spec/features/epics/epic_show_spec.rb
View file @
2bf5e864
...
@@ -28,6 +28,8 @@ RSpec.describe 'Epic show', :js do
...
@@ -28,6 +28,8 @@ 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
)
...
@@ -292,9 +294,15 @@ RSpec.describe 'Epic show', :js do
...
@@ -292,9 +294,15 @@ RSpec.describe 'Epic show', :js do
end
end
describe
'when open'
do
describe
'when open'
do
context
'when clicking the top `Close epic` button'
,
:aggregate_failures
do
let
(
:open_epic
)
{
create
(
:epic
,
group:
group
)
}
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
before
do
before
do
visit
group_epic_path
(
group
,
open_epic
)
visit
group_epic_path
(
group
,
open_epic
)
end
end
...
@@ -303,8 +311,6 @@ RSpec.describe 'Epic show', :js do
...
@@ -303,8 +311,6 @@ 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
...
@@ -314,9 +320,15 @@ RSpec.describe 'Epic show', :js do
...
@@ -314,9 +320,15 @@ RSpec.describe 'Epic show', :js do
end
end
describe
'when closed'
do
describe
'when closed'
do
context
'when clicking the top `Reopen epic` button'
,
:aggregate_failures
do
let
(
:closed_epic
)
{
create
(
:epic
,
group:
group
,
state:
'closed'
)
}
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
before
do
before
do
visit
group_epic_path
(
group
,
closed_epic
)
visit
group_epic_path
(
group
,
closed_epic
)
end
end
...
@@ -325,8 +337,6 @@ RSpec.describe 'Epic show', :js do
...
@@ -325,8 +337,6 @@ 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
...
...
ee/spec/features/issues/related_issues_spec.rb
View file @
2bf5e864
...
@@ -123,11 +123,23 @@ RSpec.describe 'Related issues', :js do
...
@@ -123,11 +123,23 @@ 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
...
...
ee/spec/features/merge_request/user_selects_branches_for_new_mr_spec.rb
View file @
2bf5e864
...
@@ -20,15 +20,12 @@ RSpec.describe 'Merge request > User selects branches for new MR', :js do
...
@@ -20,15 +20,12 @@ 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
context
'saving the MR'
do
it
'shows the saved MR'
do
it
'shows the saved MR'
do
expect
(
page
).
to
have_content
(
'A Test MR'
)
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
spec/features/discussion_comments/issue_spec.rb
View file @
2bf5e864
...
@@ -8,6 +8,8 @@ RSpec.describe 'Thread Comments Issue', :js do
...
@@ -8,6 +8,8 @@ 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
)
...
...
spec/features/discussion_comments/merge_request_spec.rb
View file @
2bf5e864
...
@@ -9,6 +9,7 @@ RSpec.describe 'Thread Comments Merge Request', :js do
...
@@ -9,6 +9,7 @@ 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
)
...
...
spec/features/issues/issue_state_spec.rb
View file @
2bf5e864
...
@@ -42,9 +42,15 @@ RSpec.describe 'issue state', :js do
...
@@ -42,9 +42,15 @@ 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
context
'when clicking the top `Close issue` button'
,
:aggregate_failures
do
let
(
:open_issue
)
{
create
(
:issue
,
project:
project
)
}
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
before
do
before
do
visit
project_issue_path
(
project
,
open_issue
)
visit
project_issue_path
(
project
,
open_issue
)
end
end
...
@@ -53,9 +59,8 @@ RSpec.describe 'issue state', :js do
...
@@ -53,9 +59,8 @@ 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
...
@@ -64,9 +69,15 @@ RSpec.describe 'issue state', :js do
...
@@ -64,9 +69,15 @@ 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
context
'when clicking the top `Reopen issue` button'
,
:aggregate_failures
do
let
(
:closed_issue
)
{
create
(
:issue
,
project:
project
,
state:
'closed'
)
}
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
before
do
before
do
visit
project_issue_path
(
project
,
closed_issue
)
visit
project_issue_path
(
project
,
closed_issue
)
end
end
...
@@ -75,9 +86,8 @@ RSpec.describe 'issue state', :js do
...
@@ -75,9 +86,8 @@ 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
...
...
spec/features/merge_request/user_closes_reopens_merge_request_state_spec.rb
View file @
2bf5e864
...
@@ -12,9 +12,15 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://
...
@@ -12,9 +12,15 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://
end
end
describe
'when open'
do
describe
'when open'
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
)
}
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
before
do
before
do
visit
merge_request_path
(
open_merge_request
)
visit
merge_request_path
(
open_merge_request
)
end
end
...
@@ -34,9 +40,8 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://
...
@@ -34,9 +40,8 @@ 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
...
@@ -56,9 +61,22 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://
...
@@ -56,9 +61,22 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://
end
end
describe
'when closed'
do
describe
'when closed'
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'
)
}
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
before
do
before
do
visit
merge_request_path
(
closed_merge_request
)
visit
merge_request_path
(
closed_merge_request
)
end
end
...
@@ -78,9 +96,8 @@ RSpec.describe 'User closes/reopens a merge request', :js, quarantine: 'https://
...
@@ -78,9 +96,8 @@ 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
...
...
spec/features/task_lists_spec.rb
View file @
2bf5e864
...
@@ -69,7 +69,13 @@ RSpec.describe 'Task Lists', :js do
...
@@ -69,7 +69,13 @@ 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"
)
expect
(
page
).
to
have_selector
(
'.btn-close'
)
end
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
...
...
spec/support/shared_examples/features/comment_and_close_button_shared_examples.rb
0 → 100644
View file @
2bf5e864
# 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
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment