Commit 301a92ff authored by Igor Drozdov's avatar Igor Drozdov

Display notes based on line_codes and positions props

Newly introduced API for displaying notes for merge-ref-head diffs
contains line_codes and positions properties. If any of the passed
line_codes or positions matches line_code and position of a particular
line, then the note is displayed.

This code is backward compatible with the previous interface
parent 43a26fd0
......@@ -99,8 +99,12 @@ export default {
return this.showCommentButton && this.hasDiscussions;
},
shouldRenderCommentButton() {
const isDiffHead = parseBoolean(getParameterByName('diff_head'));
return !isDiffHead && this.isLoggedIn && this.showCommentButton;
if (this.isLoggedIn && this.showCommentButton) {
const isDiffHead = parseBoolean(getParameterByName('diff_head'));
return !isDiffHead || gon.features?.mergeRefHeadComments;
}
return false;
},
isMatchLine() {
return this.line.type === MATCH_LINE_TYPE;
......
......@@ -182,15 +182,18 @@ export default {
[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { discussion, diffPositionByLineCode, hash }) {
const { latestDiff } = state;
const discussionLineCode = discussion.line_code;
const discussionLineCodes = [discussion.line_code, ...(discussion.line_codes || [])];
const fileHash = discussion.diff_file.file_hash;
const lineCheck = line =>
line.line_code === discussionLineCode &&
isDiscussionApplicableToLine({
discussion,
diffPosition: diffPositionByLineCode[line.line_code],
latestDiff,
});
discussionLineCodes.some(
discussionLineCode =>
line.line_code === discussionLineCode &&
isDiscussionApplicableToLine({
discussion,
diffPosition: diffPositionByLineCode[line.line_code],
latestDiff,
}),
);
const mapDiscussions = (line, extraCheck = () => true) => ({
...line,
discussions: extraCheck()
......
......@@ -440,10 +440,13 @@ export function isDiscussionApplicableToLine({ discussion, diffPosition, latestD
const { line_code, ...diffPositionCopy } = diffPosition;
if (discussion.original_position && discussion.position) {
const originalRefs = discussion.original_position;
const refs = discussion.position;
const discussionPositions = [
discussion.original_position,
discussion.position,
...(discussion.positions || []),
];
return isEqual(refs, diffPositionCopy) || isEqual(originalRefs, diffPositionCopy);
return discussionPositions.some(position => isEqual(position, diffPositionCopy));
}
// eslint-disable-next-line
......
......@@ -25,6 +25,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_frontend_feature_flag(:suggest_pipeline) if experiment_enabled?(:suggest_pipeline)
push_frontend_feature_flag(:code_navigation, @project)
push_frontend_feature_flag(:widget_visibility_polling, @project, default_enabled: true)
push_frontend_feature_flag(:merge_ref_head_comments, @project)
end
before_action do
......
......@@ -85,15 +85,18 @@ describe('DiffTableCell', () => {
describe('comment button', () => {
it.each`
showCommentButton | userData | query | expectation
${true} | ${TEST_USER} | ${'diff_head=false'} | ${true}
${true} | ${TEST_USER} | ${'diff_head=true'} | ${false}
${false} | ${TEST_USER} | ${'bogus'} | ${false}
${true} | ${null} | ${''} | ${false}
showCommentButton | userData | query | mergeRefHeadComments | expectation
${true} | ${TEST_USER} | ${'diff_head=false'} | ${false} | ${true}
${true} | ${TEST_USER} | ${'diff_head=true'} | ${true} | ${true}
${true} | ${TEST_USER} | ${'diff_head=true'} | ${false} | ${false}
${false} | ${TEST_USER} | ${'diff_head=true'} | ${true} | ${false}
${false} | ${TEST_USER} | ${'bogus'} | ${true} | ${false}
${true} | ${null} | ${''} | ${true} | ${false}
`(
'exists is $expectation - with showCommentButton ($showCommentButton) userData ($userData) query ($query)',
({ showCommentButton, userData, query, expectation }) => {
({ showCommentButton, userData, query, mergeRefHeadComments, expectation }) => {
store.state.notes.userData = userData;
gon.features = { mergeRefHeadComments };
setWindowLocation({ href: `${TEST_HOST}?${query}` });
createComponent({ showCommentButton });
......
......@@ -615,6 +615,73 @@ describe('DiffsStoreMutations', () => {
expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(1);
expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1);
});
it('should add discussions by line_codes and positions attributes', () => {
const diffPosition = {
base_sha: 'ed13df29948c41ba367caa757ab3ec4892509910',
head_sha: 'b921914f9a834ac47e6fd9420f78db0f83559130',
new_line: null,
new_path: '500-lines-4.txt',
old_line: 5,
old_path: '500-lines-4.txt',
start_sha: 'ed13df29948c41ba367caa757ab3ec4892509910',
};
const state = {
latestDiff: true,
diffFiles: [
{
file_hash: 'ABC',
parallel_diff_lines: [
{
left: {
line_code: 'ABC_1',
discussions: [],
},
right: {
line_code: 'ABC_1',
discussions: [],
},
},
],
highlighted_diff_lines: [
{
line_code: 'ABC_1',
discussions: [],
},
],
},
],
};
const discussion = {
id: 1,
line_code: 'ABC_2',
line_codes: ['ABC_1'],
diff_discussion: true,
resolvable: true,
original_position: {},
position: {},
positions: [diffPosition],
diff_file: {
file_hash: state.diffFiles[0].file_hash,
},
};
const diffPositionByLineCode = {
ABC_1: diffPosition,
};
mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, {
discussion,
diffPositionByLineCode,
});
expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions).toHaveLength(1);
expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].id).toBe(1);
expect(state.diffFiles[0].highlighted_diff_lines[0].discussions).toHaveLength(1);
expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toBe(1);
});
});
describe('REMOVE_LINE_DISCUSSIONS', () => {
......
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