From 03df54b226f63a05ee2229b9f7f1f3e90383430a Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 22 Jan 2019 13:35:28 +0100 Subject: Trim first char of diff line text on diff discussions Before, diff file `higlighted_diff_lines`/`parallel_diff_lines` and diff discussion `truncated_diff_lines` were inconsistent: `text` and `rich_text` on the latter included the leading +/-/ character, like on the backend, while the former had no `text` and its `rich_text` had dropped this char. This resulted in a bug when the suggestions feature expected these diff line objects to be identical in format and thus interchangeable, which was not the case. --- app/assets/javascripts/diffs/store/utils.js | 10 +++++++--- app/assets/javascripts/notes/components/diff_with_note.vue | 7 +------ .../javascripts/notes/components/noteable_discussion.vue | 6 +++++- app/assets/javascripts/notes/stores/mutations.js | 7 +++++-- app/assets/javascripts/notes/stores/utils.js | 4 ++++ 5 files changed, 22 insertions(+), 12 deletions(-) (limited to 'app') diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index ada93b570b0..09afacc24df 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -181,8 +181,6 @@ export function addContextLines(options) { export function trimFirstCharOfLineContent(line = {}) { // eslint-disable-next-line no-param-reassign delete line.text; - // eslint-disable-next-line no-param-reassign - line.discussions = []; const parsedLine = Object.assign({}, line); @@ -222,10 +220,12 @@ export function prepareDiffData(diffData) { line.line_code = getLineCode(line, u); if (line.left) { line.left = trimFirstCharOfLineContent(line.left); + line.left.discussions = []; line.left.hasForm = false; } if (line.right) { line.right = trimFirstCharOfLineContent(line.right); + line.right.discussions = []; line.right.hasForm = false; } } @@ -235,7 +235,11 @@ export function prepareDiffData(diffData) { const linesLength = file.highlighted_diff_lines.length; for (let u = 0; u < linesLength; u += 1) { const line = file.highlighted_diff_lines[u]; - Object.assign(line, { ...trimFirstCharOfLineContent(line), hasForm: false }); + Object.assign(line, { + ...trimFirstCharOfLineContent(line), + discussions: [], + hasForm: false, + }); } showingLines += file.parallel_diff_lines.length; } diff --git a/app/assets/javascripts/notes/components/diff_with_note.vue b/app/assets/javascripts/notes/components/diff_with_note.vue index af821df0fd2..376d4114efd 100644 --- a/app/assets/javascripts/notes/components/diff_with_note.vue +++ b/app/assets/javascripts/notes/components/diff_with_note.vue @@ -6,8 +6,6 @@ import ImageDiffOverlay from '~/diffs/components/image_diff_overlay.vue'; import { GlSkeletonLoading } from '@gitlab/ui'; import { getDiffMode } from '~/diffs/store/utils'; -const FIRST_CHAR_REGEX = /^(\+|-| )/; - export default { components: { DiffFileHeader, @@ -54,9 +52,6 @@ export default { this.error = true; }); }, - trimChar(line) { - return line.replace(FIRST_CHAR_REGEX, ''); - }, }, userColorSchemeClass: window.gon.user_color_scheme, }; @@ -85,7 +80,7 @@ export default { > {{ line.old_line }} {{ line.new_line }} - + diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index 4480ec74182..1a116161e3c 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -206,11 +206,15 @@ export default { return sprintf(text, { commitId, linkStart, linkEnd }, false); }, diffLine() { + if (this.line) { + return this.line; + } + if (this.discussion.diff_discussion && this.discussion.truncated_diff_lines) { return this.discussion.truncated_diff_lines.slice(-1)[0]; } - return this.line; + return null; }, }, watch: { diff --git a/app/assets/javascripts/notes/stores/mutations.js b/app/assets/javascripts/notes/stores/mutations.js index 8992454be2e..33d39ad2ec9 100644 --- a/app/assets/javascripts/notes/stores/mutations.js +++ b/app/assets/javascripts/notes/stores/mutations.js @@ -105,7 +105,10 @@ export default { if (discussion.diff_file) { diffData.file_hash = discussion.diff_file.file_hash; - diffData.truncated_diff_lines = discussion.truncated_diff_lines || []; + + diffData.truncated_diff_lines = utils.prepareDiffLines( + discussion.truncated_diff_lines || [], + ); } // To support legacy notes, should be very rare case. @@ -243,7 +246,7 @@ export default { [types.SET_DISCUSSION_DIFF_LINES](state, { discussionId, diffLines }) { const discussion = utils.findNoteObjectById(state.discussions, discussionId); - discussion.truncated_diff_lines = diffLines; + discussion.truncated_diff_lines = utils.prepareDiffLines(diffLines); }, [types.DISABLE_COMMENTS](state, value) { diff --git a/app/assets/javascripts/notes/stores/utils.js b/app/assets/javascripts/notes/stores/utils.js index dd57539e4d8..4b0feb0f94d 100644 --- a/app/assets/javascripts/notes/stores/utils.js +++ b/app/assets/javascripts/notes/stores/utils.js @@ -1,4 +1,5 @@ import AjaxCache from '~/lib/utils/ajax_cache'; +import { trimFirstCharOfLineContent } from '~/diffs/store/utils'; const REGEX_QUICK_ACTIONS = /^\/\w+.*$/gm; @@ -28,3 +29,6 @@ export const getQuickActionText = note => { export const hasQuickActions = note => REGEX_QUICK_ACTIONS.test(note); export const stripQuickActions = note => note.replace(REGEX_QUICK_ACTIONS, '').trim(); + +export const prepareDiffLines = diffLines => + diffLines.map(line => ({ ...trimFirstCharOfLineContent(line) })); -- cgit v1.2.1