Automatic merge of gitlab-org/gitlab-ce master

app/assets/javascripts/notes/components/noteable_discussion.vue

@@ -103,6 +103,7 @@ export default {
   },
   computed: {
     ...mapGetters([
+      'convertedDisscussionIds',
       'getNoteableData',
       'nextUnresolvedDiscussionId',
       'unresolvedDiscussionsCount',
@@ -311,6 +312,10 @@ export default {
         note: { note: noteText },
       };
 
+      if (this.convertedDisscussionIds.includes(this.discussion.id)) {
+        postData.return_discussion = true;
+      }
+
       if (this.discussion.for_commit) {
         postData.note_project_id = this.discussion.project_id;
       }

app/assets/javascripts/notes/components/notes_app.vue

@@ -60,6 +60,7 @@ export default {
     ...mapGetters([
       'isNotesFetched',
       'discussions',
+      'convertedDisscussionIds',
       'getNotesDataByProp',
       'isLoading',
       'commentsDisabled',
@@ -193,7 +194,9 @@ export default {
           />
           <placeholder-note v-else :key="discussion.id" :note="discussion.notes[0]" />
         </template>
-        <template v-else-if="discussion.individual_note">
+        <template
+          v-else-if="discussion.individual_note && !convertedDisscussionIds.includes(discussion.id)"
+        >
           <system-note
             v-if="discussion.notes[0].system"
             :key="discussion.id"

app/assets/javascripts/notes/stores/actions.js

@@ -83,12 +83,44 @@ export const updateNote = ({ commit, dispatch }, { endpoint, note }) =>
       dispatch('startTaskList');
     });
 
-export const replyToDiscussion = ({ commit }, { endpoint, data }) =>
+export const updateOrCreateNotes = ({ commit, state, getters, dispatch }, notes) => {
+  const { notesById } = getters;
+
+  notes.forEach(note => {
+    if (notesById[note.id]) {
+      commit(types.UPDATE_NOTE, note);
+    } else if (note.type === constants.DISCUSSION_NOTE || note.type === constants.DIFF_NOTE) {
+      const discussion = utils.findNoteObjectById(state.discussions, note.discussion_id);
+
+      if (discussion) {
+        commit(types.ADD_NEW_REPLY_TO_DISCUSSION, note);
+      } else if (note.type === constants.DIFF_NOTE) {
+        dispatch('fetchDiscussions', { path: state.notesData.discussionsPath });
+      } else {
+        commit(types.ADD_NEW_NOTE, note);
+      }
+    } else {
+      commit(types.ADD_NEW_NOTE, note);
+    }
+  });
+};
+
+export const replyToDiscussion = ({ commit, state, getters, dispatch }, { endpoint, data }) =>
   service
     .replyToDiscussion(endpoint, data)
     .then(res => res.json())
     .then(res => {
-      commit(types.ADD_NEW_REPLY_TO_DISCUSSION, res);
+      if (res.discussion) {
+        commit(types.UPDATE_DISCUSSION, res.discussion);
+
+        updateOrCreateNotes({ commit, state, getters, dispatch }, res.discussion.notes);
+
+        dispatch('updateMergeRequestWidget');
+        dispatch('startTaskList');
+        dispatch('updateResolvableDiscussonsCounts');
+      } else {
+        commit(types.ADD_NEW_REPLY_TO_DISCUSSION, res);
+      }
 
       return res;
     });
@@ -262,25 +294,7 @@ export const saveNote = ({ commit, dispatch }, noteData) => {
 
 const pollSuccessCallBack = (resp, commit, state, getters, dispatch) => {
   if (resp.notes && resp.notes.length) {
-    const { notesById } = getters;
-
-    resp.notes.forEach(note => {
-      if (notesById[note.id]) {
-        commit(types.UPDATE_NOTE, note);
-      } else if (note.type === constants.DISCUSSION_NOTE || note.type === constants.DIFF_NOTE) {
-        const discussion = utils.findNoteObjectById(state.discussions, note.discussion_id);
-
-        if (discussion) {
-          commit(types.ADD_NEW_REPLY_TO_DISCUSSION, note);
-        } else if (note.type === constants.DIFF_NOTE) {
-          dispatch('fetchDiscussions', { path: state.notesData.discussionsPath });
-        } else {
-          commit(types.ADD_NEW_NOTE, note);
-        }
-      } else {
-        commit(types.ADD_NEW_NOTE, note);
-      }
-    });
+    updateOrCreateNotes({ commit, state, getters, dispatch }, resp.notes);
 
     dispatch('startTaskList');
   }

app/assets/javascripts/notes/stores/getters.js

@@ -4,6 +4,8 @@ import { collapseSystemNotes } from './collapse_utils';
 
 export const discussions = state => collapseSystemNotes(state.discussions);
 
+export const convertedDisscussionIds = state => state.convertedDisscussionIds;
+
 export const targetNoteHash = state => state.targetNoteHash;
 
 export const getNotesData = state => state.notesData;

app/assets/javascripts/notes/stores/modules/index.js

@@ -5,6 +5,7 @@ import mutations from '../mutations';
 export default () => ({
   state: {
     discussions: [],
+    convertedDisscussionIds: [],
     targetNoteHash: null,
     lastFetchedAt: null,
 

app/assets/javascripts/notes/stores/mutations.js

@@ -266,7 +266,7 @@ export default {
   },
 
   [types.CONVERT_TO_DISCUSSION](state, discussionId) {
-    const discussion = utils.findNoteObjectById(state.discussions, discussionId);
-    Object.assign(discussion, { individual_note: false });
+    const convertedDisscussionIds = [...state.convertedDisscussionIds, discussionId];
+    Object.assign(state, { convertedDisscussionIds });
   },
 };

changelogs/unreleased/50559-add-milestone-progress-to-api.yml

----
-title: 'API: Expose milestone progress'
-merge_request: 25173
-author: Robert Schilling
-type: added

doc/api/group_milestones.md

@@ -42,7 +42,6 @@ Example Response:
     "due_date": "2013-11-29",
     "start_date": "2013-11-10",
     "state": "active",
-    "percentage_complete" : 66,
     "updated_at": "2013-10-02T09:24:18Z",
     "created_at": "2013-10-02T09:24:18Z"
   }

doc/api/milestones.md

@@ -39,7 +39,6 @@ Example Response:
     "due_date": "2013-11-29",
     "start_date": "2013-11-10",
     "state": "active",
-    "percentage_complete" : 66,
     "updated_at": "2013-10-02T09:24:18Z",
     "created_at": "2013-10-02T09:24:18Z"
   }

lib/api/entities.rb

@@ -504,9 +504,6 @@ module API
       expose :state, :created_at, :updated_at
       expose :due_date
       expose :start_date
-      expose :percentage_complete do |milestone, options|
-        milestone.percent_complete(options[:current_user])
-      end
 
       expose :web_url do |milestone, _options|
         Gitlab::UrlBuilder.build(milestone)

lib/api/milestone_responses.rb

@@ -35,19 +35,19 @@ module API
           milestones = filter_by_iid(milestones, params[:iids]) if params[:iids].present?
           milestones = filter_by_search(milestones, params[:search]) if params[:search]
 
-          present paginate(milestones), with: Entities::Milestone, current_user: current_user
+          present paginate(milestones), with: Entities::Milestone
         end
 
         def get_milestone_for(parent)
           milestone = parent.milestones.find(params[:milestone_id])
-          present milestone, with: Entities::Milestone, current_user: current_user
+          present milestone, with: Entities::Milestone
         end
 
         def create_milestone_for(parent)
           milestone = ::Milestones::CreateService.new(parent, current_user, declared_params).execute
 
           if milestone.valid?
-            present milestone, with: Entities::Milestone, current_user: current_user
+            present milestone, with: Entities::Milestone
           else
             render_api_error!("Failed to create milestone #{milestone.errors.messages}", 400)
           end
@@ -60,7 +60,7 @@ module API
           milestone = ::Milestones::UpdateService.new(parent, current_user, milestone_params).execute(milestone)
 
           if milestone.valid?
-            present milestone, with: Entities::Milestone, current_user: current_user
+            present milestone, with: Entities::Milestone
           else
             render_api_error!("Failed to update milestone #{milestone.errors.messages}", 400)
           end

spec/fixtures/api/schemas/public_api/v4/milestone.json

@@ -8,7 +8,6 @@
     "title": { "type": "string" },
     "description": { "type": ["string", "null"] },
     "state": { "type": "string" },
-    "percentage_complete": { "type": "integer" },
     "created_at": { "type": "date" },
     "updated_at": { "type": "date" },
     "start_date": { "type": "date" },

spec/javascripts/helpers/vuex_action_helper.js

@@ -84,7 +84,10 @@ export default (
     done();
   };
 
-  const result = action({ commit, state, dispatch, rootState: state, rootGetters: state }, payload);
+  const result = action(
+    { commit, state, dispatch, rootState: state, rootGetters: state, getters: state },
+    payload,
+  );
 
   return new Promise(resolve => {
     setImmediate(resolve);

spec/javascripts/notes/stores/actions_spec.js

 import Vue from 'vue';
 import $ from 'jquery';
 import _ from 'underscore';
+import { TEST_HOST } from 'spec/test_constants';
 import { headersInterceptor } from 'spec/helpers/vue_resource_helper';
 import * as actions from '~/notes/stores/actions';
+import * as mutationTypes from '~/notes/stores/mutation_types';
+import * as notesConstants from '~/notes/constants';
 import createStore from '~/notes/stores';
 import mrWidgetEventHub from '~/vue_merge_request_widget/event_hub';
 import testAction from '../../helpers/vuex_action_helper';
@@ -599,4 +602,139 @@ describe('Actions Notes Store', () => {
       );
     });
   });
+
+  describe('updateOrCreateNotes', () => {
+    let commit;
+    let dispatch;
+    let state;
+
+    beforeEach(() => {
+      commit = jasmine.createSpy('commit');
+      dispatch = jasmine.createSpy('dispatch');
+      state = {};
+    });
+
+    afterEach(() => {
+      commit.calls.reset();
+      dispatch.calls.reset();
+    });
+
+    it('Updates existing note', () => {
+      const note = { id: 1234 };
+      const getters = { notesById: { 1234: note } };
+
+      actions.updateOrCreateNotes({ commit, state, getters, dispatch }, [note]);
+
+      expect(commit.calls.allArgs()).toEqual([[mutationTypes.UPDATE_NOTE, note]]);
+    });
+
+    it('Creates a new note if none exisits', () => {
+      const note = { id: 1234 };
+      const getters = { notesById: {} };
+      actions.updateOrCreateNotes({ commit, state, getters, dispatch }, [note]);
+
+      expect(commit.calls.allArgs()).toEqual([[mutationTypes.ADD_NEW_NOTE, note]]);
+    });
+
+    describe('Discussion notes', () => {
+      let note;
+      let getters;
+
+      beforeEach(() => {
+        note = { id: 1234 };
+        getters = { notesById: {} };
+      });
+
+      it('Adds a reply to an existing discussion', () => {
+        state = { discussions: [note] };
+        const discussionNote = {
+          ...note,
+          type: notesConstants.DISCUSSION_NOTE,
+          discussion_id: 1234,
+        };
+
+        actions.updateOrCreateNotes({ commit, state, getters, dispatch }, [discussionNote]);
+
+        expect(commit.calls.allArgs()).toEqual([
+          [mutationTypes.ADD_NEW_REPLY_TO_DISCUSSION, discussionNote],
+        ]);
+      });
+
+      it('fetches discussions for diff notes', () => {
+        state = { discussions: [], notesData: { discussionsPath: 'Hello world' } };
+        const diffNote = { ...note, type: notesConstants.DIFF_NOTE, discussion_id: 1234 };
+
+        actions.updateOrCreateNotes({ commit, state, getters, dispatch }, [diffNote]);
+
+        expect(dispatch.calls.allArgs()).toEqual([
+          ['fetchDiscussions', { path: state.notesData.discussionsPath }],
+        ]);
+      });
+
+      it('Adds a new note', () => {
+        state = { discussions: [] };
+        const discussionNote = {
+          ...note,
+          type: notesConstants.DISCUSSION_NOTE,
+          discussion_id: 1234,
+        };
+
+        actions.updateOrCreateNotes({ commit, state, getters, dispatch }, [discussionNote]);
+
+        expect(commit.calls.allArgs()).toEqual([[mutationTypes.ADD_NEW_NOTE, discussionNote]]);
+      });
+    });
+  });
+
+  describe('replyToDiscussion', () => {
+    let res = { discussion: { notes: [] } };
+    const payload = { endpoint: TEST_HOST, data: {} };
+    const interceptor = (request, next) => {
+      next(
+        request.respondWith(JSON.stringify(res), {
+          status: 200,
+        }),
+      );
+    };
+
+    beforeEach(() => {
+      Vue.http.interceptors.push(interceptor);
+    });
+
+    afterEach(() => {
+      Vue.http.interceptors = _.without(Vue.http.interceptors, interceptor);
+    });
+
+    it('updates discussion if response contains disussion', done => {
+      testAction(
+        actions.replyToDiscussion,
+        payload,
+        {
+          notesById: {},
+        },
+        [{ type: mutationTypes.UPDATE_DISCUSSION, payload: res.discussion }],
+        [
+          { type: 'updateMergeRequestWidget' },
+          { type: 'startTaskList' },
+          { type: 'updateResolvableDiscussonsCounts' },
+        ],
+        done,
+      );
+    });
+
+    it('adds a reply to a discussion', done => {
+      res = {};
+
+      testAction(
+        actions.replyToDiscussion,
+        payload,
+        {
+          notesById: {},
+        },
+        [{ type: mutationTypes.ADD_NEW_REPLY_TO_DISCUSSION, payload: res }],
+        [],
+        done,
+      );
+    });
+  });
 });

spec/javascripts/notes/stores/mutation_spec.js

@@ -527,17 +527,13 @@ describe('Notes Store mutations', () => {
         id: 42,
         individual_note: true,
       };
-      state = { discussions: [discussion] };
+      state = { convertedDisscussionIds: [] };
     });
 
-    it('toggles individual_note', () => {
+    it('adds a disucssion to convertedDisscussionIds', () => {
       mutations.CONVERT_TO_DISCUSSION(state, discussion.id);
 
-      expect(discussion.individual_note).toBe(false);
-    });
-
-    it('throws if discussion was not found', () => {
-      expect(() => mutations.CONVERT_TO_DISCUSSION(state, 99)).toThrow();
+      expect(state.convertedDisscussionIds).toContain(discussion.id);
     });
   });
 });

spec/support/api/milestones_shared_examples.rb

@@ -8,17 +8,12 @@ shared_examples_for 'group and project milestones' do |route_definition|
 
   describe "GET #{route_definition}" do
     it 'returns milestones list' do
-      create(:issue, project: project, milestone: milestone)
-      create(:closed_issue, project: project, milestone: milestone)
-      create(:closed_issue, project: project, milestone: milestone)
-
       get api(route, user)
 
       expect(response).to have_gitlab_http_status(200)
       expect(response).to include_pagination_headers
       expect(json_response).to be_an Array
       expect(json_response.first['title']).to eq(milestone.title)
-      expect(json_response.first['percentage_complete']).to eq(66)
     end
 
     it 'returns a 401 error if user not authenticated' do