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
97b9241a
Commit
97b9241a
authored
Feb 10, 2021
by
Kerri Miller
Committed by
Alex Kalderimis
Feb 10, 2021
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Remove multiline_comments feature flag
Removes from backend and frontend code. Flag initially added in 13.3
parent
c65265b4
Changes
14
Hide whitespace changes
Inline
Side-by-side
Showing
14 changed files
with
50 additions
and
94 deletions
+50
-94
app/assets/javascripts/batch_comments/components/draft_note.vue
...sets/javascripts/batch_comments/components/draft_note.vue
+3
-5
app/assets/javascripts/batch_comments/components/preview_item.vue
...ts/javascripts/batch_comments/components/preview_item.vue
+7
-8
app/assets/javascripts/diffs/components/diff_line_note_form.vue
...sets/javascripts/diffs/components/diff_line_note_form.vue
+1
-1
app/assets/javascripts/notes/components/discussion_notes.vue
app/assets/javascripts/notes/components/discussion_notes.vue
+3
-5
app/assets/javascripts/notes/components/noteable_note.vue
app/assets/javascripts/notes/components/noteable_note.vue
+1
-3
app/controllers/projects/merge_requests_controller.rb
app/controllers/projects/merge_requests_controller.rb
+0
-1
config/feature_flags/development/multiline_comments.yml
config/feature_flags/development/multiline_comments.yml
+0
-8
doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md
...t/merge_requests/reviewing_and_managing_merge_requests.md
+1
-25
ee/spec/frontend/diffs/components/diff_line_note_form_spec.js
...pec/frontend/diffs/components/diff_line_note_form_spec.js
+1
-0
spec/frontend/batch_comments/components/draft_note_spec.js
spec/frontend/batch_comments/components/draft_note_spec.js
+8
-13
spec/frontend/batch_comments/components/preview_item_spec.js
spec/frontend/batch_comments/components/preview_item_spec.js
+16
-3
spec/frontend/diffs/components/diff_line_note_form_spec.js
spec/frontend/diffs/components/diff_line_note_form_spec.js
+1
-0
spec/frontend/notes/components/discussion_notes_spec.js
spec/frontend/notes/components/discussion_notes_spec.js
+8
-13
spec/frontend/notes/components/noteable_note_spec.js
spec/frontend/notes/components/noteable_note_spec.js
+0
-9
No files found.
app/assets/javascripts/batch_comments/components/draft_note.vue
View file @
97b9241a
...
...
@@ -3,7 +3,6 @@
import
{
mapActions
,
mapGetters
,
mapState
}
from
'
vuex
'
;
import
{
GlButton
}
from
'
@gitlab/ui
'
;
import
NoteableNote
from
'
~/notes/components/noteable_note.vue
'
;
import
glFeatureFlagsMixin
from
'
~/vue_shared/mixins/gl_feature_flags_mixin
'
;
import
PublishButton
from
'
./publish_button.vue
'
;
export
default
{
...
...
@@ -12,7 +11,6 @@ export default {
PublishButton
,
GlButton
,
},
mixins
:
[
glFeatureFlagsMixin
()],
props
:
{
draft
:
{
type
:
Object
,
...
...
@@ -63,14 +61,14 @@ export default {
this
.
isEditingDraft
=
false
;
},
handleMouseEnter
(
draft
)
{
if
(
this
.
glFeatures
.
multilineComments
&&
draft
.
position
)
{
if
(
draft
.
position
)
{
this
.
setSelectedCommentPositionHover
(
draft
.
position
.
line_range
);
}
},
handleMouseLeave
(
draft
)
{
// Even though position isn't used here we still don't want to unecessarily call a mutation
// Even though position isn't used here we still don't want to un
n
ecessarily call a mutation
// The lack of position tells us that highlighting is irrelevant in this context
if
(
this
.
glFeatures
.
multilineComments
&&
draft
.
position
)
{
if
(
draft
.
position
)
{
this
.
setSelectedCommentPositionHover
();
}
},
...
...
app/assets/javascripts/batch_comments/components/preview_item.vue
View file @
97b9241a
...
...
@@ -3,7 +3,6 @@ import { mapGetters } from 'vuex';
import
{
GlSprintf
,
GlIcon
}
from
'
@gitlab/ui
'
;
import
{
IMAGE_DIFF_POSITION_TYPE
}
from
'
~/diffs/constants
'
;
import
{
sprintf
,
__
}
from
'
~/locale
'
;
import
glFeatureFlagsMixin
from
'
~/vue_shared/mixins/gl_feature_flags_mixin
'
;
import
{
getStartLineNumber
,
getEndLineNumber
,
...
...
@@ -16,7 +15,7 @@ export default {
GlIcon
,
GlSprintf
,
},
mixins
:
[
resolvedStatusMixin
,
glFeatureFlagsMixin
()
],
mixins
:
[
resolvedStatusMixin
],
props
:
{
draft
:
{
type
:
Object
,
...
...
@@ -71,6 +70,10 @@ export default {
return
this
.
draft
.
position
||
this
.
discussion
.
position
;
},
startLineNumber
()
{
if
(
this
.
position
?.
position_type
===
IMAGE_DIFF_POSITION_TYPE
)
{
// eslint-disable-next-line @gitlab/require-i18n-strings
return
`
${
this
.
position
.
x
}
x
${
this
.
position
.
y
}
y`
;
}
return
getStartLineNumber
(
this
.
position
?.
line_range
);
},
endLineNumber
()
{
...
...
@@ -90,16 +93,12 @@ export default {
<span>
<span
class=
"review-preview-item-header"
>
<gl-icon
class=
"flex-shrink-0"
:name=
"iconName"
/>
<span
class=
"bold text-nowrap"
:class=
"
{ 'gl-align-items-center': glFeatures.multilineComments }"
>
<span
class=
"bold text-nowrap gl-align-items-center"
>
<span
class=
"review-preview-item-header-text block-truncated"
>
{{
titleText
}}
</span>
<template
v-if=
"showLinePosition"
>
<template
v-if=
"!glFeatures.multilineComments"
>
:
{{
linePosition
}}
</
template
>
<
template
v-else-if=
"startLineNumber === endLineNumber"
>
<template
v-if=
"startLineNumber === endLineNumber"
>
:
<span
:class=
"getLineClasses(startLineNumber)"
>
{{
startLineNumber
}}
</span>
</
template
>
<gl-sprintf
v-else
:message=
"__(':%{startLine} to %{endLine}')"
>
...
...
app/assets/javascripts/diffs/components/diff_line_note_form.vue
View file @
97b9241a
...
...
@@ -165,7 +165,7 @@ export default {
<
template
>
<div
class=
"content discussion-form discussion-form-container discussion-notes"
>
<div
v-if=
"glFeatures.multilineComments"
class=
"gl-mb-3 gl-text-gray-500 gl-pb-3"
>
<div
class=
"gl-mb-3 gl-text-gray-500 gl-pb-3"
>
<multiline-comment-form
v-model=
"commentLineStart"
:line=
"line"
...
...
app/assets/javascripts/notes/components/discussion_notes.vue
View file @
97b9241a
...
...
@@ -4,7 +4,6 @@ import { __ } from '~/locale';
import
PlaceholderNote
from
'
~/vue_shared/components/notes/placeholder_note.vue
'
;
import
PlaceholderSystemNote
from
'
~/vue_shared/components/notes/placeholder_system_note.vue
'
;
import
SystemNote
from
'
~/vue_shared/components/notes/system_note.vue
'
;
import
glFeatureFlagsMixin
from
'
~/vue_shared/mixins/gl_feature_flags_mixin
'
;
import
{
SYSTEM_NOTE
}
from
'
../constants
'
;
import
NoteableNote
from
'
./noteable_note.vue
'
;
import
ToggleRepliesWidget
from
'
./toggle_replies_widget.vue
'
;
...
...
@@ -18,7 +17,6 @@ export default {
NoteEditedText
,
DiscussionNotesRepliesWrapper
,
},
mixins
:
[
glFeatureFlagsMixin
()],
props
:
{
discussion
:
{
type
:
Object
,
...
...
@@ -96,14 +94,14 @@ export default {
return
note
.
isPlaceholderNote
?
note
.
notes
[
0
]
:
note
;
},
handleMouseEnter
(
discussion
)
{
if
(
this
.
glFeatures
.
multilineComments
&&
discussion
.
position
)
{
if
(
discussion
.
position
)
{
this
.
setSelectedCommentPositionHover
(
discussion
.
position
.
line_range
);
}
},
handleMouseLeave
(
discussion
)
{
// Even though position isn't used here we still don't want to unecessarily call a mutation
// Even though position isn't used here we still don't want to un
n
ecessarily call a mutation
// The lack of position tells us that highlighting is irrelevant in this context
if
(
this
.
glFeatures
.
multilineComments
&&
discussion
.
position
)
{
if
(
discussion
.
position
)
{
this
.
setSelectedCommentPositionHover
();
}
},
...
...
app/assets/javascripts/notes/components/noteable_note.vue
View file @
97b9241a
...
...
@@ -3,7 +3,6 @@ import $ from 'jquery';
import
{
mapGetters
,
mapActions
}
from
'
vuex
'
;
import
{
escape
}
from
'
lodash
'
;
import
{
GlSprintf
,
GlSafeHtmlDirective
as
SafeHtml
}
from
'
@gitlab/ui
'
;
import
glFeatureFlagsMixin
from
'
~/vue_shared/mixins/gl_feature_flags_mixin
'
;
import
{
truncateSha
}
from
'
~/lib/utils/text_utility
'
;
import
TimelineEntryItem
from
'
~/vue_shared/components/notes/timeline_entry_item.vue
'
;
import
httpStatusCodes
from
'
~/lib/utils/http_status
'
;
...
...
@@ -38,7 +37,7 @@ export default {
directives
:
{
SafeHtml
,
},
mixins
:
[
noteable
,
resolvable
,
glFeatureFlagsMixin
()
],
mixins
:
[
noteable
,
resolvable
],
props
:
{
note
:
{
type
:
Object
,
...
...
@@ -160,7 +159,6 @@ export default {
},
showMultiLineComment
()
{
if
(
!
this
.
glFeatures
.
multilineComments
||
!
this
.
discussionRoot
||
this
.
startLineNumber
.
length
===
0
||
this
.
endLineNumber
.
length
===
0
...
...
app/controllers/projects/merge_requests_controller.rb
View file @
97b9241a
...
...
@@ -30,7 +30,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
before_action
:authenticate_user!
,
only:
[
:assign_related_issues
]
before_action
:check_user_can_push_to_source_branch!
,
only:
[
:rebase
]
before_action
only:
[
:show
]
do
push_frontend_feature_flag
(
:multiline_comments
,
@project
,
default_enabled:
true
)
push_frontend_feature_flag
(
:file_identifier_hash
)
push_frontend_feature_flag
(
:batch_suggestions
,
@project
,
default_enabled:
true
)
push_frontend_feature_flag
(
:approvals_commented_by
,
@project
,
default_enabled:
true
)
...
...
config/feature_flags/development/multiline_comments.yml
deleted
100644 → 0
View file @
c65265b4
---
name
:
multiline_comments
introduced_by_url
:
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37114
rollout_issue_url
:
https://gitlab.com/gitlab-org/gitlab/-/issues/211255
milestone
:
'
13.2'
type
:
development
group
:
group::code review
default_enabled
:
true
doc/user/project/merge_requests/reviewing_and_managing_merge_requests.md
View file @
97b9241a
...
...
@@ -195,12 +195,7 @@ to expand the diff lines and leave a comment, just as you would for a changed li
### Commenting on multiple lines
> - [Introduced](https://gitlab.com/gitlab-org/ux-research/-/issues/870) in GitLab 13.2.
> - It's deployed behind a feature flag, enabled by default.
> - [Became enabled by default](https://gitlab.com/gitlab-org/gitlab/-/issues/221268) on GitLab 13.3.
> - It's enabled on GitLab.com.
> - It can be disabled or enabled per-project.
> - It's recommended for production use.
> - For GitLab self-managed instances, GitLab administrators can opt to [disable it](#enable-or-disable-multiline-comments). **(FREE SELF)**
> - [Feature flag removed](https://gitlab.com/gitlab-org/gitlab/-/issues/299121) in GitLab 13.9.
GitLab provides a way to select which lines of code a comment refers to. After starting a comment
a dropdown selector is shown to select the first line that this comment refers to.
...
...
@@ -216,25 +211,6 @@ above it.
![
Multiline comment selection displayed above comment
](
img/multiline-comment-saved.png
)
### Enable or disable multiline comments **(FREE SELF)**
The multiline comments feature is under development but ready for production use.
It is deployed behind a feature flag that is
**disabled by default**
.
[
GitLab administrators with access to the GitLab Rails console
](
../../../administration/feature_flags.md
)
can opt to enable it for your instance.
To disable it:
```
ruby
Feature
.
disable
(
:multiline_comments
)
```
To enable it:
```
ruby
Feature
.
enable
(
:multiline_comments
)
```
## Pipeline status in merge requests widgets
If you've set up
[
GitLab CI/CD
](
../../../ci/README.md
)
in your project,
...
...
ee/spec/frontend/diffs/components/diff_line_note_form_spec.js
View file @
97b9241a
...
...
@@ -38,6 +38,7 @@ describe('EE DiffLineNoteForm', () => {
diff_refs
:
{
head_sha
:
headSha
||
null
,
},
highlighted_diff_lines
:
[],
})),
},
},
...
...
spec/frontend/batch_comments/components/draft_note_spec.js
View file @
97b9241a
...
...
@@ -21,14 +21,11 @@ describe('Batch comments draft note component', () => {
const
getList
=
()
=>
getByRole
(
wrapper
.
element
,
'
list
'
);
const
createComponent
=
(
propsData
=
{
draft
}
,
features
=
{}
)
=>
{
const
createComponent
=
(
propsData
=
{
draft
})
=>
{
wrapper
=
shallowMount
(
localVue
.
extend
(
DraftNote
),
{
store
,
propsData
,
localVue
,
provide
:
{
glFeatures
:
{
multilineComments
:
true
,
...
features
},
},
});
jest
.
spyOn
(
wrapper
.
vm
.
$store
,
'
dispatch
'
).
mockImplementation
();
...
...
@@ -145,16 +142,14 @@ describe('Batch comments draft note component', () => {
describe
(
'
multiline comments
'
,
()
=>
{
describe
.
each
`
desc | props | features | event | expectedCalls
${
'
with `draft.position`
'
}
|
${
draftWithLineRange
}
|
${{}}
|
$
{
'
mouseenter
'
}
|
${[[
'
setSelectedCommentPositionHover
'
,
LINE_RANGE
]]}
${
'
with `draft.position`
'
}
|
${
draftWithLineRange
}
|
${{}}
|
$
{
'
mouseleave
'
}
|
${[[
'
setSelectedCommentPositionHover
'
]]}
${
'
with `draft.position`
'
}
|
${
draftWithLineRange
}
|
${{
multilineComments
:
false
}
} |
${
'
mouseenter
'
}
|
${[]}
${
'
with `draft.position`
'
}
|
${
draftWithLineRange
}
|
${{
multilineComments
:
false
}
} |
${
'
mouseleave
'
}
|
${[]}
${
'
without `draft.position`
'
}
|
${{}}
|
$
{{}}
|
$
{
'
mouseenter
'
}
|
${[]}
${
'
without `draft.position`
'
}
|
${{}}
|
$
{{}}
|
$
{
'
mouseleave
'
}
|
${[]}
`
(
'
$desc and features $features
'
,
({
props
,
event
,
features
,
expectedCalls
})
=>
{
desc | props | event | expectedCalls
${
'
with `draft.position`
'
}
|
${
draftWithLineRange
}
|
${
'
mouseenter
'
}
|
${[[
'
setSelectedCommentPositionHover
'
,
LINE_RANGE
]]}
${
'
with `draft.position`
'
}
|
${
draftWithLineRange
}
|
${
'
mouseleave
'
}
|
${[[
'
setSelectedCommentPositionHover
'
]]}
${
'
without `draft.position`
'
}
|
${{}}
|
$
{
'
mouseenter
'
}
|
${[]}
${
'
without `draft.position`
'
}
|
${{}}
|
$
{
'
mouseleave
'
}
|
${[]}
`
(
'
$desc
'
,
({
props
,
event
,
expectedCalls
})
=>
{
beforeEach
(()
=>
{
createComponent
({
draft
:
{
...
draft
,
...
props
}
}
,
features
);
createComponent
({
draft
:
{
...
draft
,
...
props
}
});
jest
.
spyOn
(
store
,
'
dispatch
'
);
});
...
...
spec/frontend/batch_comments/components/preview_item_spec.js
View file @
97b9241a
...
...
@@ -56,17 +56,30 @@ describe('Batch comments draft preview item component', () => {
createComponent
(
false
,
{
file_path
:
'
index.js
'
,
file_hash
:
'
abc
'
,
position
:
{
new_line
:
1
},
position
:
{
line_range
:
{
start
:
{
new_line
:
1
,
type
:
'
new
'
,
},
},
},
});
expect
(
vm
.
$el
.
querySelector
(
'
.bold
'
).
textContent
).
toContain
(
'
:1
'
);
expect
(
vm
.
$el
.
querySelector
(
'
.bold
'
).
textContent
).
toContain
(
'
:
+
1
'
);
});
it
(
'
renders old line position
'
,
()
=>
{
createComponent
(
false
,
{
file_path
:
'
index.js
'
,
file_hash
:
'
abc
'
,
position
:
{
old_line
:
2
},
position
:
{
line_range
:
{
start
:
{
old_line
:
2
,
},
},
},
});
expect
(
vm
.
$el
.
querySelector
(
'
.bold
'
).
textContent
).
toContain
(
'
:2
'
);
...
...
spec/frontend/diffs/components/diff_line_note_form_spec.js
View file @
97b9241a
...
...
@@ -17,6 +17,7 @@ describe('DiffLineNoteForm', () => {
const
store
=
createStore
();
store
.
state
.
notes
.
userData
.
id
=
1
;
store
.
state
.
notes
.
noteableData
=
noteableDataMock
;
store
.
state
.
diffs
.
diffFiles
=
[
diffFile
];
store
.
replaceState
({
...
store
.
state
,
...
args
.
state
});
...
...
spec/frontend/notes/components/discussion_notes_spec.js
View file @
97b9241a
...
...
@@ -23,7 +23,7 @@ describe('DiscussionNotes', () => {
let
wrapper
;
const
getList
=
()
=>
getByRole
(
wrapper
.
element
,
'
list
'
);
const
createComponent
=
(
props
,
features
=
{}
)
=>
{
const
createComponent
=
(
props
)
=>
{
wrapper
=
shallowMount
(
DiscussionNotes
,
{
store
,
propsData
:
{
...
...
@@ -38,9 +38,6 @@ describe('DiscussionNotes', () => {
slots
:
{
'
avatar-badge
'
:
'
<span class="avatar-badge-slot-content" />
'
,
},
provide
:
{
glFeatures
:
{
multilineComments
:
true
,
...
features
},
},
});
};
...
...
@@ -177,16 +174,14 @@ describe('DiscussionNotes', () => {
});
describe
.
each
`
desc | props | features | event | expectedCalls
${
'
with `discussion.position`
'
}
|
${{
discussion
:
DISCUSSION_WITH_LINE_RANGE
}
} |
${{}}
|
$
{
'
mouseenter
'
}
|
${[[
'
setSelectedCommentPositionHover
'
,
LINE_RANGE
]]}
${
'
with `discussion.position`
'
}
|
${{
discussion
:
DISCUSSION_WITH_LINE_RANGE
}
} |
${{}}
|
$
{
'
mouseleave
'
}
|
${[[
'
setSelectedCommentPositionHover
'
]]}
${
'
with `discussion.position`
'
}
|
${{
discussion
:
DISCUSSION_WITH_LINE_RANGE
}
} |
${{
multilineComments
:
false
}
} |
${
'
mouseenter
'
}
|
${[]}
${
'
with `discussion.position`
'
}
|
${{
discussion
:
DISCUSSION_WITH_LINE_RANGE
}
} |
${{
multilineComments
:
false
}
} |
${
'
mouseleave
'
}
|
${[]}
${
'
without `discussion.position`
'
}
|
${{}}
|
$
{{}}
|
$
{
'
mouseenter
'
}
|
${[]}
${
'
without `discussion.position`
'
}
|
${{}}
|
$
{{}}
|
$
{
'
mouseleave
'
}
|
${[]}
`
(
'
$desc and features $features
'
,
({
props
,
event
,
features
,
expectedCalls
})
=>
{
desc | props | event | expectedCalls
${
'
with `discussion.position`
'
}
|
${{
discussion
:
DISCUSSION_WITH_LINE_RANGE
}
} |
${
'
mouseenter
'
}
|
${[[
'
setSelectedCommentPositionHover
'
,
LINE_RANGE
]]}
${
'
with `discussion.position`
'
}
|
${{
discussion
:
DISCUSSION_WITH_LINE_RANGE
}
} |
${
'
mouseleave
'
}
|
${[[
'
setSelectedCommentPositionHover
'
]]}
${
'
without `discussion.position`
'
}
|
${{}}
|
$
{
'
mouseenter
'
}
|
${[]}
${
'
without `discussion.position`
'
}
|
${{}}
|
$
{
'
mouseleave
'
}
|
${[]}
`
(
'
$desc
'
,
({
props
,
event
,
expectedCalls
})
=>
{
beforeEach
(()
=>
{
createComponent
(
props
,
features
);
createComponent
(
props
);
jest
.
spyOn
(
store
,
'
dispatch
'
);
});
...
...
spec/frontend/notes/components/noteable_note_spec.js
View file @
97b9241a
...
...
@@ -8,15 +8,6 @@ import NoteActions from '~/notes/components/note_actions.vue';
import
NoteBody
from
'
~/notes/components/note_body.vue
'
;
import
{
noteableDataMock
,
notesDataMock
,
note
}
from
'
../mock_data
'
;
jest
.
mock
(
'
~/vue_shared/mixins/gl_feature_flags_mixin
'
,
()
=>
()
=>
({
inject
:
{
glFeatures
:
{
from
:
'
glFeatures
'
,
default
:
()
=>
({
multilineComments
:
true
}),
},
},
}));
describe
(
'
issue_note
'
,
()
=>
{
let
store
;
let
wrapper
;
...
...
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