diff options
author | André Luís <aluis@gitlab.com> | 2018-07-31 01:50:53 +0100 |
---|---|---|
committer | André Luís <aluis@gitlab.com> | 2018-08-01 13:45:15 +0100 |
commit | 8693aa139dcdc5d4ed8939d12b360731f5f9dab8 (patch) | |
tree | a95be3bd3baf011bb43f73879d3c2545619fedd2 /spec | |
parent | 113f9f337c678129e811fcdfb418ba011ac1eb2b (diff) | |
download | gitlab-ce-8693aa139dcdc5d4ed8939d12b360731f5f9dab8.tar.gz |
Revert "Merge branch '48817-fix-mr-changes-discussion-navigation' into 'master'"
This reverts commit ced005f330419ec81657e852c5cb9124fdb29fbb, reversing
changes made to 9b01b293ce5ddbaeedaf014cdc804af2c5e86416.
Diffstat (limited to 'spec')
5 files changed, 28 insertions, 265 deletions
diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb index 2d268ecab58..bf4d5396df9 100644 --- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb +++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb @@ -342,9 +342,8 @@ describe 'Merge request > User resolves diff notes and discussions', :js do end end - it 'shows jump to next discussion button, apart from the last one' do - expect(page).to have_selector('.discussion-reply-holder', count: 2) - expect(page).to have_selector('.discussion-reply-holder .discussion-next-btn', count: 1) + it 'shows jump to next discussion button' do + expect(page.all('.discussion-reply-holder', count: 2)).to all(have_selector('.discussion-next-btn')) end it 'displays next discussion even if hidden' do diff --git a/spec/javascripts/notes/components/discussion_counter_spec.js b/spec/javascripts/notes/components/discussion_counter_spec.js index d09bc5037ef..a3869cc6498 100644 --- a/spec/javascripts/notes/components/discussion_counter_spec.js +++ b/spec/javascripts/notes/components/discussion_counter_spec.js @@ -46,7 +46,7 @@ describe('DiscussionCounter component', () => { discussions, }); setFixtures(` - <div class="discussion" data-discussion-id="${firstDiscussionId}"></div> + <div data-discussion-id="${firstDiscussionId}"></div> `); vm.jumpToFirstUnresolvedDiscussion(); diff --git a/spec/javascripts/notes/components/noteable_discussion_spec.js b/spec/javascripts/notes/components/noteable_discussion_spec.js index 2a01bd85520..f3f50aed232 100644 --- a/spec/javascripts/notes/components/noteable_discussion_spec.js +++ b/spec/javascripts/notes/components/noteable_discussion_spec.js @@ -14,7 +14,6 @@ describe('noteable_discussion component', () => { preloadFixtures(discussionWithTwoUnresolvedNotes); beforeEach(() => { - window.mrTabs = {}; store = createStore(); store.dispatch('setNoteableData', noteableDataMock); store.dispatch('setNotesData', notesDataMock); @@ -107,29 +106,33 @@ describe('noteable_discussion component', () => { describe('methods', () => { describe('jumpToNextDiscussion', () => { - it('expands next unresolved discussion', done => { - const discussion2 = getJSONFixture(discussionWithTwoUnresolvedNotes)[0]; - discussion2.resolved = false; - discussion2.id = 'next'; // prepare this for being identified as next one (to be jumped to) - vm.$store.dispatch('setInitialNotes', [discussionMock, discussion2]); - window.mrTabs.currentAction = 'show'; - - Vue.nextTick() - .then(() => { - spyOn(vm, 'expandDiscussion').and.stub(); - - const nextDiscussionId = discussion2.id; - - setFixtures(` - <div class="discussion" data-discussion-id="${nextDiscussionId}"></div> - `); + it('expands next unresolved discussion', () => { + spyOn(vm, 'expandDiscussion').and.stub(); + const discussions = [ + discussionMock, + { + ...discussionMock, + id: discussionMock.id + 1, + notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: true }], + }, + { + ...discussionMock, + id: discussionMock.id + 2, + notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: false }], + }, + ]; + const nextDiscussionId = discussionMock.id + 2; + store.replaceState({ + ...store.state, + discussions, + }); + setFixtures(` + <div data-discussion-id="${nextDiscussionId}"></div> + `); - vm.jumpToNextDiscussion(); + vm.jumpToNextDiscussion(); - expect(vm.expandDiscussion).toHaveBeenCalledWith({ discussionId: nextDiscussionId }); - }) - .then(done) - .catch(done.fail); + expect(vm.expandDiscussion).toHaveBeenCalledWith({ discussionId: nextDiscussionId }); }); }); }); diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js index 67f6a9629d9..be2a8ba67fe 100644 --- a/spec/javascripts/notes/mock_data.js +++ b/spec/javascripts/notes/mock_data.js @@ -1168,87 +1168,3 @@ export const collapsedSystemNotes = [ diff_discussion: false, }, ]; - -export const discussion1 = { - id: 'abc1', - resolvable: true, - resolved: false, - diff_file: { - file_path: 'about.md', - }, - position: { - formatter: { - new_line: 50, - old_line: null, - }, - }, - notes: [ - { - created_at: '2018-07-04T16:25:41.749Z', - }, - ], -}; - -export const resolvedDiscussion1 = { - id: 'abc1', - resolvable: true, - resolved: true, - diff_file: { - file_path: 'about.md', - }, - position: { - formatter: { - new_line: 50, - old_line: null, - }, - }, - notes: [ - { - created_at: '2018-07-04T16:25:41.749Z', - }, - ], -}; - -export const discussion2 = { - id: 'abc2', - resolvable: true, - resolved: false, - diff_file: { - file_path: 'README.md', - }, - position: { - formatter: { - new_line: null, - old_line: 20, - }, - }, - notes: [ - { - created_at: '2018-07-04T12:05:41.749Z', - }, - ], -}; - -export const discussion3 = { - id: 'abc3', - resolvable: true, - resolved: false, - diff_file: { - file_path: 'README.md', - }, - position: { - formatter: { - new_line: 21, - old_line: null, - }, - }, - notes: [ - { - created_at: '2018-07-05T17:25:41.749Z', - }, - ], -}; - -export const unresolvableDiscussion = { - resolvable: false, -}; diff --git a/spec/javascripts/notes/stores/getters_spec.js b/spec/javascripts/notes/stores/getters_spec.js index 7f8ede51508..41599e00122 100644 --- a/spec/javascripts/notes/stores/getters_spec.js +++ b/spec/javascripts/notes/stores/getters_spec.js @@ -5,11 +5,6 @@ import { noteableDataMock, individualNote, collapseNotesMock, - discussion1, - discussion2, - discussion3, - resolvedDiscussion1, - unresolvableDiscussion, } from '../mock_data'; const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json'; @@ -114,154 +109,4 @@ describe('Getters Notes Store', () => { expect(getters.isNotesFetched(state)).toBeFalsy(); }); }); - - describe('allResolvableDiscussions', () => { - it('should return only resolvable discussions in same order', () => { - const localGetters = { - allDiscussions: [ - discussion3, - unresolvableDiscussion, - discussion1, - unresolvableDiscussion, - discussion2, - ], - }; - - expect(getters.allResolvableDiscussions(state, localGetters)).toEqual([ - discussion3, - discussion1, - discussion2, - ]); - }); - - it('should return empty array if there are no resolvable discussions', () => { - const localGetters = { - allDiscussions: [unresolvableDiscussion, unresolvableDiscussion], - }; - - expect(getters.allResolvableDiscussions(state, localGetters)).toEqual([]); - }); - }); - - describe('unresolvedDiscussionsIdsByDiff', () => { - it('should return all discussions IDs in diff order', () => { - const localGetters = { - allResolvableDiscussions: [discussion3, discussion1, discussion2], - }; - - expect(getters.unresolvedDiscussionsIdsByDiff(state, localGetters)).toEqual([ - 'abc1', - 'abc2', - 'abc3', - ]); - }); - - it('should return empty array if all discussions have been resolved', () => { - const localGetters = { - allResolvableDiscussions: [resolvedDiscussion1], - }; - - expect(getters.unresolvedDiscussionsIdsByDiff(state, localGetters)).toEqual([]); - }); - }); - - describe('unresolvedDiscussionsIdsByDate', () => { - it('should return all discussions in date ascending order', () => { - const localGetters = { - allResolvableDiscussions: [discussion3, discussion1, discussion2], - }; - - expect(getters.unresolvedDiscussionsIdsByDate(state, localGetters)).toEqual([ - 'abc2', - 'abc1', - 'abc3', - ]); - }); - - it('should return empty array if all discussions have been resolved', () => { - const localGetters = { - allResolvableDiscussions: [resolvedDiscussion1], - }; - - expect(getters.unresolvedDiscussionsIdsByDate(state, localGetters)).toEqual([]); - }); - }); - - describe('unresolvedDiscussionsIdsOrdered', () => { - const localGetters = { - unresolvedDiscussionsIdsByDate: ['123', '456'], - unresolvedDiscussionsIdsByDiff: ['abc', 'def'], - }; - - it('should return IDs ordered by diff when diffOrder param is true', () => { - expect(getters.unresolvedDiscussionsIdsOrdered(state, localGetters)(true)).toEqual([ - 'abc', - 'def', - ]); - }); - - it('should return IDs ordered by date when diffOrder param is not true', () => { - expect(getters.unresolvedDiscussionsIdsOrdered(state, localGetters)(false)).toEqual([ - '123', - '456', - ]); - expect(getters.unresolvedDiscussionsIdsOrdered(state, localGetters)(undefined)).toEqual([ - '123', - '456', - ]); - }); - }); - - describe('isLastUnresolvedDiscussion', () => { - const localGetters = { - unresolvedDiscussionsIdsOrdered: () => ['123', '456', '789'], - }; - - it('should return true if the discussion id provided is the last', () => { - expect(getters.isLastUnresolvedDiscussion(state, localGetters)('789')).toBe(true); - }); - - it('should return false if the discussion id provided is not the last', () => { - expect(getters.isLastUnresolvedDiscussion(state, localGetters)('123')).toBe(false); - expect(getters.isLastUnresolvedDiscussion(state, localGetters)('456')).toBe(false); - }); - }); - - describe('nextUnresolvedDiscussionId', () => { - const localGetters = { - unresolvedDiscussionsIdsOrdered: () => ['123', '456', '789'], - }; - - it('should return the ID of the discussion after the ID provided', () => { - expect(getters.nextUnresolvedDiscussionId(state, localGetters)('123')).toBe('456'); - expect(getters.nextUnresolvedDiscussionId(state, localGetters)('456')).toBe('789'); - expect(getters.nextUnresolvedDiscussionId(state, localGetters)('789')).toBe(undefined); - }); - }); - - describe('firstUnresolvedDiscussionId', () => { - const localGetters = { - unresolvedDiscussionsIdsByDate: ['123', '456'], - unresolvedDiscussionsIdsByDiff: ['abc', 'def'], - }; - - it('should return the first discussion id by diff when diffOrder param is true', () => { - expect(getters.firstUnresolvedDiscussionId(state, localGetters)(true)).toBe('abc'); - }); - - it('should return the first discussion id by date when diffOrder param is not true', () => { - expect(getters.firstUnresolvedDiscussionId(state, localGetters)(false)).toBe('123'); - expect(getters.firstUnresolvedDiscussionId(state, localGetters)(undefined)).toBe('123'); - }); - - it('should be falsy if all discussions are resolved', () => { - const localGettersFalsy = { - unresolvedDiscussionsIdsByDiff: [], - unresolvedDiscussionsIdsByDate: [], - }; - - expect(getters.firstUnresolvedDiscussionId(state, localGettersFalsy)(true)).toBeFalsy(); - expect(getters.firstUnresolvedDiscussionId(state, localGettersFalsy)(false)).toBeFalsy(); - }); - }); }); |