Fixed Problems with Inline View + Specs

11 files changed, 75 insertions, 77 deletions

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 f1299408904..92891b37c1e 100644
--- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
+++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue
@@ -78,7 +78,7 @@ export default {
     }),
     ...mapGetters(['isLoggedIn']),
     lineHref() {
-      return this.line.code ? `#${this.line.code}` : '#';
+      return this.line && this.line.code ? `#${this.line.code}` : '#';
     },
     shouldShowCommentButton() {
       return (
@@ -92,10 +92,10 @@ export default {
       );
     },
     hasDiscussions() {
-      return this.line.discussions && this.line.discussions.length > 0;
+      return this.line && this.line.discussions && this.line.discussions.length > 0;
     },
     shouldShowAvatarsOnGutter() {
-      if (!this.line.type && this.linePosition === LINE_POSITION_RIGHT) {
+      if (this.line && !this.line.type && this.linePosition === LINE_POSITION_RIGHT) {
         return false;
       }
 
diff --git a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue
index 6348f32d36d..46a51859da5 100644
--- a/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue
+++ b/app/assets/javascripts/diffs/components/inline_diff_comment_row.vue
@@ -21,18 +21,13 @@ export default {
       type: Number,
       required: true,
     },
-    discussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
   },
   computed: {
     ...mapState({
       diffLineCommentForms: state => state.diffs.diffLineCommentForms,
     }),
     className() {
-      return this.discussions.length ? '' : 'js-temp-notes-holder';
+      return this.line.discussions.length ? '' : 'js-temp-notes-holder';
     },
   },
 };
@@ -49,8 +44,8 @@ export default {
     >
       <div class="content">
         <diff-discussions
-          v-if="discussions.length"
-          :discussions="discussions"
+          v-if="line.discussions.length"
+          :discussions="line.discussions"
         />
         <diff-line-note-form
           v-if="diffLineCommentForms[line.lineCode]"
diff --git a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
index 32d65ff994f..0e306f39a9f 100644
--- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
+++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue
@@ -33,11 +33,6 @@ export default {
       required: false,
       default: false,
     },
-    discussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
   },
   data() {
     return {
@@ -94,7 +89,6 @@ export default {
       :is-bottom="isBottom"
       :is-hover="isHover"
       :show-comment-button="true"
-      :discussions="discussions"
       class="diff-line-num old_line"
     />
     <diff-table-cell
@@ -104,7 +98,6 @@ export default {
       :line-type="newLineType"
       :is-bottom="isBottom"
       :is-hover="isHover"
-      :discussions="discussions"
       class="diff-line-num new_line"
     />
     <td
diff --git a/app/assets/javascripts/diffs/components/inline_diff_view.vue b/app/assets/javascripts/diffs/components/inline_diff_view.vue
index e7d789734c3..947e7c98fae 100644
--- a/app/assets/javascripts/diffs/components/inline_diff_view.vue
+++ b/app/assets/javascripts/diffs/components/inline_diff_view.vue
@@ -2,7 +2,6 @@
 import { mapGetters, mapState } from 'vuex';
 import inlineDiffTableRow from './inline_diff_table_row.vue';
 import inlineDiffCommentRow from './inline_diff_comment_row.vue';
-import { trimFirstCharOfLineContent } from '../store/utils';
 
 export default {
   components: {
@@ -20,29 +19,17 @@ export default {
     },
   },
   computed: {
-    ...mapGetters('diffs', [
-      'commitId',
-      'shouldRenderInlineCommentRow',
-      'singleDiscussionByLineCode',
-    ]),
+    ...mapGetters('diffs', ['commitId', 'shouldRenderInlineCommentRow']),
     ...mapState({
       diffLineCommentForms: state => state.diffs.diffLineCommentForms,
     }),
-    normalizedDiffLines() {
-      return this.diffLines.map(line => (line.richText ? trimFirstCharOfLineContent(line) : line));
-    },
     diffLinesLength() {
-      return this.normalizedDiffLines.length;
+      return this.diffLines.length;
     },
     userColorScheme() {
       return window.gon.user_color_scheme;
     },
   },
-  methods: {
-    discussionsList(line) {
-      return line.lineCode !== undefined ? this.singleDiscussionByLineCode(line.lineCode) : [];
-    },
-  },
 };
 </script>
 
@@ -53,7 +40,7 @@ export default {
     class="code diff-wrap-lines js-syntax-highlight text-file js-diff-inline-view">
     <tbody>
       <template
-        v-for="(line, index) in normalizedDiffLines"
+        v-for="(line, index) in diffLines"
       >
         <inline-diff-table-row
           :file-hash="diffFile.fileHash"
@@ -61,7 +48,6 @@ export default {
           :line="line"
           :is-bottom="index + 1 === diffLinesLength"
           :key="line.lineCode"
-          :discussions="discussionsList(line)"
         />
         <inline-diff-comment-row
           v-if="shouldRenderInlineCommentRow(line)"
@@ -69,7 +55,6 @@ export default {
           :line="line"
           :line-index="index"
           :key="index"
-          :discussions="discussionsList(line)"
         />
       </template>
     </tbody>
diff --git a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue
index 38c6458a14e..fb68d191091 100644
--- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue
+++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue
@@ -1,6 +1,5 @@
 <script>
 import $ from 'jquery';
-import { mapGetters } from 'vuex';
 import DiffTableCell from './diff_table_cell.vue';
 import {
   NEW_LINE_TYPE,
diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue
index ebdddeb790e..501bd4450d8 100644
--- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue
+++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue
@@ -30,13 +30,6 @@ export default {
       return window.gon.user_color_scheme;
     },
   },
-  methods: {
-    discussionsByLine(line, leftOrRight) {
-      return line[leftOrRight] && line[leftOrRight].lineCode !== undefined
-        ? this.singleDiscussionByLineCode(line[leftOrRight].lineCode)
-        : [];
-    },
-  },
 };
 </script>
 
diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js
index 082934fbb63..7fdc989634a 100644
--- a/app/assets/javascripts/diffs/store/actions.js
+++ b/app/assets/javascripts/diffs/store/actions.js
@@ -32,27 +32,34 @@ export const fetchDiffFiles = ({ state, commit }) => {
 export const assignDiscussionsToDiff = ({ state }, allLineDiscussions) => {
   Object.values(allLineDiscussions).forEach(discussions => {
     if (discussions.length > 0) {
-      const fileHash = discussions[0].fileHash;
+      const { fileHash } = discussions[0];
       const selectedFile = state.diffFiles.find(file => file.fileHash === fileHash);
       if (selectedFile) {
-        const targetLine = selectedFile.parallelDiffLines.find(line => {
-          return (
+        const targetLine = selectedFile.parallelDiffLines.find(
+          line =>
             (line.left && line.left.lineCode === discussions[0].line_code) ||
-            (line.right && line.right.lineCode === discussions[0].line_code)
-          );
-        });
+            (line.right && line.right.lineCode === discussions[0].line_code),
+        );
 
         if (targetLine) {
           Object.assign(targetLine.right, { discussions });
         }
+
+        const targetInlineLine = selectedFile.highlightedDiffLines.find(
+          line => line.lineCode === discussions[0].line_code,
+        );
+
+        if (targetInlineLine) {
+          Object.assign(targetInlineLine, { discussions });
+        }
       }
     }
   });
 };
 
 export const startRenderDiffsQueue = ({ state, commit }) => {
-  const checkItem = () => {
-    return new Promise(resolve => {
+  const checkItem = () =>
+    new Promise(resolve => {
       const nextFile = state.diffFiles.find(
         file => !file.renderIt && (!file.collapsed || !file.text),
       );
@@ -73,7 +80,6 @@ export const startRenderDiffsQueue = ({ state, commit }) => {
         resolve();
       }
     });
-  };
 
   return new Promise(resolve => {
     checkItem()
diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js
index f56b0ac695e..2a9191cde8a 100644
--- a/app/assets/javascripts/diffs/store/getters.js
+++ b/app/assets/javascripts/diffs/store/getters.js
@@ -17,7 +17,10 @@ export const commitId = state => (state.commit && state.commit.id ? state.commit
 export const diffHasAllExpandedDiscussions = (state, getters) => diff => {
   const discussions = getters.getDiffFileDiscussions(diff);
 
-  return (discussions.length && discussions.every(discussion => discussion.expanded)) || false;
+  return (
+    (discussions && discussions.length && discussions.every(discussion => discussion.expanded)) ||
+    false
+  );
 };
 
 /**
@@ -28,7 +31,10 @@ export const diffHasAllExpandedDiscussions = (state, getters) => diff => {
 export const diffHasAllCollpasedDiscussions = (state, getters) => diff => {
   const discussions = getters.getDiffFileDiscussions(diff);
 
-  return (discussions.length && discussions.every(discussion => !discussion.expanded)) || false;
+  return (
+    (discussions && discussions.length && discussions.every(discussion => !discussion.expanded)) ||
+    false
+  );
 };
 
 /**
@@ -40,7 +46,9 @@ export const diffHasExpandedDiscussions = (state, getters) => diff => {
   const discussions = getters.getDiffFileDiscussions(diff);
 
   return (
-    (discussions.length && discussions.find(discussion => discussion.expanded) !== undefined) ||
+    (discussions &&
+      discussions.length &&
+      discussions.find(discussion => discussion.expanded) !== undefined) ||
     false
   );
 };
@@ -64,17 +72,6 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) =
       discussion.diff_discussion && _.isEqual(discussion.diff_file.file_hash, diff.fileHash),
   ) || [];
 
-export const singleDiscussionByLineCodeOld = (
-  state,
-  getters,
-  rootState,
-  rootGetters,
-) => lineCode => {
-  if (!lineCode || lineCode === undefined) return [];
-  const discussions = rootGetters.discussionsByLineCode;
-  return discussions[lineCode] || [];
-};
-
 /**
  * Returns an Object with discussions by their diff line code
  * To avoid rendering outdated discussions on the Changes tab we should do a bunch of SHA
@@ -113,16 +110,17 @@ export const discussionsByLineCode = (state, getters, rootState, rootGetters) =>
   }, {});
 };
 
-export const shouldRenderParallelCommentRow = (state, getters) => line => {
+export const shouldRenderParallelCommentRow = state => line => {
   const hasDiscussion =
-    (line.left && line.left.discussions.length) || (line.right && line.right.discussions.length);
+    (line.left && line.left.discussions && line.left.discussions.length) ||
+    (line.right && line.right.discussions && line.right.discussions.length);
 
   const hasExpandedDiscussionOnLeft =
-    line.left && line.left.discussions.length
+    line.left && line.left.discussions && line.left.discussions.length
       ? line.left.discussions.every(discussion => discussion.expanded)
       : false;
   const hasExpandedDiscussionOnRight =
-    line.right && line.right.discussions.length
+    line.right && line.right.discussions && line.right.discussions.length
       ? line.right.discussions.every(discussion => discussion.expanded)
       : false;
 
@@ -136,15 +134,14 @@ export const shouldRenderParallelCommentRow = (state, getters) => line => {
   return hasCommentFormOnLeft || hasCommentFormOnRight;
 };
 
-export const shouldRenderInlineCommentRow = (state, getters) => line => {
+export const shouldRenderInlineCommentRow = state => line => {
   if (state.diffLineCommentForms[line.lineCode]) return true;
 
-  const lineDiscussions = getters.singleDiscussionByLineCode(line.lineCode);
-  if (lineDiscussions.length === 0) {
+  if (!line.discussions && line.discussions.length === 0) {
     return false;
   }
 
-  return lineDiscussions.every(discussion => discussion.expanded);
+  return line.discussions.every(discussion => discussion.expanded);
 };
 
 // prevent babel-plugin-rewire from generating an invalid default during karma∂ tests
diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js
index 85843fcddd1..af40a657211 100644
--- a/app/assets/javascripts/diffs/store/mutations.js
+++ b/app/assets/javascripts/diffs/store/mutations.js
@@ -1,8 +1,13 @@
 import Vue from 'vue';
 import _ from 'underscore';
 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
-import { findDiffFile, addLineReferences, removeMatchLine, addContextLines } from './utils';
-import { trimFirstCharOfLineContent } from '../store/utils';
+import {
+  findDiffFile,
+  addLineReferences,
+  removeMatchLine,
+  addContextLines,
+  trimFirstCharOfLineContent,
+} from './utils';
 import { LINES_TO_BE_RENDERED_DIRECTLY, MAX_LINES_TO_BE_RENDERED } from '../constants';
 import * as types from './mutation_types';
 
diff --git a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js
index a1a37b342b7..757de95d695 100644
--- a/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js
+++ b/spec/javascripts/diffs/components/diff_line_gutter_content_spec.js
@@ -11,6 +11,16 @@ describe('DiffLineGutterContent', () => {
   const createComponent = (options = {}) => {
     const cmp = Vue.extend(DiffLineGutterContent);
     const props = Object.assign({}, options);
+    props.line = {
+      lineCode: 'LC_42',
+      type: 'new',
+      oldLine: null,
+      newLine: 1,
+      discussions: [],
+      text: '+<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
+      richText: '+<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
+      metaData: null,
+    };
     props.fileHash = getDiffFileMock().fileHash;
     props.contextLinesPath = '/context/lines/path';
 
diff --git a/spec/javascripts/diffs/mock_data/diff_file.js b/spec/javascripts/diffs/mock_data/diff_file.js
index cce36ecc91f..12de1bc4d51 100644
--- a/spec/javascripts/diffs/mock_data/diff_file.js
+++ b/spec/javascripts/diffs/mock_data/diff_file.js
@@ -49,6 +49,7 @@ export default {
       type: 'new',
       oldLine: null,
       newLine: 1,
+      discussions: [],
       text: '+<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
       richText: '+<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
       metaData: null,
@@ -58,6 +59,7 @@ export default {
       type: 'new',
       oldLine: null,
       newLine: 2,
+      discussions: [],
       text: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
       richText: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
       metaData: null,
@@ -67,6 +69,7 @@ export default {
       type: null,
       oldLine: 1,
       newLine: 3,
+      discussions: [],
       text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
       richText: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
       metaData: null,
@@ -76,6 +79,7 @@ export default {
       type: null,
       oldLine: 2,
       newLine: 4,
+      discussions: [],
       text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
       richText: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
       metaData: null,
@@ -85,6 +89,7 @@ export default {
       type: null,
       oldLine: 3,
       newLine: 5,
+      discussions: [],
       text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
       richText: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
       metaData: null,
@@ -112,6 +117,7 @@ export default {
         type: 'new',
         oldLine: null,
         newLine: 1,
+        discussions: [],
         text: '+<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
         richText: '<span id="LC1" class="line" lang="plaintext">  - Bad dates</span>\n',
         metaData: null,
@@ -126,6 +132,7 @@ export default {
         type: 'new',
         oldLine: null,
         newLine: 2,
+        discussions: [],
         text: '+<span id="LC2" class="line" lang="plaintext"></span>\n',
         richText: '<span id="LC2" class="line" lang="plaintext"></span>\n',
         metaData: null,
@@ -137,6 +144,7 @@ export default {
         type: null,
         oldLine: 1,
         newLine: 3,
+        discussions: [],
         text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
         richText: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
         metaData: null,
@@ -146,6 +154,7 @@ export default {
         type: null,
         oldLine: 1,
         newLine: 3,
+        discussions: [],
         text: ' <span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
         richText: '<span id="LC3" class="line" lang="plaintext">v6.8.0</span>\n',
         metaData: null,
@@ -157,6 +166,7 @@ export default {
         type: null,
         oldLine: 2,
         newLine: 4,
+        discussions: [],
         text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
         richText: '<span id="LC4" class="line" lang="plaintext"></span>\n',
         metaData: null,
@@ -166,6 +176,7 @@ export default {
         type: null,
         oldLine: 2,
         newLine: 4,
+        discussions: [],
         text: ' <span id="LC4" class="line" lang="plaintext"></span>\n',
         richText: '<span id="LC4" class="line" lang="plaintext"></span>\n',
         metaData: null,
@@ -177,6 +188,7 @@ export default {
         type: null,
         oldLine: 3,
         newLine: 5,
+        discussions: [],
         text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
         richText: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
         metaData: null,
@@ -186,6 +198,7 @@ export default {
         type: null,
         oldLine: 3,
         newLine: 5,
+        discussions: [],
         text: ' <span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
         richText: '<span id="LC5" class="line" lang="plaintext">v6.7.0</span>\n',
         metaData: null,
@@ -197,6 +210,7 @@ export default {
         type: 'match',
         oldLine: null,
         newLine: null,
+        discussions: [],
         text: '',
         richText: '',
         metaData: {
@@ -209,6 +223,7 @@ export default {
         type: 'match',
         oldLine: null,
         newLine: null,
+        discussions: [],
         text: '',
         richText: '',
         metaData: {