Fixes based on MR discussion around naming, mutations, handling of state

11 files changed, 23 insertions, 24 deletions

diff --git a/app/assets/javascripts/diffs/components/diff_discussions.vue b/app/assets/javascripts/diffs/components/diff_discussions.vue
index d7453834226..cddbe554fbd 100644
--- a/app/assets/javascripts/diffs/components/diff_discussions.vue
+++ b/app/assets/javascripts/diffs/components/diff_discussions.vue
@@ -40,7 +40,7 @@ export default {
           :render-diff-file="false"
           :always-expanded="true"
           :discussions-by-diff-order="true"
-          @handleNoteDelete="deleteNoteHandler"
+          @noteDeleted="deleteNoteHandler"
         />
       </ul>
     </div>
diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue
index a0197e95456..67e85c4eee3 100644
--- a/app/assets/javascripts/diffs/components/diff_file.vue
+++ b/app/assets/javascripts/diffs/components/diff_file.vue
@@ -49,7 +49,8 @@ export default {
         !this.isCollapsed &&
         !this.file.highlightedDiffLines &&
         !this.isLoadingCollapsedDiff &&
-        !this.file.tooLarge
+        !this.file.tooLarge &&
+        this.file.text
       );
     },
     showLoadingIcon() {
@@ -76,7 +77,6 @@ export default {
           this.file.collapsed = false;
           this.file.renderIt = true;
         })
-        .then(() => this.$nextTick())
         .then(() => {
           requestIdleCallback(
             () => {
@@ -149,7 +149,7 @@ export default {
       class="diff-content loading"
     />
     <div
-      v-if="showExpandMessage"
+      v-else-if="showExpandMessage"
       class="nothing-here-block diff-collapsed"
     >
       {{ __('This diff is collapsed.') }}
diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
index 88bf026c0dd..6eff3013dcd 100644
--- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
+++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
@@ -73,7 +73,7 @@ export default {
     }),
     ...mapGetters(['isLoggedIn']),
     lineHref() {
-      return this.line && this.line.lineCode ? `#${this.line.lineCode}` : '#';
+      return `#${this.line.lineCode || ''}`;
     },
     shouldShowCommentButton() {
       return (
@@ -87,10 +87,10 @@ export default {
       );
     },
     hasDiscussions() {
-      return this.line && this.line.discussions && this.line.discussions.length > 0;
+      return this.line.discussions && this.line.discussions.length > 0;
     },
     shouldShowAvatarsOnGutter() {
-      if (this.line && !this.line.type && this.linePosition === LINE_POSITION_RIGHT) {
+      if (!this.line.type && this.linePosition === LINE_POSITION_RIGHT) {
         return false;
       }
       return this.showCommentButton && this.hasDiscussions;
diff --git a/app/assets/javascripts/diffs/components/diff_line_note_form.vue b/app/assets/javascripts/diffs/components/diff_line_note_form.vue
index cdd64ca99e3..a0dc381ccc7 100644
--- a/app/assets/javascripts/diffs/components/diff_line_note_form.vue
+++ b/app/assets/javascripts/diffs/components/diff_line_note_form.vue
@@ -6,7 +6,7 @@ import noteForm from '../../notes/components/note_form.vue';
 import { getNoteFormData } from '../store/utils';
 import autosave from '../../notes/mixins/autosave';
 import { DIFF_NOTE_TYPE } from '../constants';
-import * as utils from '../../notes/stores/utils';
+import { reduceDiscussionsToLineCodes } from '../../notes/stores/utils';
 
 export default {
   components: {
@@ -90,7 +90,7 @@ export default {
 
           this.refetchDiscussionById({ path: endpoint, discussionId: result.discussion_id })
             .then(selectedDiscussion => {
-              const lineCodeDiscussions = utils.reduceDiscussionsToLineCodes([selectedDiscussion]);
+              const lineCodeDiscussions = reduceDiscussionsToLineCodes([selectedDiscussion]);
               this.assignDiscussionsToDiff(lineCodeDiscussions);
 
               this.handleCancelCommentForm();
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index c60a0e7f593..44ae0f2f17b 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -29,6 +29,8 @@ export const fetchDiffFiles = ({ state, commit }) => {
     .then(handleLocationHash);
 };
 
+// This is adding line discussions to the actual lines in the diff tree
+// once for parallel and once for inline mode
 export const assignDiscussionsToDiff = ({ state, commit }, allLineDiscussions) => {
   Object.values(allLineDiscussions).forEach(discussions => {
     if (discussions.length > 0) {
@@ -74,12 +76,10 @@ export const removeDiscussionsFromDiff = ({ state, commit }, removeDiscussion) =
     );
 
     if (targetLine) {
-      if (targetLine) {
-        if (targetLine.left && targetLine.left.lineCode === removeDiscussion.line_code) {
-          commit(types.REMOVE_LINE_DISCUSSIONS, targetLine.left);
-        } else {
-          commit(types.REMOVE_LINE_DISCUSSIONS, targetLine.right);
-        }
+      if (targetLine.left && targetLine.left.lineCode === removeDiscussion.line_code) {
+        commit(types.REMOVE_LINE_DISCUSSIONS, targetLine.left);
+      } else {
+        commit(types.REMOVE_LINE_DISCUSSIONS, targetLine.right);
       }
     }
 
@@ -117,11 +117,7 @@ export const startRenderDiffsQueue = ({ state, commit }) => {
       }
     });
 
-  return new Promise(resolve => {
-    checkItem()
-      .then(resolve)
-      .catch(() => {});
-  });
+  return checkItem();
 };
 
 export const setInlineDiffViewType = ({ commit }) => {
diff --git a/app/assets/javascripts/diffs/store/modules/diff_state.js b/app/assets/javascripts/diffs/store/modules/diff_state.js
index b221eb75364..39d90a64aab 100644
--- a/app/assets/javascripts/diffs/store/modules/diff_state.js
+++ b/app/assets/javascripts/diffs/store/modules/diff_state.js
@@ -8,7 +8,6 @@ const defaultViewType = INLINE_DIFF_VIEW_TYPE;
 
 export default () => ({
   isLoading: true,
-  loadingMessage: '',
   endpoint: '',
   basePath: '',
   commit: null,
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index 6dfdf95fdad..1dd43f7857a 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -73,7 +73,8 @@ export default {
     const normalizedData = convertObjectPropsToCamelCase(data, { deep: true });
     prepareDiffData(normalizedData);
     const [newFileData] = normalizedData.diffFiles.filter(f => f.fileHash === file.fileHash);
-    Object.assign(file, { ...newFileData });
+    const selectedFile = state.diffFiles.find(f => f.fileHash === file.fileHash);
+    Object.assign(selectedFile, { ...newFileData });
   },
 
   [types.EXPAND_ALL_FILES](state) {
diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js
index e15588a58a8..7772dac89a7 100644
--- a/app/assets/javascripts/diffs/store/utils.js
+++ b/app/assets/javascripts/diffs/store/utils.js
@@ -181,6 +181,8 @@ export function trimFirstCharOfLineContent(line = {}) {
   return parsedLine;
 }
 
+// This prepares and optimizes the incoming diff data from the server
+// by setting up incremental rendering and removing unneeded data
 export function prepareDiffData(diffData) {
   const filesLength = diffData.diffFiles.length;
   let showingLines = 0;
diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue
index 70425937feb..afe86911230 100644
--- a/app/assets/javascripts/notes/components/noteable_discussion.vue
+++ b/app/assets/javascripts/notes/components/noteable_discussion.vue
@@ -266,7 +266,7 @@ Please check your network connection and try again.`;
       this.jumpToDiscussion(nextId);
     },
     deleteNoteHandler(note) {
-      this.$emit('handleNoteDelete', this.discussion, note);
+      this.$emit('noteDeleted', this.discussion, note);
     },
   },
 };
diff --git a/app/assets/javascripts/notes/stores/utils.js b/app/assets/javascripts/notes/stores/utils.js
index a6cfd1590f5..7608790d042 100644
--- a/app/assets/javascripts/notes/stores/utils.js
+++ b/app/assets/javascripts/notes/stores/utils.js
@@ -31,7 +31,7 @@ export const reduceDiscussionsToLineCodes = selectedDiscussions =>
       // For context about line notes: there might be multiple notes with the same line code
       const items = acc[note.line_code] || [];
       if (note.diff_file) {
-        Object.assign(note, { fileHash: note.diff_file.file_hash });
+        // Object.assign(note, { fileHash: note.diff_file.file_hash });
       }
 
       items.push(note);
diff --git a/spec/javascripts/diffs/components/diff_file_spec.js b/spec/javascripts/diffs/components/diff_file_spec.js
index 0917af23e84..845fef23db6 100644
--- a/spec/javascripts/diffs/components/diff_file_spec.js
+++ b/spec/javascripts/diffs/components/diff_file_spec.js
@@ -51,6 +51,7 @@ describe('DiffFile', () => {
       });
 
       it('should have collapsed text and link', done => {
+        vm.file.renderIt = true;
         vm.file.collapsed = false;
         vm.file.highlightedDiffLines = null;