Loading Discussions later on Diffs

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

@@ -59,7 +59,7 @@ export default {
       emailPatchPath: state => state.diffs.emailPatchPath,
     }),
     ...mapGetters('diffs', ['isParallelView']),
-    ...mapGetters(['isNotesFetched']),
+    ...mapGetters(['isNotesFetched', 'discussionsStructuredByLineCode']),
     targetBranch() {
       return {
         branchName: this.targetBranchName,
@@ -112,13 +112,32 @@ export default {
   },
   created() {
     this.adjustView();
+    eventHub.$once('renderedFiles', this.assignDiscussionsToDiff);
   },
   methods: {
-    ...mapActions('diffs', ['setBaseConfig', 'fetchDiffFiles', 'startRenderDiffsQueue']),
+    ...mapActions('diffs', [
+      'setBaseConfig',
+      'fetchDiffFiles',
+      'startRenderDiffsQueue',
+      'assignDiscussionsToDiff',
+    ]),
+
     fetchData() {
       this.fetchDiffFiles()
         .then(() => {
-          requestIdleCallback(this.startRenderDiffsQueue, { timeout: 1000 });
+          requestIdleCallback(
+            () => {
+              this.startRenderDiffsQueue()
+                .then(() => {
+                  console.log('Done rendering Que');
+                  this.assignDiscussionsToDiff(this.discussionsStructuredByLineCode);
+                })
+                .catch(() => {
+                  createFlash(__('Something went wrong on our end. Please try again!'));
+                });
+            },
+            { timeout: 1000 },
+          );
         })
         .catch(() => {
           createFlash(__('Something went wrong on our end. Please try again!'));

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

@@ -21,51 +21,47 @@ 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,
     }),
     leftLineCode() {
-      return this.line.left.lineCode;
+      return this.line.left && this.line.left.lineCode;
     },
     rightLineCode() {
-      return this.line.right.lineCode;
+      return this.line.right && this.line.right.lineCode;
     },
     hasExpandedDiscussionOnLeft() {
-      const discussions = this.leftDiscussions;
-
-      return discussions ? discussions.every(discussion => discussion.expanded) : false;
+      return this.line.left && this.line.left.discussions
+        ? this.line.left.discussions.every(discussion => discussion.expanded)
+        : false;
     },
     hasExpandedDiscussionOnRight() {
-      const discussions = this.rightDiscussions;
-
-      return discussions ? discussions.every(discussion => discussion.expanded) : false;
+      return this.line.right && this.line.right.discussions
+        ? this.line.right.discussions.every(discussion => discussion.expanded)
+        : false;
     },
     hasAnyExpandedDiscussion() {
       return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight;
     },
     shouldRenderDiscussionsOnLeft() {
-      return this.leftDiscussions && this.hasExpandedDiscussionOnLeft;
+      return this.line.left && this.line.left.discussions && this.hasExpandedDiscussionOnLeft;
     },
     shouldRenderDiscussionsOnRight() {
-      return this.rightDiscussions && this.hasExpandedDiscussionOnRight && this.line.right.type;
+      return (
+        this.line.right &&
+        this.line.right.discussions &&
+        this.hasExpandedDiscussionOnRight &&
+        this.line.right.type
+      );
     },
     showRightSideCommentForm() {
       return this.line.right.type && this.diffLineCommentForms[this.rightLineCode];
     },
     className() {
-      return this.leftDiscussions.length > 0 || this.rightDiscussions.length > 0
+      return (this.left && this.line.left.discussions.length > 0) ||
+        (this.right && this.line.right.discussions.length > 0)
         ? ''
         : 'js-temp-notes-holder';
     },
@@ -85,8 +81,8 @@ export default {
         class="content"
       >
         <diff-discussions
-          v-if="leftDiscussions.length"
-          :discussions="leftDiscussions"
+          v-if="line.left.discussions.length"
+          :discussions="line.left.discussions"
         />
       </div>
       <diff-line-note-form
@@ -104,8 +100,8 @@ export default {
         class="content"
       >
         <diff-discussions
-          v-if="rightDiscussions.length"
-          :discussions="rightDiscussions"
+          v-if="line.right.discussions.length"
+          :discussions="line.right.discussions"
         />
       </div>
       <diff-line-note-form

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

@@ -32,8 +32,9 @@ export default {
   },
   methods: {
     discussionsByLine(line, leftOrRight) {
-      return line[leftOrRight] && line[leftOrRight].lineCode !== undefined ?
-            this.singleDiscussionByLineCode(line[leftOrRight].lineCode) : [];
+      return line[leftOrRight] && line[leftOrRight].lineCode !== undefined
+        ? this.singleDiscussionByLineCode(line[leftOrRight].lineCode)
+        : [];
     },
   },
 };
@@ -56,18 +57,14 @@ export default {
             :line="line"
             :is-bottom="index + 1 === diffLinesLength"
             :key="index"
-            :left-discussions="discussionsByLine(line, 'left')"
-            :right-discussions="discussionsByLine(line, 'right')"
           />
-          <!--<parallel-diff-comment-row
+          <parallel-diff-comment-row
             v-if="shouldRenderParallelCommentRow(line)"
             :key="`dcr-${index}`"
             :line="line"
             :diff-file-hash="diffFile.fileHash"
             :line-index="index"
-            :left-discussions="discussionsByLine(line, 'left')"
-            :right-discussions="discussionsByLine(line, 'right')"
-          />-->
+          />
         </template>
       </tbody>
     </table>

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

@@ -29,25 +29,64 @@ export const fetchDiffFiles = ({ state, commit }) => {
     .then(handleLocationHash);
 };
 
+export const assignDiscussionsToDiff = ({ state, commit }, allLineDiscussions) => {
+  console.log('DIFF : ', state.diffFiles);
+  console.log('STATE : ', allLineDiscussions);
+
+  Object.values(allLineDiscussions).forEach(discussions => {
+    if (discussions.length > 0) {
+      console.log('KE : ', discussions);
+      const fileHash = discussions[0].fileHash;
+      const selectedFile = state.diffFiles.find(file => file.fileHash === fileHash);
+      console.log('FILE : ', selectedFile);
+      if (selectedFile) {
+        const targetLine = selectedFile.parallelDiffLines.find(line => {
+          return (
+            (line.left && line.left.lineCode === discussions[0].line_code) ||
+            (line.right && line.right.lineCode === discussions[0].line_code)
+          );
+        });
+
+        if (targetLine) {
+          console.log('TARGET LINE : ', targetLine);
+          Object.assign(targetLine.right, { discussions });
+        }
+      }
+    }
+  });
+};
+
 export const startRenderDiffsQueue = ({ state, commit }) => {
   const checkItem = () => {
-    const nextFile = state.diffFiles.find(
-      file => !file.renderIt && (!file.collapsed || !file.text),
-    );
-    if (nextFile) {
-      requestAnimationFrame(() => {
-        commit(types.RENDER_FILE, nextFile);
-      });
-      requestIdleCallback(
-        () => {
-          checkItem();
-        },
-        { timeout: 1000 },
+    return new Promise(resolve => {
+      const nextFile = state.diffFiles.find(
+        file => !file.renderIt && (!file.collapsed || !file.text),
       );
-    }
+
+      if (nextFile) {
+        requestAnimationFrame(() => {
+          commit(types.RENDER_FILE, nextFile);
+        });
+        requestIdleCallback(
+          () => {
+            checkItem()
+              .then(resolve)
+              .catch(() => {});
+          },
+          { timeout: 1000 },
+        );
+      } else {
+        console.log('No more items found -> Done');
+        resolve();
+      }
+    });
   };
 
-  checkItem();
+  return new Promise(resolve => {
+    checkItem()
+      .then(resolve)
+      .catch(() => {});
+  });
 };
 
 export const setInlineDiffViewType = ({ commit }) => {

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

@@ -75,26 +75,63 @@ export const singleDiscussionByLineCodeOld = (
   return discussions[lineCode] || [];
 };
 
-export const shouldRenderParallelCommentRowOld = (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;
+/**
+ * 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
+ * comparisions. `note.position.formatter` have the current version diff refs but
+ * `note.original_position.formatter` will have the first version's diff refs.
+ * If line diff refs matches with one of them, we should render it as a discussion on Changes tab.
+ *
+ * @param {Object} diff
+ * @returns {Array}
+ */
+export const discussionsByLineCode = (state, getters, rootState, rootGetters) => {
+  const diffRefsByLineCode = getDiffRefsByLineCode(state.diffFiles);
+
+  return rootGetters.discussions.reduce((acc, note) => {
+    const isDiffDiscussion = note.diff_discussion;
+    const hasLineCode = note.line_code;
+    const isResolvable = note.resolvable;
+    const diffRefs = diffRefsByLineCode[note.line_code];
+
+    if (isDiffDiscussion && hasLineCode && isResolvable && diffRefs) {
+      const refs = convertObjectPropsToCamelCase(note.position.formatter);
+      const originalRefs = convertObjectPropsToCamelCase(note.original_position.formatter);
+
+      if (_.isEqual(refs, diffRefs) || _.isEqual(originalRefs, diffRefs)) {
+        const lineCode = note.line_code;
+
+        if (acc[lineCode]) {
+          acc[lineCode].push(note);
+        } else {
+          acc[lineCode] = [note];
+        }
+      }
+    }
+
+    return acc;
+  }, {});
+};
+
+export const shouldRenderParallelCommentRow = (state, getters) => line => {
+  const hasDiscussion =
+    (line.left && line.left.discussions.length) || (line.right && line.right.discussions.length);
+
+  const hasExpandedDiscussionOnLeft =
+    line.left && line.left.discussions.length
+      ? line.left.discussions.every(discussion => discussion.expanded)
+      : false;
+  const hasExpandedDiscussionOnRight =
+    line.right && line.right.discussions.length
+      ? line.right.discussions.every(discussion => discussion.expanded)
+      : false;
 
   if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) {
     return true;
   }
 
-  const hasCommentFormOnLeft = state.diffLineCommentForms[leftLineCode];
-  const hasCommentFormOnRight = state.diffLineCommentForms[rightLineCode];
+  const hasCommentFormOnLeft = line.left && state.diffLineCommentForms[line.left.lineCode];
+  const hasCommentFormOnRight = line.right && state.diffLineCommentForms[line.right.lineCode];
 
   return hasCommentFormOnLeft || hasCommentFormOnRight;
 };

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

@@ -162,6 +162,7 @@ export function addContextLines(options) {
  */
 export function trimFirstCharOfLineContent(line = {}) {
   delete line.text;
+  line.discussions = [];
 
   const parsedLine = Object.assign({}, line);
 

app/assets/javascripts/notes/stores/getters.js

@@ -28,11 +28,16 @@ export const notesById = state =>
     return acc;
   }, {});
 
-export const discussionsByLineCode = state =>
+export const discussionsStructuredByLineCode = state =>
   state.discussions.reduce((acc, note) => {
     if (note.diff_discussion && note.line_code && note.resolvable) {
       // 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 });
+        // delete note.diff_file;
+      }
+
       items.push(note);
 
       Object.assign(acc, { [note.line_code]: items });