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

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>

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.') }}

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;

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();

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 }) => {

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,

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) {

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;

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);
     },
   },
 };

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);

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;