Revert "Merge branch 'tz-mr-refactor-memory-reduction' into 'master'"

This reverts commit 2411ecb5, reversing
changes made to 58a0df7e.

app/assets/javascripts/diffs/components/diff_line_gutter_content.vue

@@ -71,11 +71,6 @@ export default {
       required: false,
       default: false,
     },
-    discussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
   },
   computed: {
     ...mapState({
@@ -83,6 +78,7 @@ export default {
       diffFiles: state => state.diffs.diffFiles,
     }),
     ...mapGetters(['isLoggedIn']),
+    ...mapGetters('diffs', ['discussionsByLineCode']),
     lineHref() {
       return this.lineCode ? `#${this.lineCode}` : '#';
     },
@@ -92,19 +88,24 @@ export default {
         this.showCommentButton &&
         !this.isMatchLine &&
         !this.isContextLine &&
-        !this.isMetaLine &&
-        !this.hasDiscussions
+        !this.hasDiscussions &&
+        !this.isMetaLine
       );
     },
+    discussions() {
+      return this.discussionsByLineCode[this.lineCode] || [];
+    },
     hasDiscussions() {
       return this.discussions.length > 0;
     },
     shouldShowAvatarsOnGutter() {
+      let render = this.hasDiscussions && this.showCommentButton;
+
       if (!this.lineType && this.linePosition === LINE_POSITION_RIGHT) {
-        return false;
+        render = false;
       }
 
-      return this.hasDiscussions && this.showCommentButton;
+      return render;
     },
   },
   methods: {

app/assets/javascripts/diffs/components/diff_table_cell.vue

@@ -67,11 +67,6 @@ export default {
       required: false,
       default: false,
     },
-    discussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
   },
   computed: {
     ...mapGetters(['isLoggedIn']),
@@ -141,7 +136,6 @@ export default {
       :is-match-line="isMatchLine"
       :is-context-line="isContentLine"
       :is-meta-line="isMetaLine"
-      :discussions="discussions"
     />
   </td>
 </template>

app/assets/javascripts/diffs/components/inline_diff_comment_row.vue

 <script>
-import { mapState } from 'vuex';
+import { mapState, mapGetters } from 'vuex';
 import diffDiscussions from './diff_discussions.vue';
 import diffLineNoteForm from './diff_line_note_form.vue';
 
@@ -21,16 +21,15 @@ export default {
       type: Number,
       required: true,
     },
-    discussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
   },
   computed: {
     ...mapState({
       diffLineCommentForms: state => state.diffs.diffLineCommentForms,
     }),
+    ...mapGetters('diffs', ['discussionsByLineCode']),
+    discussions() {
+      return this.discussionsByLineCode[this.line.lineCode] || [];
+    },
     className() {
       return this.discussions.length ? '' : 'js-temp-notes-holder';
     },

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

app/assets/javascripts/diffs/components/inline_diff_view.vue

@@ -20,11 +20,7 @@ export default {
     },
   },
   computed: {
-    ...mapGetters('diffs', [
-      'commitId',
-      'shouldRenderInlineCommentRow',
-      'singleDiscussionByLineCode',
-    ]),
+    ...mapGetters('diffs', ['commitId', 'discussionsByLineCode']),
     ...mapState({
       diffLineCommentForms: state => state.diffs.diffLineCommentForms,
     }),
@@ -38,7 +34,18 @@ export default {
       return window.gon.user_color_scheme;
     },
   },
-  methods: {},
+  methods: {
+    shouldRenderCommentRow(line) {
+      if (this.diffLineCommentForms[line.lineCode]) return true;
+
+      const lineDiscussions = this.discussionsByLineCode[line.lineCode];
+      if (lineDiscussions === undefined) {
+        return false;
+      }
+
+      return lineDiscussions.every(discussion => discussion.expanded);
+    },
+  },
 };
 </script>
 
@@ -57,15 +64,13 @@ export default {
           :line="line"
           :is-bottom="index + 1 === diffLinesLength"
           :key="line.lineCode"
-          :discussions="singleDiscussionByLineCode(line.lineCode)"
         />
         <inline-diff-comment-row
-          v-if="shouldRenderInlineCommentRow(line)"
+          v-if="shouldRenderCommentRow(line)"
           :diff-file-hash="diffFile.fileHash"
           :line="line"
           :line-index="index"
           :key="index"
-          :discussions="singleDiscussionByLineCode(line.lineCode)"
         />
       </template>
     </tbody>

app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue

 <script>
-import { mapState } from 'vuex';
+import { mapState, mapGetters } from 'vuex';
 import diffDiscussions from './diff_discussions.vue';
 import diffLineNoteForm from './diff_line_note_form.vue';
 
@@ -21,51 +21,48 @@ export default {
       type: Number,
       required: true,
     },
-    leftDiscussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
-    rightDiscussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
   },
   computed: {
     ...mapState({
       diffLineCommentForms: state => state.diffs.diffLineCommentForms,
     }),
+    ...mapGetters('diffs', ['discussionsByLineCode']),
     leftLineCode() {
       return this.line.left.lineCode;
     },
     rightLineCode() {
       return this.line.right.lineCode;
     },
+    hasDiscussion() {
+      const discussions = this.discussionsByLineCode;
+
+      return discussions[this.leftLineCode] || discussions[this.rightLineCode];
+    },
     hasExpandedDiscussionOnLeft() {
-      const discussions = this.leftDiscussions;
+      const discussions = this.discussionsByLineCode[this.leftLineCode];
+
       return discussions ? discussions.every(discussion => discussion.expanded) : false;
     },
     hasExpandedDiscussionOnRight() {
-      const discussions = this.rightDiscussions;
+      const discussions = this.discussionsByLineCode[this.rightLineCode];
+
       return discussions ? discussions.every(discussion => discussion.expanded) : false;
     },
     hasAnyExpandedDiscussion() {
       return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight;
     },
     shouldRenderDiscussionsOnLeft() {
-      return this.leftDiscussions && this.hasExpandedDiscussionOnLeft;
+      return this.discussionsByLineCode[this.leftLineCode] && this.hasExpandedDiscussionOnLeft;
     },
     shouldRenderDiscussionsOnRight() {
-      return this.rightDiscussions && this.hasExpandedDiscussionOnRight && this.line.right.type;
-    },
-    showRightSideCommentForm() {
-      return this.line.right.type && this.diffLineCommentForms[this.rightLineCode];
+      return (
+        this.discussionsByLineCode[this.rightLineCode] &&
+        this.hasExpandedDiscussionOnRight &&
+        this.line.right.type
+      );
     },
     className() {
-      return this.leftDiscussions.length > 0 || this.rightDiscussions.length > 0
-        ? ''
-        : 'js-temp-notes-holder';
+      return this.hasDiscussion ? '' : 'js-temp-notes-holder';
     },
   },
 };
@@ -83,12 +80,13 @@ export default {
         class="content"
       >
         <diff-discussions
-          v-if="leftDiscussions.length"
-          :discussions="leftDiscussions"
+          v-if="discussionsByLineCode[leftLineCode].length"
+          :discussions="discussionsByLineCode[leftLineCode]"
         />
       </div>
       <diff-line-note-form
-        v-if="diffLineCommentForms[leftLineCode]"
+        v-if="diffLineCommentForms[leftLineCode] &&
+        diffLineCommentForms[leftLineCode]"
         :diff-file-hash="diffFileHash"
         :line="line.left"
         :note-target-line="line.left"
@@ -102,12 +100,13 @@ export default {
         class="content"
       >
         <diff-discussions
-          v-if="rightDiscussions.length"
-          :discussions="rightDiscussions"
+          v-if="discussionsByLineCode[rightLineCode].length"
+          :discussions="discussionsByLineCode[rightLineCode]"
         />
       </div>
       <diff-line-note-form
-        v-if="showRightSideCommentForm"
+        v-if="diffLineCommentForms[rightLineCode] &&
+        diffLineCommentForms[rightLineCode] && line.right.type"
         :diff-file-hash="diffFileHash"
         :line="line.right"
         :note-target-line="line.right"

app/assets/javascripts/diffs/components/parallel_diff_table_row.vue

@@ -36,16 +36,6 @@ export default {
       required: false,
       default: false,
     },
-    leftDiscussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
-    rightDiscussions: {
-      type: Array,
-      required: false,
-      default: () => [],
-    },
   },
   data() {
     return {
@@ -126,7 +116,6 @@ export default {
       :is-hover="isLeftHover"
       :show-comment-button="true"
       :diff-view-type="parallelDiffViewType"
-      :discussions="leftDiscussions"
       class="diff-line-num old_line"
     />
     <td
@@ -147,7 +136,6 @@ export default {
       :is-hover="isRightHover"
       :show-comment-button="true"
       :diff-view-type="parallelDiffViewType"
-      :discussions="rightDiscussions"
       class="diff-line-num new_line"
     />
     <td

app/assets/javascripts/diffs/components/parallel_diff_view.vue

@@ -21,11 +21,7 @@ export default {
     },
   },
   computed: {
-    ...mapGetters('diffs', [
-      'commitId',
-      'singleDiscussionByLineCode',
-      'shouldRenderParallelCommentRow',
-    ]),
+    ...mapGetters('diffs', ['commitId', 'discussionsByLineCode']),
     ...mapState({
       diffLineCommentForms: state => state.diffs.diffLineCommentForms,
     }),
@@ -55,6 +51,32 @@ export default {
       return window.gon.user_color_scheme;
     },
   },
+  methods: {
+    shouldRenderCommentRow(line) {
+      const leftLineCode = line.left.lineCode;
+      const rightLineCode = line.right.lineCode;
+      const discussions = this.discussionsByLineCode;
+      const leftDiscussions = discussions[leftLineCode];
+      const rightDiscussions = discussions[rightLineCode];
+      const hasDiscussion = leftDiscussions || rightDiscussions;
+
+      const hasExpandedDiscussionOnLeft = leftDiscussions
+        ? leftDiscussions.every(discussion => discussion.expanded)
+        : false;
+      const hasExpandedDiscussionOnRight = rightDiscussions
+        ? rightDiscussions.every(discussion => discussion.expanded)
+        : false;
+
+      if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) {
+        return true;
+      }
+
+      const hasCommentFormOnLeft = this.diffLineCommentForms[leftLineCode];
+      const hasCommentFormOnRight = this.diffLineCommentForms[rightLineCode];
+
+      return hasCommentFormOnLeft || hasCommentFormOnRight;
+    },
+  },
 };
 </script>
 
@@ -75,17 +97,13 @@ export default {
             :line="line"
             :is-bottom="index + 1 === diffLinesLength"
             :key="index"
-            :left-discussions="singleDiscussionByLineCode(line.left.lineCode)"
-            :right-discussions="singleDiscussionByLineCode(line.right.lineCode)"
           />
           <parallel-diff-comment-row
-            v-if="shouldRenderParallelCommentRow(line)"
+            v-if="shouldRenderCommentRow(line)"
             :key="`dcr-${index}`"
             :line="line"
             :diff-file-hash="diffFile.fileHash"
             :line-index="index"
-            :left-discussions="singleDiscussionByLineCode(line.left.lineCode)"
-            :right-discussions="singleDiscussionByLineCode(line.right.lineCode)"
           />
         </template>
       </tbody>

app/assets/javascripts/diffs/store/getters.js

@@ -75,10 +75,9 @@ export const discussionsByLineCode = (state, getters, rootState, rootGetters) =>
     const isDiffDiscussion = note.diff_discussion;
     const hasLineCode = note.line_code;
     const isResolvable = note.resolvable;
-
-    if (isDiffDiscussion && hasLineCode && isResolvable) {
     const diffRefs = diffRefsByLineCode[note.line_code];
-      if (diffRefs) {
+
+    if (isDiffDiscussion && hasLineCode && isResolvable && diffRefs) {
       const refs = convertObjectPropsToCamelCase(note.position.formatter);
       const originalRefs = convertObjectPropsToCamelCase(note.original_position.formatter);
 
@@ -92,53 +91,11 @@ export const discussionsByLineCode = (state, getters, rootState, rootGetters) =>
         }
       }
     }
-    }
 
     return acc;
   }, {});
 };
 
-export const singleDiscussionByLineCode = (state, getters) => lineCode => {
-  if (!lineCode) return [];
-  const discussions = getters.discussionsByLineCode;
-  return discussions[lineCode] || [];
-};
-
-export const shouldRenderParallelCommentRow = (state, getters) => line => {
-  const leftLineCode = line.left.lineCode;
-  const rightLineCode = line.right.lineCode;
-  const leftDiscussions = getters.singleDiscussionByLineCode(leftLineCode);
-  const rightDiscussions = getters.singleDiscussionByLineCode(rightLineCode);
-  const hasDiscussion = leftDiscussions.length || rightDiscussions.length;
-
-  const hasExpandedDiscussionOnLeft = leftDiscussions.length
-    ? leftDiscussions.every(discussion => discussion.expanded)
-    : false;
-  const hasExpandedDiscussionOnRight = rightDiscussions.length
-    ? rightDiscussions.every(discussion => discussion.expanded)
-    : false;
-
-  if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) {
-    return true;
-  }
-
-  const hasCommentFormOnLeft = state.diffLineCommentForms[leftLineCode];
-  const hasCommentFormOnRight = state.diffLineCommentForms[rightLineCode];
-
-  return hasCommentFormOnLeft || hasCommentFormOnRight;
-};
-
-export const shouldRenderInlineCommentRow = (state, getters) => line => {
-  if (state.diffLineCommentForms[line.lineCode]) return true;
-
-  const lineDiscussions = getters.singleDiscussionByLineCode(line.lineCode);
-  if (lineDiscussions.length === 0) {
-    return false;
-  }
-
-  return lineDiscussions.every(discussion => discussion.expanded);
-};
-
 // prevent babel-plugin-rewire from generating an invalid default during karma∂ tests
 export const getDiffFileByHash = state => fileHash =>
   state.diffFiles.find(file => file.fileHash === fileHash);

spec/javascripts/diffs/components/diff_line_gutter_content_spec.js

@@ -50,11 +50,7 @@ describe('DiffLineGutterContent', () => {
 
       it('should return discussions for the given lineCode', () => {
         const { lineCode } = getDiffFileMock().highlightedDiffLines[1];
-        const component = createComponent({
-          lineCode,
-          showCommentButton: true,
-          discussions: getDiscussionsMockData(),
-        });
+        const component = createComponent({ lineCode, showCommentButton: true });
 
         setDiscussions(component);