From e7439c310471d024df04796c227988a152c4c072 Mon Sep 17 00:00:00 2001
From: Winnie Hellmann <winnie@gitlab.com>
Date: Mon, 26 Nov 2018 20:36:06 +0100
Subject: [PATCH] Pass commit when posting diff discussions

---
 app/assets/javascripts/diffs/store/actions.js |  3 +-
 app/assets/javascripts/diffs/store/utils.js   |  3 +-
 ...erge-request-diff-discussion-commit-id.yml |  5 +++
 spec/javascripts/diffs/store/actions_spec.js  | 32 ++++++++++++++-----
 spec/javascripts/diffs/store/utils_spec.js    |  4 +--
 5 files changed, 35 insertions(+), 12 deletions(-)
 create mode 100644 changelogs/unreleased/winh-merge-request-diff-discussion-commit-id.yml

diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index c0456c18e44..952963e0711 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -192,8 +192,9 @@ export const toggleFileDiscussions = ({ getters, dispatch }, diff) => {
   });
 };
 
-export const saveDiffDiscussion = ({ dispatch }, { note, formData }) => {
+export const saveDiffDiscussion = ({ state, dispatch }, { note, formData }) => {
   const postData = getNoteFormData({
+    commit: state.commit,
     note,
     ...formData,
   });
diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js
index 54b9ee4d2d6..a5b3d83740b 100644
--- a/app/assets/javascripts/diffs/store/utils.js
+++ b/app/assets/javascripts/diffs/store/utils.js
@@ -27,6 +27,7 @@ export const getReversePosition = linePosition => {
 
 export function getFormData(params) {
   const {
+    commit,
     note,
     noteableType,
     noteableData,
@@ -66,7 +67,7 @@ export function getFormData(params) {
       position,
       noteable_type: noteableType,
       noteable_id: noteableData.id,
-      commit_id: '',
+      commit_id: commit && commit.id,
       type:
         diffFile.diff_refs.start_sha && diffFile.diff_refs.head_sha
           ? DIFF_NOTE_TYPE
diff --git a/changelogs/unreleased/winh-merge-request-diff-discussion-commit-id.yml b/changelogs/unreleased/winh-merge-request-diff-discussion-commit-id.yml
new file mode 100644
index 00000000000..2ce16a2b6b7
--- /dev/null
+++ b/changelogs/unreleased/winh-merge-request-diff-discussion-commit-id.yml
@@ -0,0 +1,5 @@
+---
+title: Pass commit when posting diff discussions
+merge_request: 23371
+author:
+type: fixed
diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js
index 4b339a0553f..55ce19927e0 100644
--- a/spec/javascripts/diffs/store/actions_spec.js
+++ b/spec/javascripts/diffs/store/actions_spec.js
@@ -29,6 +29,7 @@ import actions, {
 } from '~/diffs/store/actions';
 import * as types from '~/diffs/store/mutation_types';
 import axios from '~/lib/utils/axios_utils';
+import mockDiffFile from 'spec/diffs/mock_data/diff_file';
 import testAction from '../../helpers/vuex_action_helper';
 
 describe('DiffsStoreActions', () => {
@@ -607,11 +608,18 @@ describe('DiffsStoreActions', () => {
   });
 
   describe('saveDiffDiscussion', () => {
-    beforeEach(() => {
-      spyOnDependency(actions, 'getNoteFormData').and.returnValue('testData');
-    });
-
     it('dispatches actions', done => {
+      const commitId = 'something';
+      const formData = {
+        diffFile: { ...mockDiffFile },
+        noteableData: {},
+      };
+      const note = {};
+      const state = {
+        commit: {
+          id: commitId,
+        },
+      };
       const dispatch = jasmine.createSpy('dispatch').and.callFake(name => {
         switch (name) {
           case 'saveNote':
@@ -625,11 +633,19 @@ describe('DiffsStoreActions', () => {
         }
       });
 
-      saveDiffDiscussion({ dispatch }, { note: {}, formData: {} })
+      saveDiffDiscussion({ state, 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', ['discussion']]);
+          const { calls } = dispatch;
+
+          expect(calls.count()).toBe(5);
+          expect(calls.argsFor(0)).toEqual(['saveNote', jasmine.any(Object), { root: true }]);
+
+          const postData = calls.argsFor(0)[1];
+
+          expect(postData.data.note.commit_id).toBe(commitId);
+
+          expect(calls.argsFor(1)).toEqual(['updateDiscussion', 'test', { root: true }]);
+          expect(calls.argsFor(2)).toEqual(['assignDiscussionsToDiff', ['discussion']]);
         })
         .then(done)
         .catch(done.fail);
diff --git a/spec/javascripts/diffs/store/utils_spec.js b/spec/javascripts/diffs/store/utils_spec.js
index d4ef17c5ef8..8988e93b1be 100644
--- a/spec/javascripts/diffs/store/utils_spec.js
+++ b/spec/javascripts/diffs/store/utils_spec.js
@@ -150,7 +150,7 @@ describe('DiffsStoreUtils', () => {
         note: {
           noteable_type: options.noteableType,
           noteable_id: options.noteableData.id,
-          commit_id: '',
+          commit_id: undefined,
           type: DIFF_NOTE_TYPE,
           line_code: options.noteTargetLine.line_code,
           note: options.note,
@@ -209,7 +209,7 @@ describe('DiffsStoreUtils', () => {
         note: {
           noteable_type: options.noteableType,
           noteable_id: options.noteableData.id,
-          commit_id: '',
+          commit_id: undefined,
           type: LEGACY_DIFF_NOTE_TYPE,
           line_code: options.noteTargetLine.line_code,
           note: options.note,