Read about our upcoming Code of Conduct on this issue

Commit 76f69753 authored by Filipa Lacerda's avatar Filipa Lacerda
Browse files

Merge branch 'fix-duplicate-notes' into 'master'

Fixed issue notes being duplicated

Closes #44099

See merge request gitlab-org/gitlab-ce!17671
......@@ -27,10 +27,11 @@ export default {
return Vue.http[method](endpoint);
},
poll(data = {}) {
const { endpoint, lastFetchedAt } = data;
const endpoint = data.notesData.notesPath;
const lastFetchedAt = data.lastFetchedAt;
const options = {
headers: {
'X-Last-Fetched-At': lastFetchedAt,
'X-Last-Fetched-At': lastFetchedAt ? `${lastFetchedAt}` : undefined,
},
};
......
......@@ -198,18 +198,16 @@ const pollSuccessCallBack = (resp, commit, state, getters) => {
});
}
commit(types.SET_LAST_FETCHED_AT, resp.lastFetchedAt);
commit(types.SET_LAST_FETCHED_AT, resp.last_fetched_at);
return resp;
};
export const poll = ({ commit, state, getters }) => {
const requestData = { endpoint: state.notesData.notesPath, lastFetchedAt: state.lastFetchedAt };
eTagPoll = new Poll({
resource: service,
method: 'poll',
data: requestData,
data: state,
successCallback: resp => resp.json()
.then(data => pollSuccessCallBack(data, commit, state, getters)),
errorCallback: () => Flash('Something went wrong while fetching latest comments.'),
......@@ -218,7 +216,7 @@ export const poll = ({ commit, state, getters }) => {
if (!Visibility.hidden()) {
eTagPoll.makeRequest();
} else {
service.poll(requestData);
service.poll(state);
}
Visibility.change(() => {
......
......@@ -90,19 +90,21 @@ export default {
const notes = [];
notesData.forEach((note) => {
const nn = Object.assign({}, note);
// To support legacy notes, should be very rare case.
if (note.individual_note && note.notes.length > 1) {
note.notes.forEach((n) => {
nn.notes = [n]; // override notes array to only have one item to mimick individual_note
notes.push(nn);
notes.push({
...note,
notes: [n], // override notes array to only have one item to mimick individual_note
});
});
} else {
const oldNote = utils.findNoteObjectById(state.notes, note.id);
nn.expanded = oldNote ? oldNote.expanded : note.expanded;
notes.push(nn);
notes.push({
...note,
expanded: (oldNote ? oldNote.expanded : note.expanded),
});
}
});
......
......@@ -169,7 +169,7 @@ def notes_data(issuable)
reopenPath: reopen_issuable_path(issuable),
notesPath: notes_url,
totalNotes: issuable.discussions.length,
lastFetchedAt: Time.now
lastFetchedAt: Time.now.to_i
}.to_json
end
......
/* eslint-disable */
export const notesDataMock = {
discussionsPath: '/gitlab-org/gitlab-ce/issues/26/discussions.json',
lastFetchedAt: '1501862675',
lastFetchedAt: 1501862675,
markdownDocsPath: '/help/user/markdown',
newSessionPath: '/users/sign_in?redirect_to_referer=yes',
notesPath: '/gitlab-org/gitlab-ce/noteable/issue/98/notes',
......
import Vue from 'vue';
import _ from 'underscore';
import { headersInterceptor } from 'spec/helpers/vue_resource_helper';
import * as actions from '~/notes/stores/actions';
import store from '~/notes/stores';
import testAction from '../../helpers/vuex_action_helper';
......@@ -129,4 +130,68 @@ describe('Actions Notes Store', () => {
], done);
});
});
describe('poll', () => {
beforeEach((done) => {
jasmine.clock().install();
spyOn(Vue.http, 'get').and.callThrough();
store.dispatch('setNotesData', notesDataMock)
.then(done)
.catch(done.fail);
});
afterEach(() => {
jasmine.clock().uninstall();
});
it('calls service with last fetched state', (done) => {
const interceptor = (request, next) => {
next(request.respondWith(JSON.stringify({
notes: [],
last_fetched_at: '123456',
}), {
status: 200,
headers: {
'poll-interval': '1000',
},
}));
};
Vue.http.interceptors.push(interceptor);
Vue.http.interceptors.push(headersInterceptor);
store.dispatch('poll')
.then(() => new Promise(resolve => requestAnimationFrame(resolve)))
.then(() => {
expect(Vue.http.get).toHaveBeenCalledWith(jasmine.anything(), {
url: jasmine.anything(),
method: 'get',
headers: {
'X-Last-Fetched-At': undefined,
},
});
expect(store.state.lastFetchedAt).toBe('123456');
jasmine.clock().tick(1500);
})
.then(() => new Promise((resolve) => {
requestAnimationFrame(resolve);
}))
.then(() => {
expect(Vue.http.get.calls.count()).toBe(2);
expect(Vue.http.get.calls.mostRecent().args[1].headers).toEqual({
'X-Last-Fetched-At': '123456',
});
})
.then(() => store.dispatch('stopPolling'))
.then(() => {
Vue.http.interceptors = _.without(Vue.http.interceptors, interceptor);
Vue.http.interceptors = _.without(Vue.http.interceptors, headersInterceptor);
})
.then(done)
.catch(done.fail);
});
});
});
......@@ -101,10 +101,21 @@ describe('Notes Store mutations', () => {
const state = {
notes: [],
};
const legacyNote = {
id: 2,
individual_note: true,
notes: [{
note: '1',
}, {
note: '2',
}],
};
mutations.SET_INITIAL_NOTES(state, [note]);
mutations.SET_INITIAL_NOTES(state, [note, legacyNote]);
expect(state.notes[0].id).toEqual(note.id);
expect(state.notes.length).toEqual(1);
expect(state.notes[1].notes[0].note).toBe(legacyNote.notes[0].note);
expect(state.notes[2].notes[0].note).toBe(legacyNote.notes[1].note);
expect(state.notes.length).toEqual(3);
});
});
......
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