summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/assets/javascripts/diffs/store/mutations.js5
-rw-r--r--app/assets/javascripts/diffs/store/utils.js7
-rw-r--r--app/assets/javascripts/notes/stores/utils.js3
-rw-r--r--spec/javascripts/diffs/store/mutations_spec.js4
-rw-r--r--spec/javascripts/diffs/store/utils_spec.js22
5 files changed, 30 insertions, 11 deletions
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index 72da64bd4db..a11ac2b292b 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -1,6 +1,5 @@
import Vue from 'vue';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
-import { isLegacyDiffNote } from '~/notes/stores/utils';
import {
findDiffFile,
addLineReferences,
@@ -87,18 +86,18 @@ export default {
},
[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, discussions, diffPositionByLineCode }) {
+ if (!state.latestDiff) return;
+
const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash);
const firstDiscussion = discussions[0];
const isDiffDiscussion = firstDiscussion.diff_discussion;
const hasLineCode = firstDiscussion.line_code;
- const isResolvable = firstDiscussion.resolvable || isLegacyDiffNote(firstDiscussion);
const diffPosition = diffPositionByLineCode[firstDiscussion.line_code];
if (
selectedFile &&
isDiffDiscussion &&
hasLineCode &&
- isResolvable &&
diffPosition &&
isDiscussionApplicableToLine(firstDiscussion, diffPosition)
) {
diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js
index d521e4584ad..36053d8db44 100644
--- a/app/assets/javascripts/diffs/store/utils.js
+++ b/app/assets/javascripts/diffs/store/utils.js
@@ -61,7 +61,10 @@ export function getNoteFormData(params) {
noteable_type: noteableType,
noteable_id: noteableData.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,
},
};
@@ -261,5 +264,5 @@ export function isDiscussionApplicableToLine(discussion, diffPosition) {
return _.isEqual(refs, diffPositionCopy) || _.isEqual(originalRefs, diffPositionCopy);
}
- return lineCode === discussion.line_code;
+ return discussion.active && lineCode === discussion.line_code;
}
diff --git a/app/assets/javascripts/notes/stores/utils.js b/app/assets/javascripts/notes/stores/utils.js
index eea6eb2b1af..0e41ff03d67 100644
--- a/app/assets/javascripts/notes/stores/utils.js
+++ b/app/assets/javascripts/notes/stores/utils.js
@@ -2,7 +2,6 @@ import AjaxCache from '~/lib/utils/ajax_cache';
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 getQuickActionText = note => {
@@ -28,7 +27,7 @@ export const getQuickActionText = note => {
export const reduceDiscussionsToLineCodes = selectedDiscussions =>
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
const items = acc[note.line_code] || [];
items.push(note);
diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js
index 8e94b21f737..9a5d8dfbd15 100644
--- a/spec/javascripts/diffs/store/mutations_spec.js
+++ b/spec/javascripts/diffs/store/mutations_spec.js
@@ -162,6 +162,7 @@ describe('DiffsStoreMutations', () => {
};
const state = {
+ latestDiff: true,
diffFiles: [
{
fileHash: 'ABC',
@@ -243,6 +244,7 @@ describe('DiffsStoreMutations', () => {
};
const state = {
+ latestDiff: true,
diffFiles: [
{
fileHash: 'ABC',
@@ -272,11 +274,13 @@ describe('DiffsStoreMutations', () => {
id: 1,
line_code: 'ABC_1',
diff_discussion: true,
+ active: true,
},
{
id: 2,
line_code: 'ABC_1',
diff_discussion: true,
+ active: true,
},
];
diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js
index 4b5955dd4b5..fd740c5e798 100644
--- a/spec/javascripts/diffs/store/utils_spec.js
+++ b/spec/javascripts/diffs/store/utils_spec.js
@@ -156,6 +156,7 @@ describe('DiffsStoreUtils', () => {
it('should create legacy note form data', () => {
const diffFile = getDiffFileMock();
delete diffFile.diffRefs.startSha;
+ delete diffFile.diffRefs.headSha;
noteableDataMock.targetType = MERGE_REQUEST_NOTEABLE_TYPE;
@@ -177,7 +178,7 @@ describe('DiffsStoreUtils', () => {
const position = JSON.stringify({
base_sha: diffFile.diffRefs.baseSha,
start_sha: undefined,
- head_sha: diffFile.diffRefs.headSha,
+ head_sha: undefined,
old_path: diffFile.oldPath,
new_path: diffFile.newPath,
position_type: TEXT_DIFF_POSITION_TYPE,
@@ -188,7 +189,7 @@ describe('DiffsStoreUtils', () => {
const postData = {
view: options.diffViewType,
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: '',
note_project_id: '',
target_type: options.noteableType,
@@ -359,8 +360,21 @@ describe('DiffsStoreUtils', () => {
).toBe(false);
});
- it('returns true when line codes match and discussion does not contain position', () => {
- const discussion = { ...discussions.outDatedDiscussion1, line_code: 'ABC_1' };
+ 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', 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.position;
View Lorry Controller Status