Merge branch 'dm-create-note-return-discussion' into 'master'

Increase performance when creating discussion on diff

Closes #49002

See merge request gitlab-org/gitlab-ce!21743

4 files changed, 60 insertions, 25 deletions

diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb
index 81badaac76b..e48c9dea976 100644
--- a/spec/controllers/projects/notes_controller_spec.rb
+++ b/spec/controllers/projects/notes_controller_spec.rb
@@ -207,6 +207,14 @@ describe Projects::NotesController do
       expect(response).to have_gitlab_http_status(200)
     end
 
+    it 'returns discussion JSON when the return_discussion param is set' do
+      post :create, request_params.merge(format: :json, return_discussion: 'true')
+
+      expect(response).to have_gitlab_http_status(200)
+      expect(json_response).to have_key 'discussion'
+      expect(json_response['discussion']['notes'][0]['note']).to eq(request_params[:note][:note])
+    end
+
     context 'when merge_request_diff_head_sha present' do
       before do
         service_params = {
diff --git a/spec/javascripts/diffs/components/diff_line_note_form_spec.js b/spec/javascripts/diffs/components/diff_line_note_form_spec.js
index 6fe5fdaf7f9..f31fc1f0e2b 100644
--- a/spec/javascripts/diffs/components/diff_line_note_form_spec.js
+++ b/spec/javascripts/diffs/components/diff_line_note_form_spec.js
@@ -69,22 +69,21 @@ describe('DiffLineNoteForm', () => {
 
     describe('saveNoteForm', () => {
       it('should call saveNote action with proper params', done => {
-        let isPromiseCalled = false;
-        const formDataSpy = spyOnDependency(DiffLineNoteForm, 'getNoteFormData').and.returnValue({
-          postData: 1,
-        });
-        const saveNoteSpy = spyOn(component, 'saveNote').and.returnValue(
-          new Promise(() => {
-            isPromiseCalled = true;
-            done();
-          }),
+        const saveDiffDiscussionSpy = spyOn(component, 'saveDiffDiscussion').and.returnValue(
+          Promise.resolve(),
         );
-
-        component.handleSaveNote('note body');
-
-        expect(formDataSpy).toHaveBeenCalled();
-        expect(saveNoteSpy).toHaveBeenCalled();
-        expect(isPromiseCalled).toEqual(true);
+        spyOnProperty(component, 'formData').and.returnValue('formData');
+
+        component
+          .handleSaveNote('note body')
+          .then(() => {
+            expect(saveDiffDiscussionSpy).toHaveBeenCalledWith({
+              note: 'note body',
+              formData: 'formData',
+            });
+          })
+          .then(done)
+          .catch(done.fail);
       });
     });
   });
diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js
index 4c662fac231..05b39bad6ea 100644
--- a/spec/javascripts/diffs/store/actions_spec.js
+++ b/spec/javascripts/diffs/store/actions_spec.js
@@ -21,6 +21,7 @@ import actions, {
   loadCollapsedDiff,
   expandAllFiles,
   toggleFileDiscussions,
+  saveDiffDiscussion,
 } from '~/diffs/store/actions';
 import * as types from '~/diffs/store/mutation_types';
 import { reduceDiscussionsToLineCodes } from '~/notes/stores/utils';
@@ -245,7 +246,7 @@ describe('DiffsStoreActions', () => {
   });
 
   describe('startRenderDiffsQueue', () => {
-    it('should set all files to RENDER_FILE', done => {
+    it('should set all files to RENDER_FILE', () => {
       const state = {
         diffFiles: [
           {
@@ -268,16 +269,10 @@ describe('DiffsStoreActions', () => {
         });
       };
 
-      startRenderDiffsQueue({ state, commit: pseudoCommit })
-        .then(() => {
-          expect(state.diffFiles[0].renderIt).toBeTruthy();
-          expect(state.diffFiles[1].renderIt).toBeTruthy();
+      startRenderDiffsQueue({ state, commit: pseudoCommit });
 
-          done();
-        })
-        .catch(() => {
-          done.fail();
-        });
+      expect(state.diffFiles[0].renderIt).toBe(true);
+      expect(state.diffFiles[1].renderIt).toBe(true);
     });
   });
 
@@ -582,4 +577,35 @@ describe('DiffsStoreActions', () => {
       expect(handleLocationHashSpy).toHaveBeenCalledTimes(1);
     });
   });
+
+  describe('saveDiffDiscussion', () => {
+    beforeEach(() => {
+      spyOnDependency(actions, 'getNoteFormData').and.returnValue('testData');
+      spyOnDependency(actions, 'reduceDiscussionsToLineCodes').and.returnValue('discussions');
+    });
+
+    it('dispatches actions', done => {
+      const dispatch = jasmine.createSpy('dispatch').and.callFake(name => {
+        switch (name) {
+          case 'saveNote':
+            return Promise.resolve({
+              discussion: 'test',
+            });
+          case 'updateDiscussion':
+            return Promise.resolve('discussion');
+          default:
+            return Promise.resolve({});
+        }
+      });
+
+      saveDiffDiscussion({ dispatch }, { note: {}, formData: {} })
+        .then(() => {
+          expect(dispatch.calls.argsFor(0)).toEqual(['saveNote', 'testData', { root: true }]);
+          expect(dispatch.calls.argsFor(1)).toEqual(['updateDiscussion', 'test', { root: true }]);
+          expect(dispatch.calls.argsFor(2)).toEqual(['assignDiscussionsToDiff', 'discussions']);
+        })
+        .then(done)
+        .catch(done.fail);
+    });
+  });
 });
diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js
index 6138b9701f4..897cd1483aa 100644
--- a/spec/javascripts/diffs/store/utils_spec.js
+++ b/spec/javascripts/diffs/store/utils_spec.js
@@ -136,6 +136,7 @@ describe('DiffsStoreUtils', () => {
         note_project_id: '',
         target_type: options.noteableType,
         target_id: options.noteableData.id,
+        return_discussion: true,
         note: {
           noteable_type: options.noteableType,
           noteable_id: options.noteableData.id,
@@ -194,6 +195,7 @@ describe('DiffsStoreUtils', () => {
         note_project_id: '',
         target_type: options.noteableType,
         target_id: options.noteableData.id,
+        return_discussion: true,
         note: {
           noteable_type: options.noteableType,
           noteable_id: options.noteableData.id,