Merge branch 'ph/236143/useUtilMethodForParallelDifflines' into 'master'

Override the parallel diff lines property with new data

See merge request gitlab-org/gitlab!40131

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

@@ -203,7 +203,7 @@ export default {
       }
     },
     diffViewType() {
-      if (this.needsReload() || this.needsFirstLoad()) {
+      if (!this.glFeatures.unifiedDiffLines && (this.needsReload() || this.needsFirstLoad())) {
         this.refetchDiffData();
       }
       this.adjustView();

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

 <script>
 import { mapActions, mapGetters, mapState } from 'vuex';
 import { GlLoadingIcon } from '@gitlab/ui';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import diffLineNoteFormMixin from '~/notes/mixins/diff_line_note_form';
 import draftCommentsMixin from '~/diffs/mixins/draft_comments';
 import DiffViewer from '~/vue_shared/components/diff_viewer/diff_viewer.vue';
@@ -32,7 +33,7 @@ export default {
     userAvatarLink,
     DiffFileDrafts,
   },
-  mixins: [diffLineNoteFormMixin, draftCommentsMixin],
+  mixins: [diffLineNoteFormMixin, draftCommentsMixin, glFeatureFlagsMixin()],
   props: {
     diffFile: {
       type: Object,
@@ -114,7 +115,11 @@ export default {
         <inline-diff-view
           v-if="isInlineView"
           :diff-file="diffFile"
-          :diff-lines="diffFile.highlighted_diff_lines || []"
+          :diff-lines="
+            glFeatures.unifiedDiffLines
+              ? diffFile.parallel_diff_lines
+              : diffFile.highlighted_diff_lines || []
+          "
           :help-page-path="helpPagePath"
         />
         <parallel-diff-view

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

@@ -3,6 +3,7 @@ import { mapState, mapActions } from 'vuex';
 import { GlIcon } from '@gitlab/ui';
 import { deprecatedCreateFlash as createFlash } from '~/flash';
 import { s__ } from '~/locale';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import { UNFOLD_COUNT, INLINE_DIFF_VIEW_TYPE, PARALLEL_DIFF_VIEW_TYPE } from '../constants';
 import * as utils from '../store/utils';
 import tooltip from '../../vue_shared/directives/tooltip';
@@ -28,6 +29,7 @@ export default {
   components: {
     GlIcon,
   },
+  mixins: [glFeatureFlagsMixin()],
   props: {
     fileHash: {
       type: String,
@@ -59,7 +61,11 @@ export default {
   },
   computed: {
     ...mapState({
-      diffViewType: state => state.diffs.diffViewType,
+      diffViewType(state) {
+        return this.glFeatures.unifiedDiffLines
+          ? PARALLEL_DIFF_VIEW_TYPE
+          : state.diffs.diffViewType;
+      },
       diffFiles: state => state.diffs.diffFiles,
     }),
     canExpandUp() {

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

 <script>
 import { mapGetters, mapState } from 'vuex';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import draftCommentsMixin from '~/diffs/mixins/draft_comments';
 import InlineDraftCommentRow from '~/batch_comments/components/inline_draft_comment_row.vue';
 import inlineDiffTableRow from './inline_diff_table_row.vue';
@@ -14,7 +15,7 @@ export default {
     InlineDraftCommentRow,
     inlineDiffExpansionRow,
   },
-  mixins: [draftCommentsMixin],
+  mixins: [draftCommentsMixin, glFeatureFlagsMixin()],
   props: {
     diffFile: {
       type: Object,
@@ -63,6 +64,67 @@ export default {
       <col />
     </colgroup>
     <tbody>
+      <template v-if="glFeatures.unifiedDiffLines">
+        <template v-for="({ left, right }, index) in diffLines">
+          <inline-diff-expansion-row
+            :key="`expand-${index}`"
+            :file-hash="diffFile.file_hash"
+            :context-lines-path="diffFile.context_lines_path"
+            :line="left || right"
+            :is-top="index === 0"
+            :is-bottom="index + 1 === diffLinesLength"
+          />
+          <template v-if="left">
+            <inline-diff-table-row
+              :key="`${left.line_code || index}`"
+              :file-hash="diffFile.file_hash"
+              :file-path="diffFile.file_path"
+              :line="left"
+              :is-bottom="index + 1 === diffLinesLength"
+              :is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine"
+            />
+            <inline-diff-comment-row
+              :key="`icr-${left.line_code || index}`"
+              :diff-file-hash="diffFile.file_hash"
+              :line="left"
+              :help-page-path="helpPagePath"
+              :has-draft="shouldRenderDraftRow(diffFile.file_hash, left) || false"
+            />
+            <inline-draft-comment-row
+              v-if="shouldRenderDraftRow(diffFile.file_hash, left)"
+              :key="`draft_${index}`"
+              :draft="draftForLine(diffFile.file_hash, left)"
+              :diff-file="diffFile"
+              :line="left"
+            />
+          </template>
+          <template v-if="right && right.type === 'new'">
+            <inline-diff-table-row
+              :key="`new-${right.line_code || index}`"
+              :file-hash="diffFile.file_hash"
+              :file-path="diffFile.file_path"
+              :line="right"
+              :is-bottom="index + 1 === diffLinesLength"
+              :is-commented="index >= commentedLines.startLine && index <= commentedLines.endLine"
+            />
+            <inline-diff-comment-row
+              :key="`new-icr-${right.line_code || index}`"
+              :diff-file-hash="diffFile.file_hash"
+              :line="right"
+              :help-page-path="helpPagePath"
+              :has-draft="shouldRenderDraftRow(diffFile.file_hash, right) || false"
+            />
+            <inline-draft-comment-row
+              v-if="shouldRenderDraftRow(diffFile.file_hash, right)"
+              :key="`new-draft_${index}`"
+              :draft="draftForLine(diffFile.file_hash, right)"
+              :diff-file="diffFile"
+              :line="right"
+            />
+          </template>
+        </template>
+      </template>
+      <template v-else>
         <template v-for="(line, index) in diffLines">
           <inline-diff-expansion-row
             :key="`expand-${index}`"
@@ -95,6 +157,7 @@ export default {
             :line="line"
           />
         </template>
+      </template>
     </tbody>
   </table>
 </template>

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

@@ -74,7 +74,7 @@ export const fetchDiffFiles = ({ state, commit }) => {
   let returnData;
 
   if (state.useSingleDiffStyle) {
-    urlParams.view = state.diffViewType;
+    urlParams.view = window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType;
   }
 
   commit(types.SET_LOADING, true);
@@ -114,7 +114,7 @@ export const fetchDiffFilesBatch = ({ commit, state, dispatch }) => {
   };
 
   if (state.useSingleDiffStyle) {
-    urlParams.view = state.diffViewType;
+    urlParams.view = window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType;
   }
 
   commit(types.SET_BATCH_LOADING, true);
@@ -178,7 +178,7 @@ export const fetchDiffFilesMeta = ({ commit, state }) => {
   const urlParams = {};
 
   if (state.useSingleDiffStyle) {
-    urlParams.view = state.diffViewType;
+    urlParams.view = window.gon?.features?.unifiedDiffLines ? 'inline' : state.diffViewType;
   }
 
   commit(types.SET_LOADING, true);

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

 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
+import { PARALLEL_DIFF_VIEW_TYPE } from '../constants';
 import {
   findDiffFile,
   addLineReferences,
@@ -154,7 +155,9 @@ export default {
     addContextLines({
       inlineLines: diffFile.highlighted_diff_lines,
       parallelLines: diffFile.parallel_diff_lines,
-      diffViewType: state.diffViewType,
+      diffViewType: window.gon?.features?.unifiedDiffLines
+        ? PARALLEL_DIFF_VIEW_TYPE
+        : state.diffViewType,
       contextLines: lines,
       bottom,
       lineNumbers,

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

@@ -20,6 +20,77 @@ import {
 } from '../constants';
 import { prepareRawDiffFile } from '../diff_file';
 
+export const isAdded = line => ['new', 'new-nonewline'].includes(line.type);
+export const isRemoved = line => ['old', 'old-nonewline'].includes(line.type);
+export const isUnchanged = line => !line.type;
+export const isMeta = line => ['match', 'new-nonewline', 'old-nonewline'].includes(line.type);
+
+/**
+ * Pass in the inline diff lines array which gets converted
+ * to the parallel diff lines.
+ * This allows for us to convert inline diff lines to parallel
+ * on the frontend without needing to send any requests
+ * to the API.
+ *
+ * This method has been taken from the already existing backend
+ * implementation at lib/gitlab/diff/parallel_diff.rb
+ *
+ * @param {Object[]} diffLines - inline diff lines
+ *
+ * @returns {Object[]} parallel lines
+ */
+export const parallelizeDiffLines = (diffLines = []) => {
+  let freeRightIndex = null;
+  const lines = [];
+
+  for (let i = 0, diffLinesLength = diffLines.length, index = 0; i < diffLinesLength; i += 1) {
+    const line = diffLines[i];
+
+    if (isRemoved(line)) {
+      lines.push({
+        [LINE_POSITION_LEFT]: line,
+        [LINE_POSITION_RIGHT]: null,
+      });
+
+      if (freeRightIndex === null) {
+        // Once we come upon a new line it can be put on the right of this old line
+        freeRightIndex = index;
+      }
+      index += 1;
+    } else if (isAdded(line)) {
+      if (freeRightIndex !== null) {
+        // If an old line came before this without a line on the right, this
+        // line can be put to the right of it.
+        lines[freeRightIndex].right = line;
+
+        // If there are any other old lines on the left that don't yet have
+        // a new counterpart on the right, update the free_right_index
+        const nextFreeRightIndex = freeRightIndex + 1;
+        freeRightIndex = nextFreeRightIndex < index ? nextFreeRightIndex : null;
+      } else {
+        lines.push({
+          [LINE_POSITION_LEFT]: null,
+          [LINE_POSITION_RIGHT]: line,
+        });
+
+        freeRightIndex = null;
+        index += 1;
+      }
+    } else if (isMeta(line) || isUnchanged(line)) {
+      // line in the right panel is the same as in the left one
+      lines.push({
+        [LINE_POSITION_LEFT]: line,
+        [LINE_POSITION_RIGHT]: line,
+      });
+
+      freeRightIndex = null;
+      index += 1;
+    }
+  }
+
+  return lines;
+};
+
 export function findDiffFile(files, match, matchKey = 'file_hash') {
   return files.find(file => file[matchKey] === match);
 }
@@ -280,6 +351,13 @@ function mergeTwoFiles(target, source) {
 }
 
 function ensureBasicDiffFileLines(file) {
+  if (window.gon?.features?.unifiedDiffLines) {
+    return Object.assign(file, {
+      highlighted_diff_lines: [],
+      parallel_diff_lines: parallelizeDiffLines(file.highlighted_diff_lines || []),
+    });
+  }
+
   const missingInline = !file.highlighted_diff_lines;
   const missingParallel = !file.parallel_diff_lines;
 
@@ -717,74 +795,3 @@ export const getDefaultWhitespace = (queryString, cookie) => {
   if (cookie === NO_SHOW_WHITESPACE) return false;
   return true;
 };
-
-export const isAdded = line => ['new', 'new-nonewline'].includes(line.type);
-export const isRemoved = line => ['old', 'old-nonewline'].includes(line.type);
-export const isUnchanged = line => !line.type;
-export const isMeta = line => ['match', 'new-nonewline', 'old-nonewline'].includes(line.type);
-
-/**
- * Pass in the inline diff lines array which gets converted
- * to the parallel diff lines.
- * This allows for us to convert inline diff lines to parallel
- * on the frontend without needing to send any requests
- * to the API.
- *
- * This method has been taken from the already existing backend
- * implementation at lib/gitlab/diff/parallel_diff.rb
- *
- * @param {Object[]} diffLines - inline diff lines
- *
- * @returns {Object[]} parallel lines
- */
-export const parallelizeDiffLines = (diffLines = []) => {
-  let freeRightIndex = null;
-  const lines = [];
-
-  for (let i = 0, diffLinesLength = diffLines.length, index = 0; i < diffLinesLength; i += 1) {
-    const line = diffLines[i];
-
-    if (isRemoved(line)) {
-      lines.push({
-        [LINE_POSITION_LEFT]: line,
-        [LINE_POSITION_RIGHT]: null,
-      });
-
-      if (freeRightIndex === null) {
-        // Once we come upon a new line it can be put on the right of this old line
-        freeRightIndex = index;
-      }
-      index += 1;
-    } else if (isAdded(line)) {
-      if (freeRightIndex !== null) {
-        // If an old line came before this without a line on the right, this
-        // line can be put to the right of it.
-        lines[freeRightIndex].right = line;
-
-        // If there are any other old lines on the left that don't yet have
-        // a new counterpart on the right, update the free_right_index
-        const nextFreeRightIndex = freeRightIndex + 1;
-        freeRightIndex = nextFreeRightIndex < index ? nextFreeRightIndex : null;
-      } else {
-        lines.push({
-          [LINE_POSITION_LEFT]: null,
-          [LINE_POSITION_RIGHT]: line,
-        });
-
-        freeRightIndex = null;
-        index += 1;
-      }
-    } else if (isMeta(line) || isUnchanged(line)) {
-      // line in the right panel is the same as in the left one
-      lines.push({
-        [LINE_POSITION_LEFT]: line,
-        [LINE_POSITION_RIGHT]: line,
-      });
-
-      freeRightIndex = null;
-      index += 1;
-    }
-  }
-
-  return lines;
-};

app/controllers/projects/merge_requests_controller.rb

@@ -40,6 +40,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
     push_frontend_feature_flag(:approvals_commented_by, @project, default_enabled: true)
     push_frontend_feature_flag(:hide_jump_to_next_unresolved_in_threads, default_enabled: true)
     push_frontend_feature_flag(:merge_request_widget_graphql, @project)
+    push_frontend_feature_flag(:unified_diff_lines, @project)
   end
 
   before_action do

config/feature_flags/development/unified_diff_lines.yml

+---
+name: unified_diff_lines
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/40131
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/241188
+group: group::source code
+type: development
+default_enabled: false

spec/spec_helper.rb

@@ -194,6 +194,10 @@ RSpec.configure do |config|
       stub_feature_flags(vue_issuable_sidebar: false)
       stub_feature_flags(vue_issuable_epic_sidebar: false)
 
+      # The following can be removed once we are confident the
+      # unified diff lines works as expected
+      stub_feature_flags(unified_diff_lines: false)
+
       enable_rugged = example.metadata[:enable_rugged].present?
 
       # Disable Rugged features by default