Commit e5897481 authored by Justin Boyson's avatar Justin Boyson Committed by Paul Slaughter

Hide comment button if on diff HEAD

Also update specs to `@vue/test-utils`

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24207
parent 6b389148
<script> <script>
import { mapState, mapGetters, mapActions } from 'vuex'; import { mapState, mapGetters, mapActions } from 'vuex';
import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils';
import Icon from '~/vue_shared/components/icon.vue'; import Icon from '~/vue_shared/components/icon.vue';
import DiffGutterAvatars from './diff_gutter_avatars.vue'; import DiffGutterAvatars from './diff_gutter_avatars.vue';
import { LINE_POSITION_RIGHT } from '../constants'; import { LINE_POSITION_RIGHT } from '../constants';
...@@ -98,7 +99,8 @@ export default { ...@@ -98,7 +99,8 @@ export default {
return this.showCommentButton && this.hasDiscussions; return this.showCommentButton && this.hasDiscussions;
}, },
shouldRenderCommentButton() { shouldRenderCommentButton() {
return this.isLoggedIn && this.showCommentButton; const isDiffHead = parseBoolean(getParameterByName('diff_head'));
return !isDiffHead && this.isLoggedIn && this.showCommentButton;
}, },
}, },
methods: { methods: {
...@@ -130,6 +132,7 @@ export default { ...@@ -130,6 +132,7 @@ export default {
</button> </button>
<a <a
v-if="lineNumber" v-if="lineNumber"
ref="lineNumberRef"
:data-linenumber="lineNumber" :data-linenumber="lineNumber"
:href="lineHref" :href="lineHref"
@click="setHighlightedRow(lineCode)" @click="setHighlightedRow(lineCode)"
......
---
title: Hide comment button if on diff HEAD
merge_request: 24207
author:
type: changed
import Vue from 'vue'; import Vuex from 'vuex';
import { createComponentWithStore } from 'helpers/vue_mount_component_helper'; import { shallowMount, createLocalVue } from '@vue/test-utils';
import DiffLineGutterContent from '~/diffs/components/diff_line_gutter_content.vue'; import DiffLineGutterContent from '~/diffs/components/diff_line_gutter_content.vue';
import DiffGutterAvatars from '~/diffs/components/diff_gutter_avatars.vue';
import { LINE_POSITION_RIGHT } from '~/diffs/constants';
import { createStore } from '~/mr_notes/stores'; import { createStore } from '~/mr_notes/stores';
import { TEST_HOST } from 'helpers/test_constants';
import discussionsMockData from '../mock_data/diff_discussions'; import discussionsMockData from '../mock_data/diff_discussions';
import diffFileMockData from '../mock_data/diff_file'; import diffFileMockData from '../mock_data/diff_file';
const localVue = createLocalVue();
localVue.use(Vuex);
const TEST_USER_ID = 'abc123';
const TEST_USER = { id: TEST_USER_ID };
const TEST_LINE_NUMBER = 1;
const TEST_LINE_CODE = 'LC_42';
const TEST_FILE_HASH = diffFileMockData.file_hash;
describe('DiffLineGutterContent', () => { describe('DiffLineGutterContent', () => {
const getDiffFileMock = () => Object.assign({}, diffFileMockData); let wrapper;
const createComponent = (options = {}) => { let line;
const cmp = Vue.extend(DiffLineGutterContent); let store;
const props = Object.assign({}, options);
props.line = { beforeEach(() => {
line_code: 'LC_42', store = createStore();
store.state.notes.userData = TEST_USER;
line = {
line_code: TEST_LINE_CODE,
type: 'new', type: 'new',
old_line: null, old_line: null,
new_line: 1, new_line: 1,
discussions: [{ ...discussionsMockData }], discussions: [{ ...discussionsMockData }],
discussionsExpanded: true,
text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n',
rich_text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n', rich_text: '+<span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n',
meta_data: null, meta_data: null,
}; };
props.fileHash = getDiffFileMock().file_hash; });
props.contextLinesPath = '/context/lines/path';
return createComponentWithStore(cmp, createStore(), props).$mount(); afterEach(() => {
}; wrapper.destroy();
});
describe('computed', () => { const setWindowLocation = value => {
describe('lineHref', () => { Object.defineProperty(window, 'location', {
it('should prepend # to lineCode', () => { writable: true,
const lineCode = 'LC_42'; value,
const component = createComponent(); });
};
const createComponent = (props = {}) => {
wrapper = shallowMount(DiffLineGutterContent, {
localVue,
store,
propsData: {
line,
fileHash: TEST_FILE_HASH,
contextLinesPath: '/context/lines/path',
...props,
},
});
};
const findNoteButton = () => wrapper.find('.js-add-diff-note-button');
const findLineNumber = () => wrapper.find({ ref: 'lineNumberRef' });
const findAvatars = () => wrapper.find(DiffGutterAvatars);
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}
`(
'exists is $expectation - with showCommentButton ($showCommentButton) userData ($userData) query ($query)',
({ showCommentButton, userData, query, expectation }) => {
store.state.notes.userData = userData;
setWindowLocation({ href: `${TEST_HOST}?${query}` });
createComponent({ showCommentButton });
expect(findNoteButton().exists()).toBe(expectation);
},
);
it.each`
isHover | otherProps | discussions | expectation
${true} | ${{}} | ${[]} | ${true}
${false} | ${{}} | ${[]} | ${false}
${true} | ${{ isMatchLine: true }} | ${[]} | ${false}
${true} | ${{ isContextLine: true }} | ${[]} | ${false}
${true} | ${{ isMetaLine: true }} | ${[]} | ${false}
${true} | ${{}} | ${[{}]} | ${false}
`(
'visible is $expectation - with isHover ($isHover), discussions ($discussions), otherProps ($otherProps)',
({ isHover, otherProps, discussions, expectation }) => {
line.discussions = discussions;
createComponent({
showCommentButton: true,
isHover,
...otherProps,
});
expect(component.lineHref).toEqual(`#${lineCode}`); expect(findNoteButton().isVisible()).toBe(expectation);
}); },
);
});
it('should return # if there is no lineCode', () => { describe('line number', () => {
const component = createComponent(); describe('without lineNumber prop', () => {
component.line.line_code = ''; it('does not render', () => {
createComponent();
expect(component.lineHref).toEqual('#'); expect(findLineNumber().exists()).toBe(false);
}); });
}); });
describe('discussions, hasDiscussions, shouldShowAvatarsOnGutter', () => { describe('with lineNumber prop', () => {
it('should return empty array when there is no discussion', () => { describe.each`
const component = createComponent(); lineProps | expectedHref | expectedClickArg
component.line.discussions = []; ${{ line_code: TEST_LINE_CODE }} | ${`#${TEST_LINE_CODE}`} | ${TEST_LINE_CODE}
${{ line_code: undefined }} | ${'#'} | ${undefined}
expect(component.hasDiscussions).toEqual(false); ${{ line_code: undefined, left: { line_code: TEST_LINE_CODE } }} | ${'#'} | ${TEST_LINE_CODE}
expect(component.shouldShowAvatarsOnGutter).toEqual(false); ${{ line_code: undefined, right: { line_code: TEST_LINE_CODE } }} | ${'#'} | ${TEST_LINE_CODE}
}); `('with line ($lineProps)', ({ lineProps, expectedHref, expectedClickArg }) => {
beforeEach(() => {
it('should return discussions for the given lineCode', () => { jest.spyOn(store, 'dispatch').mockImplementation();
const cmp = Vue.extend(DiffLineGutterContent); Object.assign(line, lineProps);
const props = { createComponent({ lineNumber: TEST_LINE_NUMBER });
line: getDiffFileMock().highlighted_diff_lines[1], });
fileHash: getDiffFileMock().file_hash,
showCommentButton: true, it('renders', () => {
contextLinesPath: '/context/lines/path', expect(findLineNumber().exists()).toBe(true);
}; expect(findLineNumber().attributes()).toEqual({
props.line.discussions = [Object.assign({}, discussionsMockData)]; href: expectedHref,
const component = createComponentWithStore(cmp, createStore(), props).$mount(); 'data-linenumber': TEST_LINE_NUMBER.toString(),
});
expect(component.hasDiscussions).toEqual(true); });
expect(component.shouldShowAvatarsOnGutter).toEqual(true);
it('on click, dispatches setHighlightedRow', () => {
expect(store.dispatch).not.toHaveBeenCalled();
findLineNumber().trigger('click');
expect(store.dispatch).toHaveBeenCalledWith('diffs/setHighlightedRow', expectedClickArg);
});
}); });
}); });
}); });
describe('template', () => { describe('diff-gutter-avatars', () => {
it('should render comment button', () => { describe('with showCommentButton', () => {
const component = createComponent({ beforeEach(() => {
showCommentButton: true, jest.spyOn(store, 'dispatch').mockImplementation();
});
Object.defineProperty(component, 'isLoggedIn', { createComponent({ showCommentButton: true });
get() {
return true;
},
}); });
expect(component.$el.querySelector('.js-add-diff-note-button')).toBeDefined(); it('renders', () => {
}); expect(findAvatars().props()).toEqual({
discussions: line.discussions,
discussionsExpanded: line.discussionsExpanded,
});
});
it('should render line link', () => { it('toggles line discussion', () => {
const lineNumber = 42; expect(store.dispatch).not.toHaveBeenCalled();
const lineCode = `LC_${lineNumber}`;
const component = createComponent({ lineNumber, lineCode });
const link = component.$el.querySelector('a');
expect(link.href.indexOf(`#${lineCode}`)).toBeGreaterThan(-1); findAvatars().vm.$emit('toggleLineDiscussions');
expect(link.dataset.linenumber).toEqual(lineNumber.toString());
});
it('should render user avatars', () => { expect(store.dispatch).toHaveBeenCalledWith('diffs/toggleLineDiscussions', {
const component = createComponent({ lineCode: TEST_LINE_CODE,
showCommentButton: true, fileHash: TEST_FILE_HASH,
lineCode: getDiffFileMock().highlighted_diff_lines[1].line_code, expanded: !line.discussionsExpanded,
});
}); });
expect(component.$el.querySelector('.diff-comment-avatar-holders')).not.toBe(null);
}); });
it.each`
props | lineProps | expectation
${{ showCommentButton: true }} | ${{}} | ${true}
${{ showCommentButton: false }} | ${{}} | ${false}
${{ showCommentButton: true, linePosition: LINE_POSITION_RIGHT }} | ${{ type: null }} | ${false}
${{ showCommentButton: true }} | ${{ discussions: [] }} | ${false}
`(
'exists is $expectation - with props ($props), line ($lineProps)',
({ props, lineProps, expectation }) => {
Object.assign(line, lineProps);
createComponent(props);
expect(findAvatars().exists()).toBe(expectation);
},
);
}); });
}); });
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