Commit 41d6b992 authored by Phil Hughes's avatar Phil Hughes

Merge branch 'jdb/mutliline-comment-fe' into 'master'

Multiline comments frontend MVC inline only

See merge request gitlab-org/gitlab!29516
parents bf241b1d 088b0f25
...@@ -15,6 +15,16 @@ export default { ...@@ -15,6 +15,16 @@ export default {
type: Object, type: Object,
required: true, required: true,
}, },
diffFile: {
type: Object,
required: false,
default: () => ({}),
},
line: {
type: Object,
required: false,
default: null,
},
}, },
data() { data() {
return { return {
...@@ -61,6 +71,8 @@ export default { ...@@ -61,6 +71,8 @@ export default {
<ul class="notes draft-notes"> <ul class="notes draft-notes">
<noteable-note <noteable-note
:note="draft" :note="draft"
:diff-lines="diffFile.highlighted_diff_lines"
:line="line"
class="draft-note" class="draft-note"
@handleEdit="handleEditing" @handleEdit="handleEditing"
@cancelForm="handleNotEditing" @cancelForm="handleNotEditing"
......
...@@ -10,6 +10,15 @@ export default { ...@@ -10,6 +10,15 @@ export default {
type: Object, type: Object,
required: true, required: true,
}, },
diffFile: {
type: Object,
required: true,
},
line: {
type: Object,
required: false,
default: null,
},
}, },
}; };
</script> </script>
...@@ -17,7 +26,7 @@ export default { ...@@ -17,7 +26,7 @@ export default {
<template> <template>
<tr class="notes_holder js-temp-notes-holder"> <tr class="notes_holder js-temp-notes-holder">
<td class="notes-content" colspan="4"> <td class="notes-content" colspan="4">
<div class="content"><draft-note :draft="draft" /></div> <div class="content"><draft-note :draft="draft" :diff-file="diffFile" :line="line" /></div>
</td> </td>
</tr> </tr>
</template> </template>
...@@ -4,12 +4,20 @@ import { IMAGE_DIFF_POSITION_TYPE } from '~/diffs/constants'; ...@@ -4,12 +4,20 @@ import { IMAGE_DIFF_POSITION_TYPE } from '~/diffs/constants';
import { sprintf, __ } from '~/locale'; import { sprintf, __ } from '~/locale';
import Icon from '~/vue_shared/components/icon.vue'; import Icon from '~/vue_shared/components/icon.vue';
import resolvedStatusMixin from '../mixins/resolved_status'; import resolvedStatusMixin from '../mixins/resolved_status';
import { GlSprintf } from '@gitlab/ui';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import {
getStartLineNumber,
getEndLineNumber,
getLineClasses,
} from '~/notes/components/multiline_comment_utils';
export default { export default {
components: { components: {
Icon, Icon,
GlSprintf,
}, },
mixins: [resolvedStatusMixin], mixins: [resolvedStatusMixin, glFeatureFlagsMixin()],
props: { props: {
draft: { draft: {
type: Object, type: Object,
...@@ -51,7 +59,7 @@ export default { ...@@ -51,7 +59,7 @@ export default {
const position = this.discussion ? this.discussion.position : this.draft.position; const position = this.discussion ? this.discussion.position : this.draft.position;
return position.new_line || position.old_line; return position?.new_line || position?.old_line;
}, },
content() { content() {
const el = document.createElement('div'); const el = document.createElement('div');
...@@ -62,9 +70,18 @@ export default { ...@@ -62,9 +70,18 @@ export default {
showLinePosition() { showLinePosition() {
return this.draft.file_hash || this.isDiffDiscussion; return this.draft.file_hash || this.isDiffDiscussion;
}, },
startLineNumber() {
return getStartLineNumber(this.draft.position?.line_range);
},
endLineNumber() {
return getEndLineNumber(this.draft.position?.line_range);
},
}, },
methods: { methods: {
...mapActions('batchComments', ['scrollToDraft']), ...mapActions('batchComments', ['scrollToDraft']),
getLineClasses(lineNumber) {
return getLineClasses(lineNumber);
},
}, },
showStaysResolved: false, showStaysResolved: false,
}; };
...@@ -83,11 +100,33 @@ export default { ...@@ -83,11 +100,33 @@ export default {
@click="scrollToDraft(draft)" @click="scrollToDraft(draft)"
> >
<span class="review-preview-item-header"> <span class="review-preview-item-header">
<icon class="gl-mr-3 flex-shrink-0" :name="iconName" /> <icon class="flex-shrink-0" :name="iconName" />
<span class="bold text-nowrap"> <span
<span class="review-preview-item-header-text block-truncated"> {{ titleText }} </span> class="bold text-nowrap"
:class="{ 'gl-align-items-center': glFeatures.multilineComments }"
>
<span class="review-preview-item-header-text block-truncated">
{{ titleText }}
</span>
<template v-if="showLinePosition"> <template v-if="showLinePosition">
:{{ linePosition }} <template v-if="!glFeatures.multilineComments"
>:{{ linePosition }}</template
>
<template v-else-if="startLineNumber === endLineNumber">
:<span :class="getLineClasses(startLineNumber)">{{ startLineNumber }}</span>
</template>
<gl-sprintf v-else :message="__(':%{startLine} to %{endLine}')">
<template #startLine>
<span class="gl-mr-2" :class="getLineClasses(startLineNumber)">{{
startLineNumber
}}</span>
</template>
<template #endLine>
<span class="gl-ml-2" :class="getLineClasses(endLineNumber)">{{
endLineNumber
}}</span>
</template>
</gl-sprintf>
</template> </template>
</span> </span>
</span> </span>
......
...@@ -25,9 +25,9 @@ export default { ...@@ -25,9 +25,9 @@ export default {
discard(endpoint) { discard(endpoint) {
return axios.delete(endpoint); return axios.delete(endpoint);
}, },
update(endpoint, { draftId, note, resolveDiscussion }) { update(endpoint, { draftId, note, resolveDiscussion, position }) {
return axios.put(`${endpoint}/${draftId}`, { return axios.put(`${endpoint}/${draftId}`, {
draft_note: { note, resolve_discussion: resolveDiscussion }, draft_note: { note, resolve_discussion: resolveDiscussion, position },
}); });
}, },
}; };
...@@ -84,12 +84,16 @@ export const discardReview = ({ commit, getters }) => { ...@@ -84,12 +84,16 @@ export const discardReview = ({ commit, getters }) => {
.catch(() => commit(types.RECEIVE_DISCARD_REVIEW_ERROR)); .catch(() => commit(types.RECEIVE_DISCARD_REVIEW_ERROR));
}; };
export const updateDraft = ({ commit, getters }, { note, noteText, resolveDiscussion, callback }) => export const updateDraft = (
{ commit, getters },
{ note, noteText, resolveDiscussion, position, callback },
) =>
service service
.update(getters.getNotesData.draftsPath, { .update(getters.getNotesData.draftsPath, {
draftId: note.id, draftId: note.id,
note: noteText, note: noteText,
resolveDiscussion, resolveDiscussion,
position: JSON.stringify(position),
}) })
.then(res => res.data) .then(res => res.data)
.then(data => commit(types.RECEIVE_DRAFT_UPDATE_SUCCESS, data)) .then(data => commit(types.RECEIVE_DRAFT_UPDATE_SUCCESS, data))
......
<script> <script>
import { mapState, mapGetters, mapActions } from 'vuex'; import { mapState, mapGetters, mapActions } from 'vuex';
import diffLineNoteFormMixin from '~/notes/mixins/diff_line_note_form'; import diffLineNoteFormMixin from '~/notes/mixins/diff_line_note_form';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { s__ } from '~/locale'; import { s__ } from '~/locale';
import noteForm from '../../notes/components/note_form.vue'; import noteForm from '../../notes/components/note_form.vue';
import MultilineCommentForm from '../../notes/components/multiline_comment_form.vue';
import autosave from '../../notes/mixins/autosave'; import autosave from '../../notes/mixins/autosave';
import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue';
import { DIFF_NOTE_TYPE } from '../constants'; import { DIFF_NOTE_TYPE } from '../constants';
import { commentLineOptions } from '../../notes/components/multiline_comment_utils';
export default { export default {
components: { components: {
noteForm, noteForm,
userAvatarLink, userAvatarLink,
MultilineCommentForm,
}, },
mixins: [autosave, diffLineNoteFormMixin], mixins: [autosave, diffLineNoteFormMixin, glFeatureFlagsMixin()],
props: { props: {
diffFileHash: { diffFileHash: {
type: String, type: String,
...@@ -37,6 +41,14 @@ export default { ...@@ -37,6 +41,14 @@ export default {
default: '', default: '',
}, },
}, },
data() {
return {
commentLineStart: {
lineCode: this.line.line_code,
type: this.line.type,
},
};
},
computed: { computed: {
...mapState({ ...mapState({
noteableData: state => state.notes.noteableData, noteableData: state => state.notes.noteableData,
...@@ -62,11 +74,20 @@ export default { ...@@ -62,11 +74,20 @@ export default {
diffViewType: this.diffViewType, diffViewType: this.diffViewType,
diffFile: this.diffFile, diffFile: this.diffFile,
linePosition: this.linePosition, linePosition: this.linePosition,
lineRange: {
start_line_code: this.commentLineStart.lineCode,
start_line_type: this.commentLineStart.type,
end_line_code: this.line.line_code,
end_line_type: this.line.type,
},
}; };
}, },
diffFile() { diffFile() {
return this.getDiffFileByHash(this.diffFileHash); return this.getDiffFileByHash(this.diffFileHash);
}, },
commentLineOptions() {
return commentLineOptions(this.diffFile.highlighted_diff_lines, this.line.line_code);
},
}, },
mounted() { mounted() {
if (this.isLoggedIn) { if (this.isLoggedIn) {
...@@ -83,7 +104,6 @@ export default { ...@@ -83,7 +104,6 @@ export default {
methods: { methods: {
...mapActions('diffs', [ ...mapActions('diffs', [
'cancelCommentForm', 'cancelCommentForm',
'assignDiscussionsToDiff',
'saveDiffDiscussion', 'saveDiffDiscussion',
'setSuggestPopoverDismissed', 'setSuggestPopoverDismissed',
]), ]),
...@@ -116,6 +136,16 @@ export default { ...@@ -116,6 +136,16 @@ export default {
<template> <template>
<div class="content discussion-form discussion-form-container discussion-notes"> <div class="content discussion-form discussion-form-container discussion-notes">
<div
v-if="glFeatures.multilineComments"
class="gl-mb-3 gl-text-gray-700 gl-border-gray-200 gl-border-b-solid gl-border-b-1 gl-pb-3"
>
<multiline-comment-form
v-model="commentLineStart"
:line="line"
:comment-line-options="commentLineOptions"
/>
</div>
<user-avatar-link <user-avatar-link
v-if="author" v-if="author"
:link-href="author.path" :link-href="author.path"
...@@ -133,7 +163,7 @@ export default { ...@@ -133,7 +163,7 @@ export default {
:diff-file="diffFile" :diff-file="diffFile"
:show-suggest-popover="showSuggestPopover" :show-suggest-popover="showSuggestPopover"
save-button-title="Comment" save-button-title="Comment"
class="diff-comment-form" class="diff-comment-form prepend-top-10"
@handleFormUpdateAddToReview="addToReview" @handleFormUpdateAddToReview="addToReview"
@cancelForm="handleCancelCommentForm" @cancelForm="handleCancelCommentForm"
@handleFormUpdate="handleSaveNote" @handleFormUpdate="handleSaveNote"
......
...@@ -80,6 +80,8 @@ export default { ...@@ -80,6 +80,8 @@ export default {
v-if="shouldRenderDraftRow(diffFile.file_hash, line)" v-if="shouldRenderDraftRow(diffFile.file_hash, line)"
:key="`draft_${index}`" :key="`draft_${index}`"
:draft="draftForLine(diffFile.file_hash, line)" :draft="draftForLine(diffFile.file_hash, line)"
:diff-file="diffFile"
:line="line"
/> />
</template> </template>
</tbody> </tbody>
......
...@@ -40,6 +40,7 @@ export function getFormData(params) { ...@@ -40,6 +40,7 @@ export function getFormData(params) {
diffViewType, diffViewType,
linePosition, linePosition,
positionType, positionType,
lineRange,
} = params; } = params;
const position = JSON.stringify({ const position = JSON.stringify({
...@@ -55,6 +56,7 @@ export function getFormData(params) { ...@@ -55,6 +56,7 @@ export function getFormData(params) {
y: params.y, y: params.y,
width: params.width, width: params.width,
height: params.height, height: params.height,
line_range: lineRange,
}); });
const postData = { const postData = {
......
<script>
import { GlFormSelect, GlSprintf } from '@gitlab/ui';
import { getSymbol, getLineClasses } from './multiline_comment_utils';
export default {
components: { GlFormSelect, GlSprintf },
props: {
lineRange: {
type: Object,
required: false,
default: null,
},
line: {
type: Object,
required: true,
},
commentLineOptions: {
type: Array,
required: true,
},
},
data() {
return {
commentLineStart: {
lineCode: this.lineRange ? this.lineRange.start_line_code : this.line.line_code,
type: this.lineRange ? this.lineRange.start_line_type : this.line.type,
},
};
},
methods: {
getSymbol({ type }) {
return getSymbol(type);
},
getLineClasses(line) {
return getLineClasses(line);
},
},
};
</script>
<template>
<div>
<gl-sprintf
:message="
s__('MergeRequestDiffs|Commenting on lines %{selectStart}start%{selectEnd} to %{end}')
"
>
<template #select>
<label for="comment-line-start" class="sr-only">{{
s__('MergeRequestDiffs|Select comment starting line')
}}</label>
<gl-form-select
id="comment-line-start"
:value="commentLineStart"
:options="commentLineOptions"
size="sm"
class="gl-w-auto gl-vertical-align-baseline"
@change="$emit('input', $event)"
/>
</template>
<template #end>
<span :class="getLineClasses(line)">
{{ getSymbol(line) + (line.new_line || line.old_line) }}
</span>
</template>
</gl-sprintf>
</div>
</template>
import { takeRightWhile } from 'lodash';
export function getSymbol(type) {
if (type === 'new') return '+';
if (type === 'old') return '-';
return '';
}
function getLineNumber(lineRange, key) {
if (!lineRange || !key) return '';
const lineCode = lineRange[`${key}_line_code`] || '';
const lineType = lineRange[`${key}_line_type`] || '';
const lines = lineCode.split('_') || [];
const lineNumber = lineType === 'old' ? lines[1] : lines[2];
return (lineNumber && getSymbol(lineType) + lineNumber) || '';
}
export function getStartLineNumber(lineRange) {
return getLineNumber(lineRange, 'start');
}
export function getEndLineNumber(lineRange) {
return getLineNumber(lineRange, 'end');
}
export function getLineClasses(line) {
const symbol = typeof line === 'string' ? line.charAt(0) : getSymbol(line?.type);
if (symbol !== '+' && symbol !== '-') return '';
return [
'gl-px-1 gl-rounded-small gl-border-solid gl-border-1 gl-border-white',
{
'gl-bg-green-100 gl-text-green-800': symbol === '+',
'gl-bg-red-100 gl-text-red-800': symbol === '-',
},
];
}
export function commentLineOptions(diffLines, lineCode) {
const selectedIndex = diffLines.findIndex(line => line.line_code === lineCode);
const notMatchType = l => l.type !== 'match';
// We're limiting adding comments to only lines above the current line
// to make rendering simpler. Future interations will use a more
// intuitive dragging interface that will make this unnecessary
const upToSelected = diffLines.slice(0, selectedIndex + 1);
// Only include the lines up to the first "Show unchanged lines" block
// i.e. not a "match" type
const lines = takeRightWhile(upToSelected, notMatchType);
return lines.map(l => ({
value: { lineCode: l.line_code, type: l.type },
text: `${getSymbol(l.type)}${l.new_line || l.old_line}`,
}));
}
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
import $ from 'jquery'; import $ from 'jquery';
import { mapGetters, mapActions } from 'vuex'; import { mapGetters, mapActions } from 'vuex';
import { escape } from 'lodash'; import { escape } from 'lodash';
import { GlSprintf } from '@gitlab/ui';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import { truncateSha } from '~/lib/utils/text_utility'; import { truncateSha } from '~/lib/utils/text_utility';
import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue'; import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue';
import { __, s__, sprintf } from '../../locale'; import { __, s__, sprintf } from '../../locale';
...@@ -14,17 +16,26 @@ import eventHub from '../event_hub'; ...@@ -14,17 +16,26 @@ import eventHub from '../event_hub';
import noteable from '../mixins/noteable'; import noteable from '../mixins/noteable';
import resolvable from '../mixins/resolvable'; import resolvable from '../mixins/resolvable';
import httpStatusCodes from '~/lib/utils/http_status'; import httpStatusCodes from '~/lib/utils/http_status';
import {
getStartLineNumber,
getEndLineNumber,
getLineClasses,
commentLineOptions,
} from './multiline_comment_utils';
import MultilineCommentForm from './multiline_comment_form.vue';
export default { export default {
name: 'NoteableNote', name: 'NoteableNote',
components: { components: {
GlSprintf,
userAvatarLink, userAvatarLink,
noteHeader, noteHeader,
noteActions, noteActions,
NoteBody, NoteBody,
TimelineEntryItem, TimelineEntryItem,
MultilineCommentForm,
}, },
mixins: [noteable, resolvable], mixins: [noteable, resolvable, glFeatureFlagsMixin()],
props: { props: {
note: { note: {
type: Object, type: Object,
...@@ -50,6 +61,11 @@ export default { ...@@ -50,6 +61,11 @@ export default {
required: false, required: false,
default: false, default: false,
}, },
diffLines: {
type: Object,
required: false,
default: null,
},
}, },
data() { data() {
return { return {
...@@ -57,9 +73,14 @@ export default { ...@@ -57,9 +73,14 @@ export default {
isDeleting: false, isDeleting: false,
isRequesting: false, isRequesting: false,
isResolving: false, isResolving: false,
commentLineStart: {
line_code: this.line?.line_code,
type: this.line?.type,
},
}; };
}, },
computed: { computed: {
...mapGetters('diffs', ['getDiffFileByHash']),
...mapGetters(['targetNoteHash', 'getNoteableData', 'getUserData', 'commentsDisabled']), ...mapGetters(['targetNoteHash', 'getNoteableData', 'getUserData', 'commentsDisabled']),
author() { author() {
return this.note.author; return this.note.author;
...@@ -113,6 +134,32 @@ export default { ...@@ -113,6 +134,32 @@ export default {
(this.note.isDraft && this.note.discussion_id !== null) (this.note.isDraft && this.note.discussion_id !== null)
); );
}, },
lineRange() {
return this.note.position?.line_range;
},
startLineNumber() {
return getStartLineNumber(this.lineRange);
},
endLineNumber() {
return getEndLineNumber(this.lineRange);
},
showMultiLineComment() {
return (
this.glFeatures.multilineComments &&
this.startLineNumber &&
this.endLineNumber &&
(this.startLineNumber !== this.endLineNumber || this.isEditing)
);
},
commentLineOptions() {
if (this.diffLines) {
return commentLineOptions(this.diffLines, this.line.line_code);
}
const diffFile = this.diffFile || this.getDiffFileByHash(this.targetNoteHash);
if (!diffFile) return null;
return commentLineOptions(diffFile.highlighted_diff_lines, this.line.line_code);
},
}, },
created() { created() {
...@@ -174,10 +221,20 @@ export default { ...@@ -174,10 +221,20 @@ export default {
this.$emit('updateSuccess'); this.$emit('updateSuccess');
}, },
formUpdateHandler(noteText, parentElement, callback, resolveDiscussion) { formUpdateHandler(noteText, parentElement, callback, resolveDiscussion) {
const position = {
...this.note.position,
line_range: {
start_line_code: this.commentLineStart?.lineCode,
start_line_type: this.commentLineStart?.type,
end_line_code: this.line?.line_code,
end_line_type: this.line?.type,
},
};
this.$emit('handleUpdateNote', { this.$emit('handleUpdateNote', {
note: this.note, note: this.note,
noteText, noteText,
resolveDiscussion, resolveDiscussion,
position,
callback: () => this.updateSuccess(), callback: () => this.updateSuccess(),
}); });
...@@ -239,6 +296,9 @@ export default { ...@@ -239,6 +296,9 @@ export default {
noteBody.note.note = noteText; noteBody.note.note = noteText;
} }
}, },
getLineClasses(lineNumber) {
return getLineClasses(lineNumber);
},
}, },
}; };
</script> </script>
...@@ -251,6 +311,26 @@ export default { ...@@ -251,6 +311,26 @@ export default {
:data-note-id="note.id" :data-note-id="note.id"
class="note note-wrapper qa-noteable-note-item" class="note note-wrapper qa-noteable-note-item"
> >
<div v-if="showMultiLineComment" data-testid="multiline-comment">
<multiline-comment-form
v-if="isEditing && commentLineOptions && line"
v-model="commentLineStart"
:line="line"
:comment-line-options="commentLineOptions"
:line-range="note.position.line_range"
class="gl-mb-3 gl-text-gray-700 gl-border-gray-200 gl-border-b-solid gl-border-b-1 gl-pb-3"
/>
<div v-else class="gl-mb-3 gl-text-gray-700">
<gl-sprintf :message="__('Comment on lines %{startLine} to %{endLine}')">
<template #startLine>
<span :class="getLineClasses(startLineNumber)">{{ startLineNumber }}</span>
</template>
<template #endLine>
<span :class="getLineClasses(endLineNumber)">{{ endLineNumber }}</span>
</template>
</gl-sprintf>
</div>
</div>
<div v-once class="timeline-icon"> <div v-once class="timeline-icon">
<user-avatar-link <user-avatar-link
:link-href="author.path" :link-href="author.path"
......
...@@ -33,6 +33,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo ...@@ -33,6 +33,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_frontend_feature_flag(:widget_visibility_polling, @project, default_enabled: true) push_frontend_feature_flag(:widget_visibility_polling, @project, default_enabled: true)
push_frontend_feature_flag(:merge_ref_head_comments, @project) push_frontend_feature_flag(:merge_ref_head_comments, @project)
push_frontend_feature_flag(:mr_commit_neighbor_nav, @project, default_enabled: true) push_frontend_feature_flag(:mr_commit_neighbor_nav, @project, default_enabled: true)
push_frontend_feature_flag(:multiline_comments, @project)
end end
before_action do before_action do
......
---
title: Add frontend support for multiline comments
merge_request: 29516
author:
type: added
...@@ -783,7 +783,9 @@ Diff comments also contain position: ...@@ -783,7 +783,9 @@ Diff comments also contain position:
"new_line": 27, "new_line": 27,
"line_range": { "line_range": {
"start_line_code": "588440f66559714280628a4f9799f0c4eb880a4a_10_10", "start_line_code": "588440f66559714280628a4f9799f0c4eb880a4a_10_10",
"end_line_code": "588440f66559714280628a4f9799f0c4eb880a4a_11_11" "start_line_type": "new",
"end_line_code": "588440f66559714280628a4f9799f0c4eb880a4a_11_11",
"end_line_type": "old"
} }
}, },
"resolved": false, "resolved": false,
...@@ -848,6 +850,8 @@ Parameters: ...@@ -848,6 +850,8 @@ Parameters:
| `position[line_range]` | hash | no | Line range for a multi-line diff note | | `position[line_range]` | hash | no | Line range for a multi-line diff note |
| `position[line_range][start_line_code]` | string | yes | Line code for the start line | | `position[line_range][start_line_code]` | string | yes | Line code for the start line |
| `position[line_range][end_line_code]` | string | yes | Line code for the end line | | `position[line_range][end_line_code]` | string | yes | Line code for the end line |
| `position[line_range][start_line_type]` | string | yes | Line type for the start line |
| `position[line_range][end_line_type]` | string | yes | Line type for the end line |
| `position[width]` | integer | no | Width of the image (for 'image' diff notes) | | `position[width]` | integer | no | Width of the image (for 'image' diff notes) |
| `position[height]` | integer | no | Height of the image (for 'image' diff notes) | | `position[height]` | integer | no | Height of the image (for 'image' diff notes) |
| `position[x]` | integer | no | X coordinate (for 'image' diff notes) | | `position[x]` | integer | no | X coordinate (for 'image' diff notes) |
......
...@@ -78,6 +78,8 @@ module API ...@@ -78,6 +78,8 @@ module API
optional :line_range, type: Hash, desc: 'Multi-line start and end' do optional :line_range, type: Hash, desc: 'Multi-line start and end' do
requires :start_line_code, type: String, desc: 'Start line code for multi-line note' requires :start_line_code, type: String, desc: 'Start line code for multi-line note'
requires :end_line_code, type: String, desc: 'End line code for multi-line note' requires :end_line_code, type: String, desc: 'End line code for multi-line note'
requires :start_line_type, type: String, desc: 'Start line type for multi-line note'
requires :end_line_type, type: String, desc: 'End line type for multi-line note'
end end
end end
end end
......
...@@ -833,6 +833,9 @@ msgstr "" ...@@ -833,6 +833,9 @@ msgstr ""
msgid "8 hours" msgid "8 hours"
msgstr "" msgstr ""
msgid ":%{startLine} to %{endLine}"
msgstr ""
msgid "< 1 hour" msgid "< 1 hour"
msgstr "" msgstr ""
...@@ -5568,6 +5571,9 @@ msgstr "" ...@@ -5568,6 +5571,9 @@ msgstr ""
msgid "Comment is being updated" msgid "Comment is being updated"
msgstr "" msgstr ""
msgid "Comment on lines %{startLine} to %{endLine}"
msgstr ""
msgid "Comment/Reply (quoting selected text)" msgid "Comment/Reply (quoting selected text)"
msgstr "" msgstr ""
...@@ -13621,6 +13627,12 @@ msgstr "" ...@@ -13621,6 +13627,12 @@ msgstr ""
msgid "MergeConflict|origin//their changes" msgid "MergeConflict|origin//their changes"
msgstr "" msgstr ""
msgid "MergeRequestDiffs|Commenting on lines %{selectStart}start%{selectEnd} to %{end}"
msgstr ""
msgid "MergeRequestDiffs|Select comment starting line"
msgstr ""
msgid "MergeRequests|Add a reply" msgid "MergeRequests|Add a reply"
msgstr "" msgstr ""
......
...@@ -114,6 +114,40 @@ describe 'User comments on a diff', :js do ...@@ -114,6 +114,40 @@ describe 'User comments on a diff', :js do
include_examples 'comment on merge request file' include_examples 'comment on merge request file'
end end
context 'when adding multiline comments' do
it 'saves a multiline comment' do
click_diff_line(find("[id='#{sample_commit.line_code}']"))
page.within('.discussion-form') do
find('#comment-line-start option', text: '-13').select_option
end
page.within('.js-discussion-note-form') do
fill_in(:note_note, with: 'Line is wrong')
click_button('Add comment now')
end
wait_for_requests
page.within('.notes_holder') do
expect(page).to have_content('Line is wrong')
expect(page).to have_content('Comment on lines -13 to +14')
end
visit(merge_request_path(merge_request))
page.within('.notes .discussion') do
expect(page).to have_content("#{user.name} #{user.to_reference} started a thread")
expect(page).to have_content(sample_commit.line_code_path)
expect(page).to have_content('Line is wrong')
end
page.within('.notes-tab .badge') do
expect(page).to have_content('1')
end
end
end
context 'when editing comments' do context 'when editing comments' do
it 'edits a comment' do it 'edits a comment' do
click_diff_line(find("[id='#{sample_commit.line_code}']")) click_diff_line(find("[id='#{sample_commit.line_code}']"))
......
...@@ -77,12 +77,24 @@ describe('DiffLineNoteForm', () => { ...@@ -77,12 +77,24 @@ describe('DiffLineNoteForm', () => {
.spyOn(wrapper.vm, 'saveDiffDiscussion') .spyOn(wrapper.vm, 'saveDiffDiscussion')
.mockReturnValue(Promise.resolve()); .mockReturnValue(Promise.resolve());
const lineRange = {
start_line_code: wrapper.vm.commentLineStart.lineCode,
start_line_type: wrapper.vm.commentLineStart.type,
end_line_code: wrapper.vm.line.line_code,
end_line_type: wrapper.vm.line.type,
};
const formData = {
...wrapper.vm.formData,
lineRange,
};
wrapper.vm wrapper.vm
.handleSaveNote('note body') .handleSaveNote('note body')
.then(() => { .then(() => {
expect(saveDiffDiscussionSpy).toHaveBeenCalledWith({ expect(saveDiffDiscussionSpy).toHaveBeenCalledWith({
note: 'note body', note: 'note body',
formData: wrapper.vm.formData, formData,
}); });
}) })
.then(done) .then(done)
......
...@@ -543,9 +543,9 @@ describe('DiffsStoreActions', () => { ...@@ -543,9 +543,9 @@ describe('DiffsStoreActions', () => {
new_path: 'file1', new_path: 'file1',
old_line: 5, old_line: 5,
old_path: 'file2', old_path: 'file2',
line_range: null,
line_code: 'ABC_1_1', line_code: 'ABC_1_1',
position_type: 'text', position_type: 'text',
line_range: null,
}, },
}, },
hash: 'ABC_123', hash: 'ABC_123',
......
...@@ -187,6 +187,7 @@ describe('DiffsStoreUtils', () => { ...@@ -187,6 +187,7 @@ describe('DiffsStoreUtils', () => {
}, },
diffViewType: PARALLEL_DIFF_VIEW_TYPE, diffViewType: PARALLEL_DIFF_VIEW_TYPE,
linePosition: LINE_POSITION_LEFT, linePosition: LINE_POSITION_LEFT,
lineRange: { start_line_code: 'abc_1_1', end_line_code: 'abc_2_2' },
}; };
const position = JSON.stringify({ const position = JSON.stringify({
...@@ -198,6 +199,7 @@ describe('DiffsStoreUtils', () => { ...@@ -198,6 +199,7 @@ describe('DiffsStoreUtils', () => {
position_type: TEXT_DIFF_POSITION_TYPE, position_type: TEXT_DIFF_POSITION_TYPE,
old_line: options.noteTargetLine.old_line, old_line: options.noteTargetLine.old_line,
new_line: options.noteTargetLine.new_line, new_line: options.noteTargetLine.new_line,
line_range: options.lineRange,
}); });
const postData = { const postData = {
......
import {
getSymbol,
getStartLineNumber,
getEndLineNumber,
} from '~/notes/components/multiline_comment_utils';
describe('Multiline comment utilities', () => {
describe('getStartLineNumber', () => {
it.each`
lineCode | type | result
${'abcdef_1_1'} | ${'old'} | ${'-1'}
${'abcdef_1_1'} | ${'new'} | ${'+1'}
${'abcdef_1_1'} | ${null} | ${'1'}
${'abcdef'} | ${'new'} | ${''}
${'abcdef'} | ${'old'} | ${''}
${'abcdef'} | ${null} | ${''}
`('returns line number', ({ lineCode, type, result }) => {
expect(getStartLineNumber({ start_line_code: lineCode, start_line_type: type })).toEqual(
result,
);
});
});
describe('getEndLineNumber', () => {
it.each`
lineCode | type | result
${'abcdef_1_1'} | ${'old'} | ${'-1'}
${'abcdef_1_1'} | ${'new'} | ${'+1'}
${'abcdef_1_1'} | ${null} | ${'1'}
${'abcdef'} | ${'new'} | ${''}
${'abcdef'} | ${'old'} | ${''}
${'abcdef'} | ${null} | ${''}
`('returns line number', ({ lineCode, type, result }) => {
expect(getEndLineNumber({ end_line_code: lineCode, end_line_type: type })).toEqual(result);
});
});
describe('getSymbol', () => {
it.each`
type | result
${'new'} | ${'+'}
${'old'} | ${'-'}
${'unused'} | ${''}
${''} | ${''}
${null} | ${''}
${undefined} | ${''}
`('`$type` returns `$result`', ({ type, result }) => {
expect(getSymbol(type)).toEqual(result);
});
});
});
import { escape } from 'lodash'; import { escape } from 'lodash';
import { shallowMount, createLocalVue } from '@vue/test-utils'; import { mount, createLocalVue } from '@vue/test-utils';
import createStore from '~/notes/stores'; import createStore from '~/notes/stores';
import issueNote from '~/notes/components/noteable_note.vue'; import issueNote from '~/notes/components/noteable_note.vue';
import NoteHeader from '~/notes/components/note_header.vue'; import NoteHeader from '~/notes/components/note_header.vue';
...@@ -8,9 +8,19 @@ import NoteActions from '~/notes/components/note_actions.vue'; ...@@ -8,9 +8,19 @@ import NoteActions from '~/notes/components/note_actions.vue';
import NoteBody from '~/notes/components/note_body.vue'; import NoteBody from '~/notes/components/note_body.vue';
import { noteableDataMock, notesDataMock, note } from '../mock_data'; 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', () => { describe('issue_note', () => {
let store; let store;
let wrapper; let wrapper;
const findMultilineComment = () => wrapper.find('[data-testid="multiline-comment"]');
beforeEach(() => { beforeEach(() => {
store = createStore(); store = createStore();
...@@ -18,12 +28,13 @@ describe('issue_note', () => { ...@@ -18,12 +28,13 @@ describe('issue_note', () => {
store.dispatch('setNotesData', notesDataMock); store.dispatch('setNotesData', notesDataMock);
const localVue = createLocalVue(); const localVue = createLocalVue();
wrapper = shallowMount(localVue.extend(issueNote), { wrapper = mount(localVue.extend(issueNote), {
store, store,
propsData: { propsData: {
note, note,
}, },
localVue, localVue,
stubs: ['note-header', 'user-avatar-link', 'note-actions', 'note-body'],
}); });
}); });
...@@ -31,6 +42,44 @@ describe('issue_note', () => { ...@@ -31,6 +42,44 @@ describe('issue_note', () => {
wrapper.destroy(); wrapper.destroy();
}); });
describe('mutiline comments', () => {
it('should render if has multiline comment', () => {
const position = {
line_range: {
start_line_code: 'abc_1_1',
end_line_code: 'abc_2_2',
},
};
wrapper.setProps({
note: { ...note, position },
});
return wrapper.vm.$nextTick().then(() => {
expect(findMultilineComment().text()).toEqual('Comment on lines 1 to 2');
});
});
it('should not render if has single line comment', () => {
const position = {
line_range: {
start_line_code: 'abc_1_1',
end_line_code: 'abc_1_1',
},
};
wrapper.setProps({
note: { ...note, position },
});
return wrapper.vm.$nextTick().then(() => {
expect(findMultilineComment().exists()).toBe(false);
});
});
it('should not render if `line_range` is unavailable', () => {
expect(findMultilineComment().exists()).toBe(false);
});
});
it('should render user information', () => { it('should render user information', () => {
const { author } = note; const { author } = note;
const avatar = wrapper.find(UserAvatarLink); const avatar = wrapper.find(UserAvatarLink);
......
...@@ -13,6 +13,7 @@ RSpec.shared_examples 'comment on merge request file' do ...@@ -13,6 +13,7 @@ RSpec.shared_examples 'comment on merge request file' do
page.within('.notes_holder') do page.within('.notes_holder') do
expect(page).to have_content('Line is wrong') expect(page).to have_content('Line is wrong')
expect(page).not_to have_content('Comment on lines')
end end
visit(merge_request_path(merge_request)) visit(merge_request_path(merge_request))
......
...@@ -30,7 +30,9 @@ RSpec.shared_examples 'diff discussions API' do |parent_type, noteable_type, id_ ...@@ -30,7 +30,9 @@ RSpec.shared_examples 'diff discussions API' do |parent_type, noteable_type, id_
it "creates a new diff note" do it "creates a new diff note" do
line_range = { line_range = {
"start_line_code" => Gitlab::Git.diff_line_code(diff_note.position.file_path, 1, 1), "start_line_code" => Gitlab::Git.diff_line_code(diff_note.position.file_path, 1, 1),
"end_line_code" => Gitlab::Git.diff_line_code(diff_note.position.file_path, 2, 2) "end_line_code" => Gitlab::Git.diff_line_code(diff_note.position.file_path, 2, 2),
"start_line_type" => diff_note.position.type,
"end_line_type" => diff_note.position.type
} }
position = diff_note.position.to_h.merge({ line_range: line_range }) position = diff_note.position.to_h.merge({ line_range: line_range })
......
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