Commit 9dc6b039 authored by Simon Knox's avatar Simon Knox Committed by Fatih Acet

Resolve "MRs are showing some discussions as unresolved"

parent 5e70a094
...@@ -85,9 +85,9 @@ export const allDiscussions = (state, getters) => { ...@@ -85,9 +85,9 @@ export const allDiscussions = (state, getters) => {
export const resolvedDiscussionsById = state => { export const resolvedDiscussionsById = state => {
const map = {}; const map = {};
state.discussions.forEach(n => { state.discussions.filter(d => d.resolvable).forEach(n => {
if (n.notes) { if (n.notes) {
const resolved = n.notes.every(note => note.resolved && !note.system); const resolved = n.notes.filter(note => note.resolvable).every(note => note.resolved);
if (resolved) { if (resolved) {
map[n.id] = n; map[n.id] = n;
......
...@@ -80,6 +80,13 @@ describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: :cont ...@@ -80,6 +80,13 @@ describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: :cont
render_discussions_json(merge_request, example.description) render_discussions_json(merge_request, example.description)
end end
it 'merge_requests/resolved_diff_discussion.json' do |example|
note = create(:discussion_note_on_merge_request, :resolved, project: project, author: admin, position: position, noteable: merge_request)
create(:system_note, project: project, author: admin, noteable: merge_request, discussion_id: note.discussion.id)
render_discussions_json(merge_request, example.description)
end
context 'with image diff' do context 'with image diff' do
let(:merge_request2) { create(:merge_request_with_diffs, :with_image_diffs, source_project: project, title: "Added images") } let(:merge_request2) { create(:merge_request_with_diffs, :with_image_diffs, source_project: project, title: "Added images") }
let(:image_path) { "files/images/ee_repo_logo.png" } let(:image_path) { "files/images/ee_repo_logo.png" }
......
...@@ -32,12 +32,12 @@ describe('DiscussionCounter component', () => { ...@@ -32,12 +32,12 @@ describe('DiscussionCounter component', () => {
{ {
...discussionMock, ...discussionMock,
id: discussionMock.id, id: discussionMock.id,
notes: [{ ...discussionMock.notes[0], resolved: true }], notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: true }],
}, },
{ {
...discussionMock, ...discussionMock,
id: discussionMock.id + 1, id: discussionMock.id + 1,
notes: [{ ...discussionMock.notes[0], resolved: false }], notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: false }],
}, },
]; ];
const firstDiscussionId = discussionMock.id + 1; const firstDiscussionId = discussionMock.id + 1;
......
...@@ -4,22 +4,23 @@ import noteableDiscussion from '~/notes/components/noteable_discussion.vue'; ...@@ -4,22 +4,23 @@ import noteableDiscussion from '~/notes/components/noteable_discussion.vue';
import '~/behaviors/markdown/render_gfm'; import '~/behaviors/markdown/render_gfm';
import { noteableDataMock, discussionMock, notesDataMock } from '../mock_data'; import { noteableDataMock, discussionMock, notesDataMock } from '../mock_data';
const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json';
describe('noteable_discussion component', () => { describe('noteable_discussion component', () => {
const Component = Vue.extend(noteableDiscussion);
let store; let store;
let vm; let vm;
beforeEach(() => { preloadFixtures(discussionWithTwoUnresolvedNotes);
const Component = Vue.extend(noteableDiscussion);
beforeEach(() => {
store = createStore(); store = createStore();
store.dispatch('setNoteableData', noteableDataMock); store.dispatch('setNoteableData', noteableDataMock);
store.dispatch('setNotesData', notesDataMock); store.dispatch('setNotesData', notesDataMock);
vm = new Component({ vm = new Component({
store, store,
propsData: { propsData: { discussion: discussionMock },
discussion: discussionMock,
},
}).$mount(); }).$mount();
}); });
...@@ -84,7 +85,9 @@ describe('noteable_discussion component', () => { ...@@ -84,7 +85,9 @@ describe('noteable_discussion component', () => {
}); });
it('is true if there are two unresolved discussions', done => { it('is true if there are two unresolved discussions', done => {
spyOnProperty(vm, 'unresolvedDiscussions').and.returnValue([{}, {}]); const discussion = getJSONFixture(discussionWithTwoUnresolvedNotes)[0];
discussion.notes[0].resolved = false;
vm.$store.dispatch('setInitialNotes', [discussion, discussion]);
Vue.nextTick() Vue.nextTick()
.then(() => { .then(() => {
...@@ -105,12 +108,12 @@ describe('noteable_discussion component', () => { ...@@ -105,12 +108,12 @@ describe('noteable_discussion component', () => {
{ {
...discussionMock, ...discussionMock,
id: discussionMock.id + 1, id: discussionMock.id + 1,
notes: [{ ...discussionMock.notes[0], resolved: true }], notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: true }],
}, },
{ {
...discussionMock, ...discussionMock,
id: discussionMock.id + 2, id: discussionMock.id + 2,
notes: [{ ...discussionMock.notes[0], resolved: false }], notes: [{ ...discussionMock.notes[0], resolvable: true, resolved: false }],
}, },
]; ];
const nextDiscussionId = discussionMock.id + 2; const nextDiscussionId = discussionMock.id + 2;
......
...@@ -303,6 +303,7 @@ export const discussionMock = { ...@@ -303,6 +303,7 @@ export const discussionMock = {
}, },
], ],
individual_note: false, individual_note: false,
resolvable: true,
}; };
export const loggedOutnoteableData = { export const loggedOutnoteableData = {
......
...@@ -7,9 +7,13 @@ import { ...@@ -7,9 +7,13 @@ import {
collapseNotesMock, collapseNotesMock,
} from '../mock_data'; } from '../mock_data';
const discussionWithTwoUnresolvedNotes = 'merge_requests/resolved_diff_discussion.json';
describe('Getters Notes Store', () => { describe('Getters Notes Store', () => {
let state; let state;
preloadFixtures(discussionWithTwoUnresolvedNotes);
beforeEach(() => { beforeEach(() => {
state = { state = {
discussions: [individualNote], discussions: [individualNote],
...@@ -22,12 +26,26 @@ describe('Getters Notes Store', () => { ...@@ -22,12 +26,26 @@ describe('Getters Notes Store', () => {
noteableData: noteableDataMock, noteableData: noteableDataMock,
}; };
}); });
describe('discussions', () => { describe('discussions', () => {
it('should return all discussions in the store', () => { it('should return all discussions in the store', () => {
expect(getters.discussions(state)).toEqual([individualNote]); expect(getters.discussions(state)).toEqual([individualNote]);
}); });
}); });
describe('resolvedDiscussionsById', () => {
it('ignores unresolved system notes', () => {
const [discussion] = getJSONFixture(discussionWithTwoUnresolvedNotes);
discussion.notes[0].resolved = true;
discussion.notes[1].resolved = false;
state.discussions.push(discussion);
expect(getters.resolvedDiscussionsById(state)).toEqual({
[discussion.id]: discussion,
});
});
});
describe('Collapsed notes', () => { describe('Collapsed notes', () => {
const stateCollapsedNotes = { const stateCollapsedNotes = {
discussions: collapseNotesMock, discussions: collapseNotesMock,
......
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