Commit 10688cfd authored by Natalia Tepluhina's avatar Natalia Tepluhina

Merge branch 'ss/sort-issue' into 'master'

Add toggle to issue/MR discussion sort order

See merge request gitlab-org/gitlab!26929
parents cd2b8ee2 b37b7a29
......@@ -4,6 +4,7 @@ import initNotesApp from './init_notes';
import initDiffsApp from '../diffs';
import discussionCounter from '../notes/components/discussion_counter.vue';
import initDiscussionFilters from '../notes/discussion_filters';
import initSortDiscussions from '../notes/sort_discussions';
import MergeRequest from '../merge_request';
import { resetServiceWorkersPublicPath } from '../lib/utils/webpack';
......@@ -32,5 +33,6 @@ export default function initMrNotes() {
});
initDiscussionFilters(store);
initSortDiscussions(store);
initDiffsApp(store);
}
......@@ -12,6 +12,7 @@ import commentForm from './comment_form.vue';
import placeholderNote from '../../vue_shared/components/notes/placeholder_note.vue';
import placeholderSystemNote from '../../vue_shared/components/notes/placeholder_system_note.vue';
import skeletonLoadingContainer from '../../vue_shared/components/notes/skeleton_note.vue';
import OrderedLayout from '~/vue_shared/components/ordered_layout.vue';
import highlightCurrentUser from '~/behaviors/markdown/highlight_current_user';
import { __ } from '~/locale';
import initUserPopovers from '~/user_popovers';
......@@ -27,6 +28,7 @@ export default {
placeholderSystemNote,
skeletonLoadingContainer,
discussionFilterNote,
OrderedLayout,
},
props: {
noteableData: {
......@@ -70,7 +72,11 @@ export default {
'getNoteableData',
'userCanReply',
'discussionTabCounter',
'sortDirection',
]),
sortDirDesc() {
return this.sortDirection === constants.DESC;
},
discussionTabCounterText() {
return this.isLoading ? '' : this.discussionTabCounter;
},
......@@ -91,6 +97,9 @@ export default {
canReply() {
return this.userCanReply && !this.commentsDisabled;
},
slotKeys() {
return this.sortDirDesc ? ['form', 'comments'] : ['comments', 'form'];
},
},
watch: {
shouldShow() {
......@@ -156,6 +165,9 @@ export default {
'convertToDiscussion',
'stopPolling',
]),
discussionIsIndividualNoteAndNotConverted(discussion) {
return discussion.individual_note && !this.convertedDisscussionIds.includes(discussion.id);
},
handleHashChanged() {
const noteId = this.checkLocationHash();
......@@ -232,44 +244,51 @@ export default {
<template>
<div v-show="shouldShow" id="notes">
<ul id="notes-list" class="notes main-notes-list timeline">
<template v-for="discussion in allDiscussions">
<skeleton-loading-container v-if="discussion.isSkeletonNote" :key="discussion.id" />
<template v-else-if="discussion.isPlaceholderNote">
<placeholder-system-note
v-if="discussion.placeholderType === $options.systemNote"
:key="discussion.id"
:note="discussion.notes[0]"
/>
<placeholder-note v-else :key="discussion.id" :note="discussion.notes[0]" />
</template>
<template
v-else-if="discussion.individual_note && !convertedDisscussionIds.includes(discussion.id)"
>
<system-note
v-if="discussion.notes[0].system"
:key="discussion.id"
:note="discussion.notes[0]"
/>
<noteable-note
v-else
:key="discussion.id"
:note="discussion.notes[0]"
:show-reply-button="canReply"
@startReplying="startReplying(discussion.id)"
/>
</template>
<noteable-discussion
v-else
:key="discussion.id"
:discussion="discussion"
:render-diff-file="true"
:help-page-path="helpPagePath"
<ordered-layout :slot-keys="slotKeys">
<template #form>
<comment-form
v-if="!commentsDisabled"
class="js-comment-form"
:noteable-type="noteableType"
/>
</template>
<discussion-filter-note v-show="commentsDisabled" />
</ul>
<comment-form v-if="!commentsDisabled" :noteable-type="noteableType" />
<template #comments>
<ul id="notes-list" class="notes main-notes-list timeline">
<template v-for="discussion in allDiscussions">
<skeleton-loading-container v-if="discussion.isSkeletonNote" :key="discussion.id" />
<template v-else-if="discussion.isPlaceholderNote">
<placeholder-system-note
v-if="discussion.placeholderType === $options.systemNote"
:key="discussion.id"
:note="discussion.notes[0]"
/>
<placeholder-note v-else :key="discussion.id" :note="discussion.notes[0]" />
</template>
<template v-else-if="discussionIsIndividualNoteAndNotConverted(discussion)">
<system-note
v-if="discussion.notes[0].system"
:key="discussion.id"
:note="discussion.notes[0]"
/>
<noteable-note
v-else
:key="discussion.id"
:note="discussion.notes[0]"
:show-reply-button="canReply"
@startReplying="startReplying(discussion.id)"
/>
</template>
<noteable-discussion
v-else
:key="discussion.id"
:discussion="discussion"
:render-diff-file="true"
:help-page-path="helpPagePath"
/>
</template>
<discussion-filter-note v-show="commentsDisabled" />
</ul>
</template>
</ordered-layout>
</div>
</template>
<script>
import { GlIcon } from '@gitlab/ui';
import { mapActions, mapGetters } from 'vuex';
import { __ } from '~/locale';
import { ASC, DESC } from '../constants';
const SORT_OPTIONS = [
{ key: DESC, text: __('Newest first'), cls: 'js-newest-first' },
{ key: ASC, text: __('Oldest first'), cls: 'js-oldest-first' },
];
export default {
SORT_OPTIONS,
components: {
GlIcon,
},
computed: {
...mapGetters(['sortDirection']),
selectedOption() {
return SORT_OPTIONS.find(({ key }) => this.sortDirection === key);
},
dropdownText() {
return this.selectedOption.text;
},
},
methods: {
...mapActions(['setDiscussionSortDirection']),
fetchSortedDiscussions(direction) {
if (this.isDropdownItemActive(direction)) {
return;
}
this.setDiscussionSortDirection(direction);
},
isDropdownItemActive(sortDir) {
return sortDir === this.sortDirection;
},
},
};
</script>
<template>
<div class="mr-2 d-inline-block align-bottom full-width-mobile">
<button class="btn btn-sm js-dropdown-text" data-toggle="dropdown" aria-expanded="false">
{{ dropdownText }}
<gl-icon name="chevron-down" />
</button>
<div ref="dropdownMenu" class="dropdown-menu dropdown-menu-selectable dropdown-menu-right">
<div class="dropdown-content">
<ul>
<li v-for="{ text, key, cls } in $options.SORT_OPTIONS" :key="key">
<button
:class="[cls, { 'is-active': isDropdownItemActive(key) }]"
type="button"
@click="fetchSortedDiscussions(key)"
>
{{ text }}
</button>
</li>
</ul>
</div>
</div>
</div>
</template>
......@@ -19,6 +19,8 @@ export const DISCUSSION_FILTERS_DEFAULT_VALUE = 0;
export const DISCUSSION_TAB_LABEL = 'show';
export const NOTE_UNDERSCORE = 'note_';
export const TIME_DIFFERENCE_VALUE = 10;
export const ASC = 'asc';
export const DESC = 'desc';
export const NOTEABLE_TYPE_MAPPING = {
Issue: ISSUE_NOTEABLE_TYPE,
......
import Vue from 'vue';
import notesApp from './components/notes_app.vue';
import initDiscussionFilters from './discussion_filters';
import initSortDiscussions from './sort_discussions';
import createStore from './stores';
document.addEventListener('DOMContentLoaded', () => {
......@@ -50,4 +51,5 @@ document.addEventListener('DOMContentLoaded', () => {
});
initDiscussionFilters(store);
initSortDiscussions(store);
});
import Vue from 'vue';
import SortDiscussion from './components/sort_discussion.vue';
export default store => {
const el = document.getElementById('js-vue-sort-issue-discussions');
if (!el) return null;
return new Vue({
el,
store,
render(createElement) {
return createElement(SortDiscussion);
},
});
};
......@@ -69,6 +69,10 @@ export const updateDiscussion = ({ commit, state }, discussion) => {
return utils.findNoteObjectById(state.discussions, discussion.id);
};
export const setDiscussionSortDirection = ({ commit }, direction) => {
commit(types.SET_DISCUSSIONS_SORT, direction);
};
export const removeNote = ({ commit, dispatch, state }, note) => {
const discussion = state.discussions.find(({ id }) => id === note.discussion_id);
......
import { flattenDeep } from 'lodash';
import { flattenDeep, clone } from 'lodash';
import * as constants from '../constants';
import { collapseSystemNotes } from './collapse_utils';
export const discussions = state => collapseSystemNotes(state.discussions);
export const discussions = state => {
let discussionsInState = clone(state.discussions);
// NOTE: not testing bc will be removed when backend is finished.
if (state.discussionSortOrder === constants.DESC) {
discussionsInState = discussionsInState.reverse();
}
return collapseSystemNotes(discussionsInState);
};
export const convertedDisscussionIds = state => state.convertedDisscussionIds;
......@@ -12,6 +20,13 @@ export const getNotesData = state => state.notesData;
export const isNotesFetched = state => state.isNotesFetched;
/*
* WARNING: This is an example of an "unnecessary" getter
* more info found here: https://gitlab.com/groups/gitlab-org/-/epics/2913.
*/
export const sortDirection = state => state.discussionSortOrder;
export const isLoading = state => state.isLoading;
export const getNotesDataByProp = state => prop => state.notesData[prop];
......
import * as actions from '../actions';
import * as getters from '../getters';
import mutations from '../mutations';
import { ASC } from '../../constants';
export default () => ({
state: {
discussions: [],
discussionSortOrder: ASC,
convertedDisscussionIds: [],
targetNoteHash: null,
lastFetchedAt: null,
......
......@@ -27,6 +27,7 @@ export const TOGGLE_DISCUSSION = 'TOGGLE_DISCUSSION';
export const SET_EXPAND_DISCUSSIONS = 'SET_EXPAND_DISCUSSIONS';
export const UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS = 'UPDATE_RESOLVABLE_DISCUSSIONS_COUNTS';
export const SET_CURRENT_DISCUSSION_ID = 'SET_CURRENT_DISCUSSION_ID';
export const SET_DISCUSSIONS_SORT = 'SET_DISCUSSIONS_SORT';
// Issue
export const CLOSE_ISSUE = 'CLOSE_ISSUE';
......
......@@ -263,6 +263,10 @@ export default {
discussion.truncated_diff_lines = utils.prepareDiffLines(diffLines);
},
[types.SET_DISCUSSIONS_SORT](state, sort) {
state.discussionSortOrder = sort;
},
[types.DISABLE_COMMENTS](state, value) {
state.commentsDisabled = value;
},
......
<script>
export default {
functional: true,
render(h, context) {
const { slotKeys } = context.props;
const slots = context.slots();
const children = slotKeys.map(key => slots[key]).filter(x => x);
return children;
},
};
</script>
......@@ -47,6 +47,10 @@ class Projects::IssuesController < Projects::ApplicationController
push_frontend_feature_flag(:save_issuable_health_status, project.group)
end
before_action only: :show do
push_frontend_feature_flag(:sort_discussions, @project)
end
around_action :allow_gitaly_ref_name_caching, only: [:discussions]
respond_to :html
......
......@@ -29,6 +29,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_frontend_feature_flag(:vue_issuable_sidebar, @project.group)
end
before_action only: :show do
push_frontend_feature_flag(:sort_discussions, @project)
end
around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :discussions]
def index
......
......@@ -87,6 +87,8 @@
.col-md-12.col-lg-6.js-noteable-awards
= render 'award_emoji/awards_block', awardable: @issue, inline: true
.col-md-12.col-lg-6.new-branch-col
- if Feature.enabled?(:sort_discussions, @project)
#js-vue-sort-issue-discussions
#js-vue-discussion-filter{ data: { default_filter: current_user&.notes_filter_for(@issue), notes_filters: UserPreference.notes_filters.to_json } }
= render 'new_branch' if show_new_branch_button?
......
......@@ -2,4 +2,6 @@
= render 'award_emoji/awards_block', awardable: @merge_request, inline: true do
- if mr_tabs_position_enabled?
.ml-auto.mt-auto.mb-auto
- if Feature.enabled?(:sort_discussions, @merge_request.target_project)
#js-vue-sort-issue-discussions
= render "projects/merge_requests/discussion_filter"
......@@ -13213,6 +13213,9 @@ msgstr ""
msgid "New..."
msgstr ""
msgid "Newest first"
msgstr ""
msgid "Newly registered users will by default be external"
msgstr ""
......@@ -13693,6 +13696,9 @@ msgstr ""
msgid "Ok let's go"
msgstr ""
msgid "Oldest first"
msgstr ""
msgid "OmniAuth"
msgstr ""
......
import $ from 'jquery';
import AxiosMockAdapter from 'axios-mock-adapter';
import Vue from 'vue';
import { mount } from '@vue/test-utils';
import { mount, shallowMount } from '@vue/test-utils';
import { setTestTimeout } from 'helpers/timeout';
import axios from '~/lib/utils/axios_utils';
import NotesApp from '~/notes/components/notes_app.vue';
import CommentForm from '~/notes/components/comment_form.vue';
import createStore from '~/notes/stores';
import * as constants from '~/notes/constants';
import '~/behaviors/markdown/render_gfm';
// TODO: use generated fixture (https://gitlab.com/gitlab-org/gitlab-foss/issues/62491)
import * as mockData from '../../notes/mock_data';
import * as urlUtility from '~/lib/utils/url_utility';
import OrderedLayout from '~/vue_shared/components/ordered_layout.vue';
jest.mock('~/user_popovers', () => jest.fn());
setTestTimeout(1000);
const TYPE_COMMENT_FORM = 'comment-form';
const TYPE_NOTES_LIST = 'notes-list';
const propsData = {
noteableData: mockData.noteableDataMock,
notesData: mockData.notesDataMock,
userData: mockData.userDataMock,
};
describe('note_app', () => {
let axiosMock;
let mountComponent;
let wrapper;
let store;
const getComponentOrder = () => {
return wrapper
.findAll('#notes-list,.js-comment-form')
.wrappers.map(node => (node.is(CommentForm) ? TYPE_COMMENT_FORM : TYPE_NOTES_LIST));
};
/**
* waits for fetchNotes() to complete
*/
......@@ -43,13 +61,7 @@ describe('note_app', () => {
axiosMock = new AxiosMockAdapter(axios);
store = createStore();
mountComponent = data => {
const propsData = data || {
noteableData: mockData.noteableDataMock,
notesData: mockData.notesDataMock,
userData: mockData.userDataMock,
};
mountComponent = () => {
return mount(
{
components: {
......@@ -346,4 +358,39 @@ describe('note_app', () => {
expect(setTargetNoteHash).toHaveBeenCalled();
});
});
describe('when sort direction is desc', () => {
beforeEach(() => {
store = createStore();
store.state.discussionSortOrder = constants.DESC;
wrapper = shallowMount(NotesApp, {
propsData,
store,
stubs: {
'ordered-layout': OrderedLayout,
},
});
});
it('finds CommentForm before notes list', () => {
expect(getComponentOrder()).toStrictEqual([TYPE_COMMENT_FORM, TYPE_NOTES_LIST]);
});
});
describe('when sort direction is asc', () => {
beforeEach(() => {
store = createStore();
wrapper = shallowMount(NotesApp, {
propsData,
store,
stubs: {
'ordered-layout': OrderedLayout,
},
});
});
it('finds CommentForm after notes list', () => {
expect(getComponentOrder()).toStrictEqual([TYPE_NOTES_LIST, TYPE_COMMENT_FORM]);
});
});
});
import { shallowMount, createLocalVue } from '@vue/test-utils';
import Vuex from 'vuex';
import SortDiscussion from '~/notes/components/sort_discussion.vue';
import createStore from '~/notes/stores';
import { ASC, DESC } from '~/notes/constants';
const localVue = createLocalVue();
localVue.use(Vuex);
describe('Sort Discussion component', () => {
let wrapper;
let store;
const createComponent = () => {
jest.spyOn(store, 'dispatch').mockImplementation();
wrapper = shallowMount(SortDiscussion, {
localVue,
store,
});
};
beforeEach(() => {
store = createStore();
});
afterEach(() => {
wrapper.destroy();
wrapper = null;
});
describe('when asc', () => {
describe('when the dropdown is clicked', () => {
it('calls the right actions', () => {
createComponent();
wrapper.find('.js-newest-first').trigger('click');
expect(store.dispatch).toHaveBeenCalledWith('setDiscussionSortDirection', DESC);
});
});
it('shows the "Oldest First" as the dropdown', () => {
createComponent();
expect(wrapper.find('.js-dropdown-text').text()).toBe('Oldest first');
});
});
describe('when desc', () => {
beforeEach(() => {
store.state.discussionSortOrder = DESC;
createComponent();
});
describe('when the dropdown item is clicked', () => {
it('calls the right actions', () => {
wrapper.find('.js-oldest-first').trigger('click');
expect(store.dispatch).toHaveBeenCalledWith('setDiscussionSortDirection', ASC);
});
it('applies the active class to the correct button in the dropdown', () => {
expect(wrapper.find('.js-newest-first').classes()).toContain('is-active');
});
});
it('shows the "Newest First" as the dropdown', () => {
expect(wrapper.find('.js-dropdown-text').text()).toBe('Newest first');
});
});
});
......@@ -902,4 +902,17 @@ describe('Actions Notes Store', () => {
]);
});
});
describe('setDiscussionSortDirection', () => {
it('calls the correct mutation with the correct args', done => {
testAction(
actions.setDiscussionSortDirection,
notesConstants.DESC,
{},
[{ type: mutationTypes.SET_DISCUSSIONS_SORT, payload: notesConstants.DESC }],
[],
done,
);
});
});
});
import * as getters from '~/notes/stores/getters';
import { DESC } from '~/notes/constants';
import {
notesDataMock,
userDataMock,
......@@ -36,6 +37,7 @@ describe('Getters Notes Store', () => {
userData: userDataMock,
noteableData: noteableDataMock,
descriptionVersions: 'descriptionVersions',
discussionSortOrder: DESC,
};
});
......@@ -392,4 +394,10 @@ describe('Getters Notes Store', () => {
expect(getters.descriptionVersions(state)).toEqual('descriptionVersions');
});
});
describe('sortDirection', () => {
it('should return `discussionSortOrder`', () => {
expect(getters.sortDirection(state)).toBe(DESC);
});
});
});
import Vue from 'vue';
import mutations from '~/notes/stores/mutations';
import { DISCUSSION_NOTE } from '~/notes/constants';
import { DISCUSSION_NOTE, ASC, DESC } from '~/notes/constants';
import {
note,
discussionMock,
......@@ -22,7 +22,10 @@ describe('Notes Store mutations', () => {
let noteData;
beforeEach(() => {
state = { discussions: [] };
state = {
discussions: [],
discussionSortOrder: ASC,
};
noteData = {
expanded: true,
id: note.discussion_id,
......@@ -34,9 +37,7 @@ describe('Notes Store mutations', () => {
});
it('should add a new note to an array of notes', () => {
expect(state).toEqual({
discussions: [noteData],
});
expect(state).toEqual(expect.objectContaining({ discussions: [noteData] }));
expect(state.discussions.length).toBe(1);
});
......@@ -649,4 +650,18 @@ describe('Notes Store mutations', () => {
expect(state.descriptionVersions[versionId]).toBe(deleted);
});
});
describe('SET_DISCUSSIONS_SORT', () => {
let state;
beforeEach(() => {
state = { discussionSortOrder: ASC };
});
it('sets sort order', () => {
mutations.SET_DISCUSSIONS_SORT(state, DESC);
expect(state.discussionSortOrder).toBe(DESC);
});
});
});
import { mount } from '@vue/test-utils';
import orderedLayout from '~/vue_shared/components/ordered_layout.vue';
const children = `
<template v-slot:header>
<header></header>
</template>
<template v-slot:footer>
<footer></footer>
</template>
`;
const TestComponent = {
components: { orderedLayout },
template: `
<div>
<ordered-layout v-bind="$attrs">
${children}
</ordered-layout>
</div>
`,
};
const regularSlotOrder = ['header', 'footer'];
describe('Ordered Layout', () => {
let wrapper;
const verifyOrder = () =>
wrapper.findAll('footer,header').wrappers.map(x => (x.is('footer') ? 'footer' : 'header'));
const createComponent = (props = {}) => {
wrapper = mount(TestComponent, {
propsData: props,
});
};
afterEach(() => {
wrapper.destroy();
});
describe('when slotKeys are in initial slot order', () => {
beforeEach(() => {
createComponent({ slotKeys: regularSlotOrder });
});
it('confirms order of the component is reflective of slotKeys', () => {
expect(verifyOrder()).toEqual(regularSlotOrder);
});
});
describe('when slotKeys reverse the order of the props', () => {
const reversedSlotOrder = regularSlotOrder.reverse();
beforeEach(() => {
createComponent({ slotKeys: reversedSlotOrder });
});
it('confirms order of the component is reflective of slotKeys', () => {
expect(verifyOrder()).toEqual(reversedSlotOrder);
});
});
});
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