Commit 911701ae authored by Paul Gascou-Vaillancourt's avatar Paul Gascou-Vaillancourt Committed by Phil Hughes

Extract discussion notes into new component

- Moved discussion notes out of `NoteableDiscussion` component into a
new `DiscussionNotes` component
- Wrote Jest tests for the new `DiscussionNotes` component
- Updated Jest config for emojis fixtures
- Updated Karma tests `NoteableDiscussion` to match its new structure
- Convert `DiffDiscussions` tests to use Vue test utils
parent 9de3130e
<script>
import { mapGetters } from 'vuex';
import { SYSTEM_NOTE } from '../constants';
import { __ } from '~/locale';
import NoteableNote from './noteable_note.vue';
import PlaceholderNote from '../../vue_shared/components/notes/placeholder_note.vue';
import PlaceholderSystemNote from '../../vue_shared/components/notes/placeholder_system_note.vue';
import SystemNote from '~/vue_shared/components/notes/system_note.vue';
import ToggleRepliesWidget from './toggle_replies_widget.vue';
import NoteEditedText from './note_edited_text.vue';
export default {
name: 'DiscussionNotes',
components: {
ToggleRepliesWidget,
NoteEditedText,
},
props: {
discussion: {
type: Object,
required: true,
},
isExpanded: {
type: Boolean,
required: false,
default: false,
},
diffLine: {
type: Object,
required: false,
default: null,
},
line: {
type: Object,
required: false,
default: null,
},
shouldGroupReplies: {
type: Boolean,
required: false,
default: false,
},
helpPagePath: {
type: String,
required: false,
default: '',
},
},
computed: {
...mapGetters(['userCanReply']),
hasReplies() {
return !!this.replies.length;
},
replies() {
return this.discussion.notes.slice(1);
},
firstNote() {
return this.discussion.notes.slice(0, 1)[0];
},
resolvedText() {
return this.discussion.resolved_by_push ? __('Automatically resolved') : __('Resolved');
},
commit() {
if (!this.discussion.for_commit) {
return null;
}
return {
id: this.discussion.commit_id,
url: this.discussion.discussion_path,
};
},
},
methods: {
componentName(note) {
if (note.isPlaceholderNote) {
if (note.placeholderType === SYSTEM_NOTE) {
return PlaceholderSystemNote;
}
return PlaceholderNote;
}
if (note.system) {
return SystemNote;
}
return NoteableNote;
},
componentData(note) {
return note.isPlaceholderNote ? note.notes[0] : note;
},
},
};
</script>
<template>
<div class="discussion-notes">
<ul class="notes">
<template v-if="shouldGroupReplies">
<component
:is="componentName(firstNote)"
:note="componentData(firstNote)"
:line="line"
:commit="commit"
:help-page-path="helpPagePath"
:show-reply-button="userCanReply"
@handle-delete-note="$emit('deleteNote')"
@start-replying="$emit('startReplying')"
>
<note-edited-text
v-if="discussion.resolved"
slot="discussion-resolved-text"
:edited-at="discussion.resolved_at"
:edited-by="discussion.resolved_by"
:action-text="resolvedText"
class-name="discussion-headline-light js-discussion-headline discussion-resolved-text"
/>
<slot slot="avatar-badge" name="avatar-badge"></slot>
</component>
<toggle-replies-widget
v-if="hasReplies"
:collapsed="!isExpanded"
:replies="replies"
@toggle="$emit('toggleDiscussion')"
/>
<template v-if="isExpanded">
<component
:is="componentName(note)"
v-for="note in replies"
:key="note.id"
:note="componentData(note)"
:help-page-path="helpPagePath"
:line="line"
@handle-delete-note="$emit('deleteNote')"
/>
</template>
</template>
<template v-else>
<component
:is="componentName(note)"
v-for="(note, index) in discussion.notes"
:key="note.id"
:note="componentData(note)"
:help-page-path="helpPagePath"
:line="diffLine"
@handle-delete-note="$emit('deleteNote')"
>
<slot v-if="index === 0" slot="avatar-badge" name="avatar-badge"></slot>
</component>
</template>
</ul>
<slot :show-replies="isExpanded || !hasReplies" name="footer"></slot>
</div>
</template>
...@@ -67,6 +67,7 @@ export default { ...@@ -67,6 +67,7 @@ export default {
'isLoading', 'isLoading',
'commentsDisabled', 'commentsDisabled',
'getNoteableData', 'getNoteableData',
'userCanReply',
]), ]),
noteableType() { noteableType() {
return this.noteableData.noteableType; return this.noteableData.noteableType;
...@@ -83,7 +84,7 @@ export default { ...@@ -83,7 +84,7 @@ export default {
return this.discussions; return this.discussions;
}, },
canReply() { canReply() {
return this.getNoteableData.current_user.can_create_note && !this.commentsDisabled; return this.userCanReply && !this.commentsDisabled;
}, },
}, },
watch: { watch: {
......
...@@ -20,6 +20,8 @@ export const getNoteableData = state => state.noteableData; ...@@ -20,6 +20,8 @@ export const getNoteableData = state => state.noteableData;
export const getNoteableDataByProp = state => prop => state.noteableData[prop]; export const getNoteableDataByProp = state => prop => state.noteableData[prop];
export const userCanReply = state => !!state.noteableData.current_user.can_create_note;
export const openState = state => state.noteableData.state; export const openState = state => state.noteableData.state;
export const getUserData = state => state.userData || {}; export const getUserData = state => state.userData || {};
......
---
title: Extract DiscussionNotes component from NoteableDiscussion
merge_request: 27066
author:
type: other
...@@ -24,6 +24,7 @@ module.exports = { ...@@ -24,6 +24,7 @@ module.exports = {
'^helpers(/.*)$': '<rootDir>/spec/frontend/helpers$1', '^helpers(/.*)$': '<rootDir>/spec/frontend/helpers$1',
'^vendor(/.*)$': '<rootDir>/vendor/assets/javascripts$1', '^vendor(/.*)$': '<rootDir>/vendor/assets/javascripts$1',
'\\.(jpg|jpeg|png|svg)$': '<rootDir>/spec/frontend/__mocks__/file_mock.js', '\\.(jpg|jpeg|png|svg)$': '<rootDir>/spec/frontend/__mocks__/file_mock.js',
'emojis(/.*).json': '<rootDir>/fixtures/emojis$1.json',
}, },
collectCoverageFrom: ['<rootDir>/app/assets/javascripts/**/*.{js,vue}'], collectCoverageFrom: ['<rootDir>/app/assets/javascripts/**/*.{js,vue}'],
coverageDirectory: '<rootDir>/coverage-frontend/', coverageDirectory: '<rootDir>/coverage-frontend/',
......
import { mount, createLocalVue } from '@vue/test-utils';
import '~/behaviors/markdown/render_gfm';
import { SYSTEM_NOTE } from '~/notes/constants';
import DiscussionNotes from '~/notes/components/discussion_notes.vue';
import NoteableNote from '~/notes/components/noteable_note.vue';
import PlaceholderNote from '~/vue_shared/components/notes/placeholder_note.vue';
import PlaceholderSystemNote from '~/vue_shared/components/notes/placeholder_system_note.vue';
import SystemNote from '~/vue_shared/components/notes/system_note.vue';
import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue';
import createStore from '~/notes/stores';
import {
noteableDataMock,
discussionMock,
notesDataMock,
} from '../../../javascripts/notes/mock_data';
const localVue = createLocalVue();
describe('DiscussionNotes', () => {
let wrapper;
const createComponent = props => {
const store = createStore();
store.dispatch('setNoteableData', noteableDataMock);
store.dispatch('setNotesData', notesDataMock);
wrapper = mount(DiscussionNotes, {
localVue,
store,
propsData: {
discussion: discussionMock,
isExpanded: false,
shouldGroupReplies: false,
...props,
},
scopedSlots: {
footer: '<p slot-scope="{ showReplies }">showReplies:{{showReplies}}</p>',
},
slots: {
'avatar-badge': '<span class="avatar-badge-slot-content" />',
},
sync: false,
});
};
afterEach(() => {
wrapper.destroy();
});
describe('rendering', () => {
it('renders an element for each note in the discussion', () => {
createComponent();
const notesCount = discussionMock.notes.length;
const els = wrapper.findAll(TimelineEntryItem);
expect(els.length).toBe(notesCount);
});
it('renders one element if replies groupping is enabled', () => {
createComponent({ shouldGroupReplies: true });
const els = wrapper.findAll(TimelineEntryItem);
expect(els.length).toBe(1);
});
it('uses proper component to render each note type', () => {
const discussion = { ...discussionMock };
const notesData = [
// PlaceholderSystemNote
{
id: 1,
isPlaceholderNote: true,
placeholderType: SYSTEM_NOTE,
notes: [{ body: 'PlaceholderSystemNote' }],
},
// PlaceholderNote
{
id: 2,
isPlaceholderNote: true,
notes: [{ body: 'PlaceholderNote' }],
},
// SystemNote
{
id: 3,
system: true,
note: 'SystemNote',
},
// NoteableNote
discussion.notes[0],
];
discussion.notes = notesData;
createComponent({ discussion });
const notes = wrapper.findAll('.notes > li');
expect(notes.at(0).is(PlaceholderSystemNote)).toBe(true);
expect(notes.at(1).is(PlaceholderNote)).toBe(true);
expect(notes.at(2).is(SystemNote)).toBe(true);
expect(notes.at(3).is(NoteableNote)).toBe(true);
});
it('renders footer scoped slot with showReplies === true when expanded', () => {
createComponent({ isExpanded: true });
expect(wrapper.text()).toMatch('showReplies:true');
});
it('renders footer scoped slot with showReplies === false when collapsed', () => {
createComponent({ isExpanded: false });
expect(wrapper.text()).toMatch('showReplies:false');
});
it('passes down avatar-badge slot content', () => {
createComponent();
expect(wrapper.find('.avatar-badge-slot-content').exists()).toBe(true);
});
});
describe('componentData', () => {
beforeEach(() => {
createComponent();
});
it('should return first note object for placeholder note', () => {
const data = {
isPlaceholderNote: true,
notes: [{ body: 'hello world!' }],
};
const note = wrapper.vm.componentData(data);
expect(note).toEqual(data.notes[0]);
});
it('should return given note for nonplaceholder notes', () => {
const data = {
notes: [{ id: 12 }],
};
const note = wrapper.vm.componentData(data);
expect(note).toEqual(data);
});
});
});
import Vue from 'vue'; import { mount, createLocalVue } from '@vue/test-utils';
import DiffDiscussions from '~/diffs/components/diff_discussions.vue'; import DiffDiscussions from '~/diffs/components/diff_discussions.vue';
import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue';
import NoteableDiscussion from '~/notes/components/noteable_discussion.vue';
import DiscussionNotes from '~/notes/components/discussion_notes.vue';
import Icon from '~/vue_shared/components/icon.vue';
import { createStore } from '~/mr_notes/stores'; import { createStore } from '~/mr_notes/stores';
import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper';
import '~/behaviors/markdown/render_gfm'; import '~/behaviors/markdown/render_gfm';
import discussionsMockData from '../mock_data/diff_discussions'; import discussionsMockData from '../mock_data/diff_discussions';
const localVue = createLocalVue();
describe('DiffDiscussions', () => { describe('DiffDiscussions', () => {
let vm; let store;
let wrapper;
const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)]; const getDiscussionsMockData = () => [Object.assign({}, discussionsMockData)];
function createComponent(props = {}) { const createComponent = props => {
const store = createStore(); store = createStore();
wrapper = mount(localVue.extend(DiffDiscussions), {
vm = createComponentWithStore(Vue.extend(DiffDiscussions), store, { store,
discussions: getDiscussionsMockData(), propsData: {
...props, discussions: getDiscussionsMockData(),
}).$mount(); ...props,
} },
localVue,
sync: false,
});
};
afterEach(() => { afterEach(() => {
vm.$destroy(); wrapper.destroy();
}); });
describe('template', () => { describe('template', () => {
it('should have notes list', () => { it('should have notes list', () => {
createComponent(); createComponent();
expect(vm.$el.querySelectorAll('.discussion .note.timeline-entry').length).toEqual(5); expect(wrapper.find(NoteableDiscussion).exists()).toBe(true);
expect(wrapper.find(DiscussionNotes).exists()).toBe(true);
expect(wrapper.find(DiscussionNotes).findAll(TimelineEntryItem).length).toBe(
discussionsMockData.notes.length,
);
}); });
}); });
describe('image commenting', () => { describe('image commenting', () => {
const findDiffNotesToggle = () => wrapper.find('.js-diff-notes-toggle');
it('renders collapsible discussion button', () => { it('renders collapsible discussion button', () => {
createComponent({ shouldCollapseDiscussions: true }); createComponent({ shouldCollapseDiscussions: true });
const diffNotesToggle = findDiffNotesToggle();
expect(vm.$el.querySelector('.js-diff-notes-toggle')).not.toBe(null); expect(diffNotesToggle.exists()).toBe(true);
expect(vm.$el.querySelector('.js-diff-notes-toggle svg')).not.toBe(null); expect(diffNotesToggle.find(Icon).exists()).toBe(true);
expect(vm.$el.querySelector('.js-diff-notes-toggle').classList).toContain( expect(diffNotesToggle.classes('diff-notes-collapse')).toBe(true);
'diff-notes-collapse',
);
}); });
it('dispatches toggleDiscussion when clicking collapse button', () => { it('dispatches toggleDiscussion when clicking collapse button', () => {
createComponent({ shouldCollapseDiscussions: true }); createComponent({ shouldCollapseDiscussions: true });
spyOn(wrapper.vm.$store, 'dispatch').and.stub();
const diffNotesToggle = findDiffNotesToggle();
diffNotesToggle.trigger('click');
spyOn(vm.$store, 'dispatch').and.stub(); expect(wrapper.vm.$store.dispatch).toHaveBeenCalledWith('toggleDiscussion', {
discussionId: discussionsMockData.id,
vm.$el.querySelector('.js-diff-notes-toggle').click();
expect(vm.$store.dispatch).toHaveBeenCalledWith('toggleDiscussion', {
discussionId: vm.discussions[0].id,
}); });
}); });
it('renders expand button when discussion is collapsed', done => { it('renders expand button when discussion is collapsed', () => {
createComponent({ shouldCollapseDiscussions: true }); const discussions = getDiscussionsMockData();
discussions[0].expanded = false;
vm.discussions[0].expanded = false; createComponent({ discussions, shouldCollapseDiscussions: true });
const diffNotesToggle = findDiffNotesToggle();
vm.$nextTick(() => {
expect(vm.$el.querySelector('.js-diff-notes-toggle').textContent.trim()).toBe('1');
expect(vm.$el.querySelector('.js-diff-notes-toggle').className).toContain(
'btn-transparent badge badge-pill',
);
done(); expect(diffNotesToggle.text().trim()).toBe('1');
}); expect(diffNotesToggle.classes()).toEqual(
jasmine.arrayContaining(['btn-transparent', 'badge', 'badge-pill']),
);
}); });
it('hides discussion when collapsed', done => { it('hides discussion when collapsed', () => {
createComponent({ shouldCollapseDiscussions: true }); const discussions = getDiscussionsMockData();
discussions[0].expanded = false;
createComponent({ discussions, shouldCollapseDiscussions: true });
vm.discussions[0].expanded = false; expect(wrapper.find(NoteableDiscussion).isVisible()).toBe(false);
vm.$nextTick(() => {
expect(vm.$el.querySelector('.note-discussion').style.display).toBe('none');
done();
});
}); });
it('renders badge on avatar', () => { it('renders badge on avatar', () => {
createComponent({ renderAvatarBadge: true, discussions: [{ ...discussionsMockData }] }); createComponent({ renderAvatarBadge: true });
const noteableDiscussion = wrapper.find(NoteableDiscussion);
expect(vm.$el.querySelector('.user-avatar-link .badge-pill')).not.toBe(null);
expect(vm.$el.querySelector('.user-avatar-link .badge-pill').textContent.trim()).toBe('1'); expect(noteableDiscussion.find('.badge-pill').exists()).toBe(true);
expect(
noteableDiscussion
.find('.badge-pill')
.text()
.trim(),
).toBe('1');
}); });
}); });
}); });
...@@ -130,29 +130,6 @@ describe('noteable_discussion component', () => { ...@@ -130,29 +130,6 @@ describe('noteable_discussion component', () => {
}); });
}); });
describe('componentData', () => {
it('should return first note object for placeholder note', () => {
const data = {
isPlaceholderNote: true,
notes: [{ body: 'hello world!' }],
};
const note = wrapper.vm.componentData(data);
expect(note).toEqual(data.notes[0]);
});
it('should return given note for nonplaceholder notes', () => {
const data = {
notes: [{ id: 12 }],
};
const note = wrapper.vm.componentData(data);
expect(note).toEqual(data);
});
});
describe('action text', () => { describe('action text', () => {
const commitId = 'razupaltuff'; const commitId = 'razupaltuff';
const truncatedCommitId = commitId.substr(0, 8); const truncatedCommitId = commitId.substr(0, 8);
......
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