Commit 5eef5f24 authored by Phil Hughes's avatar Phil Hughes

Updated LegacyDiffNote logic

parent ef4e3b6e
import Vue from 'vue'; import Vue from 'vue';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import { isLegacyDiffNote } from '~/notes/stores/utils';
import { import {
findDiffFile, findDiffFile,
addLineReferences, addLineReferences,
...@@ -87,18 +86,18 @@ export default { ...@@ -87,18 +86,18 @@ export default {
}, },
[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, discussions, diffPositionByLineCode }) { [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, discussions, diffPositionByLineCode }) {
if (!state.latestDiff) return;
const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash); const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash);
const firstDiscussion = discussions[0]; const firstDiscussion = discussions[0];
const isDiffDiscussion = firstDiscussion.diff_discussion; const isDiffDiscussion = firstDiscussion.diff_discussion;
const hasLineCode = firstDiscussion.line_code; const hasLineCode = firstDiscussion.line_code;
const isResolvable = firstDiscussion.resolvable || isLegacyDiffNote(firstDiscussion);
const diffPosition = diffPositionByLineCode[firstDiscussion.line_code]; const diffPosition = diffPositionByLineCode[firstDiscussion.line_code];
if ( if (
selectedFile && selectedFile &&
isDiffDiscussion && isDiffDiscussion &&
hasLineCode && hasLineCode &&
isResolvable &&
diffPosition && diffPosition &&
isDiscussionApplicableToLine(firstDiscussion, diffPosition) isDiscussionApplicableToLine(firstDiscussion, diffPosition)
) { ) {
......
...@@ -61,7 +61,10 @@ export function getNoteFormData(params) { ...@@ -61,7 +61,10 @@ export function getNoteFormData(params) {
noteable_type: noteableType, noteable_type: noteableType,
noteable_id: noteableData.id, noteable_id: noteableData.id,
commit_id: '', commit_id: '',
type: diffFile.diffRefs.startSha ? DIFF_NOTE_TYPE : LEGACY_DIFF_NOTE_TYPE, type:
diffFile.diffRefs.startSha && diffFile.diffRefs.headSha
? DIFF_NOTE_TYPE
: LEGACY_DIFF_NOTE_TYPE,
line_code: noteTargetLine.lineCode, line_code: noteTargetLine.lineCode,
}, },
}; };
...@@ -261,5 +264,5 @@ export function isDiscussionApplicableToLine(discussion, diffPosition) { ...@@ -261,5 +264,5 @@ export function isDiscussionApplicableToLine(discussion, diffPosition) {
return _.isEqual(refs, diffPositionCopy) || _.isEqual(originalRefs, diffPositionCopy); return _.isEqual(refs, diffPositionCopy) || _.isEqual(originalRefs, diffPositionCopy);
} }
return lineCode === discussion.line_code; return discussion.active && lineCode === discussion.line_code;
} }
...@@ -2,7 +2,6 @@ import AjaxCache from '~/lib/utils/ajax_cache'; ...@@ -2,7 +2,6 @@ import AjaxCache from '~/lib/utils/ajax_cache';
const REGEX_QUICK_ACTIONS = /^\/\w+.*$/gm; const REGEX_QUICK_ACTIONS = /^\/\w+.*$/gm;
export const isLegacyDiffNote = note => !note.resolvable && !note.position;
export const findNoteObjectById = (notes, id) => notes.filter(n => n.id === id)[0]; export const findNoteObjectById = (notes, id) => notes.filter(n => n.id === id)[0];
export const getQuickActionText = note => { export const getQuickActionText = note => {
...@@ -28,7 +27,7 @@ export const getQuickActionText = note => { ...@@ -28,7 +27,7 @@ export const getQuickActionText = note => {
export const reduceDiscussionsToLineCodes = selectedDiscussions => export const reduceDiscussionsToLineCodes = selectedDiscussions =>
selectedDiscussions.reduce((acc, note) => { selectedDiscussions.reduce((acc, note) => {
if (note.diff_discussion && note.line_code && (note.resolvable || isLegacyDiffNote(note))) { if (note.diff_discussion && note.line_code) {
// For context about line notes: there might be multiple notes with the same line code // For context about line notes: there might be multiple notes with the same line code
const items = acc[note.line_code] || []; const items = acc[note.line_code] || [];
items.push(note); items.push(note);
......
...@@ -162,6 +162,7 @@ describe('DiffsStoreMutations', () => { ...@@ -162,6 +162,7 @@ describe('DiffsStoreMutations', () => {
}; };
const state = { const state = {
latestDiff: true,
diffFiles: [ diffFiles: [
{ {
fileHash: 'ABC', fileHash: 'ABC',
...@@ -243,6 +244,7 @@ describe('DiffsStoreMutations', () => { ...@@ -243,6 +244,7 @@ describe('DiffsStoreMutations', () => {
}; };
const state = { const state = {
latestDiff: true,
diffFiles: [ diffFiles: [
{ {
fileHash: 'ABC', fileHash: 'ABC',
...@@ -272,11 +274,13 @@ describe('DiffsStoreMutations', () => { ...@@ -272,11 +274,13 @@ describe('DiffsStoreMutations', () => {
id: 1, id: 1,
line_code: 'ABC_1', line_code: 'ABC_1',
diff_discussion: true, diff_discussion: true,
active: true,
}, },
{ {
id: 2, id: 2,
line_code: 'ABC_1', line_code: 'ABC_1',
diff_discussion: true, diff_discussion: true,
active: true,
}, },
]; ];
......
...@@ -156,6 +156,7 @@ describe('DiffsStoreUtils', () => { ...@@ -156,6 +156,7 @@ describe('DiffsStoreUtils', () => {
it('should create legacy note form data', () => { it('should create legacy note form data', () => {
const diffFile = getDiffFileMock(); const diffFile = getDiffFileMock();
delete diffFile.diffRefs.startSha; delete diffFile.diffRefs.startSha;
delete diffFile.diffRefs.headSha;
noteableDataMock.targetType = MERGE_REQUEST_NOTEABLE_TYPE; noteableDataMock.targetType = MERGE_REQUEST_NOTEABLE_TYPE;
...@@ -177,7 +178,7 @@ describe('DiffsStoreUtils', () => { ...@@ -177,7 +178,7 @@ describe('DiffsStoreUtils', () => {
const position = JSON.stringify({ const position = JSON.stringify({
base_sha: diffFile.diffRefs.baseSha, base_sha: diffFile.diffRefs.baseSha,
start_sha: undefined, start_sha: undefined,
head_sha: diffFile.diffRefs.headSha, head_sha: undefined,
old_path: diffFile.oldPath, old_path: diffFile.oldPath,
new_path: diffFile.newPath, new_path: diffFile.newPath,
position_type: TEXT_DIFF_POSITION_TYPE, position_type: TEXT_DIFF_POSITION_TYPE,
...@@ -188,7 +189,7 @@ describe('DiffsStoreUtils', () => { ...@@ -188,7 +189,7 @@ describe('DiffsStoreUtils', () => {
const postData = { const postData = {
view: options.diffViewType, view: options.diffViewType,
line_type: options.linePosition === LINE_POSITION_RIGHT ? NEW_LINE_TYPE : OLD_LINE_TYPE, line_type: options.linePosition === LINE_POSITION_RIGHT ? NEW_LINE_TYPE : OLD_LINE_TYPE,
merge_request_diff_head_sha: diffFile.diffRefs.headSha, merge_request_diff_head_sha: undefined,
in_reply_to_discussion_id: '', in_reply_to_discussion_id: '',
note_project_id: '', note_project_id: '',
target_type: options.noteableType, target_type: options.noteableType,
...@@ -359,8 +360,21 @@ describe('DiffsStoreUtils', () => { ...@@ -359,8 +360,21 @@ describe('DiffsStoreUtils', () => {
).toBe(false); ).toBe(false);
}); });
it('returns true when line codes match and discussion does not contain position', () => { it('returns true when line codes match and discussion does not contain position and is not active', () => {
const discussion = { ...discussions.outDatedDiscussion1, line_code: 'ABC_1' }; const discussion = { ...discussions.outDatedDiscussion1, line_code: 'ABC_1', active: false };
delete discussion.original_position;
delete discussion.position;
expect(
utils.isDiscussionApplicableToLine(discussion, {
...diffPosition,
lineCode: 'ABC_1',
}),
).toBe(false);
});
it('returns true when line codes match and discussion does not contain position and is active', () => {
const discussion = { ...discussions.outDatedDiscussion1, line_code: 'ABC_1', active: true };
delete discussion.original_position; delete discussion.original_position;
delete discussion.position; delete discussion.position;
......
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