Merge branch 'ph/281171/diffFileConflictAlert' into 'master'

Added warning to diff files when there are conflicts

See merge request gitlab-org/gitlab!67216

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

@@ -14,9 +14,11 @@ import {
 } from '~/behaviors/shortcuts/keybindings';
 import createFlash from '~/flash';
 import { isSingleViewStyle } from '~/helpers/diffs_helper';
+import { helpPagePath } from '~/helpers/help_page_helper';
 import { parseBoolean } from '~/lib/utils/common_utils';
 import { updateHistory } from '~/lib/utils/url_utility';
 import { __ } from '~/locale';
+import MrWidgetHowToMergeModal from '~/vue_merge_request_widget/components/mr_widget_how_to_merge_modal.vue';
 import PanelResizer from '~/vue_shared/components/panel_resizer.vue';
 
 import notesEventHub from '../../notes/event_hub';
@@ -52,7 +54,6 @@ import CommitWidget from './commit_widget.vue';
 import CompareVersions from './compare_versions.vue';
 import DiffFile from './diff_file.vue';
 import HiddenFilesWarning from './hidden_files_warning.vue';
-import MergeConflictWarning from './merge_conflict_warning.vue';
 import NoChanges from './no_changes.vue';
 import PreRenderer from './pre_renderer.vue';
 import TreeList from './tree_list.vue';
@@ -65,7 +66,6 @@ export default {
     DiffFile,
     NoChanges,
     HiddenFilesWarning,
-    MergeConflictWarning,
     CollapsedFilesWarning,
     CommitWidget,
     TreeList,
@@ -77,6 +77,7 @@ export default {
     DynamicScrollerItem,
     PreRenderer,
     VirtualScrollerScrollSync,
+    MrWidgetHowToMergeModal,
   },
   alerts: {
     ALERT_OVERFLOW_HIDDEN,
@@ -164,6 +165,21 @@ export default {
       required: false,
       default: () => ({}),
     },
+    sourceProjectDefaultUrl: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    sourceProjectFullPath: {
+      type: String,
+      required: false,
+      default: '',
+    },
+    isForked: {
+      type: Boolean,
+      required: false,
+      default: false,
+    },
   },
   data() {
     const treeWidth =
@@ -203,6 +219,8 @@ export default {
       'mrReviews',
       'renderTreeList',
       'showWhitespace',
+      'targetBranchName',
+      'branchName',
     ]),
     ...mapGetters('diffs', [
       'whichCollapsedTypes',
@@ -596,6 +614,9 @@ export default {
   },
   minTreeWidth: MIN_TREE_WIDTH,
   maxTreeWidth: MAX_TREE_WIDTH,
+  howToMergeDocsPath: helpPagePath('user/project/merge_requests/reviews/index.md', {
+    anchor: 'checkout-merge-requests-locally-through-the-head-ref',
+  }),
 };
 </script>
 
@@ -615,12 +636,6 @@ export default {
         :plain-diff-path="plainDiffPath"
         :email-patch-path="emailPatchPath"
       />
-      <merge-conflict-warning
-        v-if="visibleWarning == $options.alerts.ALERT_MERGE_CONFLICT"
-        :limited="isLimitedContainer"
-        :resolution-path="conflictResolutionPath"
-        :mergeable="canMerge"
-      />
       <collapsed-files-warning
         v-if="visibleWarning == $options.alerts.ALERT_COLLAPSED_FILES"
         :limited="isLimitedContainer"
@@ -733,6 +748,15 @@ export default {
           <no-changes v-else :changes-empty-state-illustration="changesEmptyStateIllustration" />
         </div>
       </div>
+      <mr-widget-how-to-merge-modal
+        :is-fork="isForked"
+        :can-merge="canMerge"
+        :source-branch="branchName"
+        :source-project-path="sourceProjectFullPath"
+        :target-branch="targetBranchName"
+        :source-project-default-url="sourceProjectDefaultUrl"
+        :reviewing-docs-path="$options.howToMergeDocsPath"
+      />
     </div>
   </div>
 </template>

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

 <script>
-import { GlButton, GlLoadingIcon, GlSafeHtmlDirective as SafeHtml, GlSprintf } from '@gitlab/ui';
+import {
+  GlButton,
+  GlLoadingIcon,
+  GlSafeHtmlDirective as SafeHtml,
+  GlSprintf,
+  GlAlert,
+  GlModalDirective,
+} from '@gitlab/ui';
 import { escape } from 'lodash';
 import { mapActions, mapGetters, mapState } from 'vuex';
 import { IdState } from 'vendor/vue-virtual-scroller';
@@ -19,7 +26,7 @@ import {
   EVT_PERF_MARK_FIRST_DIFF_FILE_SHOWN,
 } from '../constants';
 import eventHub from '../event_hub';
-import { DIFF_FILE, GENERIC_ERROR } from '../i18n';
+import { DIFF_FILE, GENERIC_ERROR, CONFLICT_TEXT } from '../i18n';
 import { collapsedType, getShortShaFromFile } from '../utils/diff_file';
 import DiffContent from './diff_content.vue';
 import DiffFileHeader from './diff_file_header.vue';
@@ -31,9 +38,11 @@ export default {
     GlButton,
     GlLoadingIcon,
     GlSprintf,
+    GlAlert,
   },
   directives: {
     SafeHtml,
+    GlModalDirective,
   },
   mixins: [glFeatureFlagsMixin(), IdState({ idProp: (vm) => vm.file.file_hash })],
   props: {
@@ -93,7 +102,12 @@ export default {
     genericError: GENERIC_ERROR,
   },
   computed: {
-    ...mapState('diffs', ['currentDiffFileId', 'codequalityDiff']),
+    ...mapState('diffs', [
+      'currentDiffFileId',
+      'codequalityDiff',
+      'conflictResolutionPath',
+      'canMerge',
+    ]),
     ...mapGetters(['isNotesFetched']),
     ...mapGetters('diffs', ['getDiffFileDiscussions', 'isVirtualScrollingEnabled']),
     viewBlobHref() {
@@ -312,6 +326,7 @@ export default {
       this.idState.forkMessageVisible = false;
     },
   },
+  CONFLICT_TEXT,
 };
 </script>
 
@@ -390,6 +405,55 @@ export default {
           <div v-else v-safe-html="errorMessage" class="nothing-here-block"></div>
         </div>
         <template v-else>
+          <gl-alert
+            v-if="file.conflict_type"
+            variant="danger"
+            :dismissible="false"
+            data-testid="conflictsAlert"
+          >
+            {{ $options.CONFLICT_TEXT[file.conflict_type] }}
+            <template v-if="!canMerge">
+              {{ __('Ask someone with write access to resolve it.') }}
+            </template>
+            <gl-sprintf
+              v-else-if="conflictResolutionPath"
+              :message="
+                __(
+                  'You can %{gitlabLinkStart}resolve conflicts on GitLab%{gitlabLinkEnd} or %{resolveLocallyStart}resolve it locally%{resolveLocallyEnd}.',
+                )
+              "
+            >
+              <template #gitlabLink="{ content }">
+                <gl-button
+                  :href="conflictResolutionPath"
+                  variant="link"
+                  class="gl-vertical-align-text-bottom"
+                  >{{ content }}</gl-button
+                >
+              </template>
+              <template #resolveLocally="{ content }">
+                <gl-button
+                  v-gl-modal-directive="'modal-merge-info'"
+                  variant="link"
+                  class="gl-vertical-align-text-bottom"
+                  >{{ content }}</gl-button
+                >
+              </template>
+            </gl-sprintf>
+            <gl-sprintf
+              v-else
+              :message="__('You can %{resolveLocallyStart}resolve it locally%{resolveLocallyEnd}.')"
+            >
+              <template #resolveLocally="{ content }">
+                <gl-button
+                  v-gl-modal-directive="'modal-merge-info'"
+                  variant="link"
+                  class="gl-vertical-align-text-bottom"
+                  >{{ content }}</gl-button
+                >
+              </template>
+            </gl-sprintf>
+          </gl-alert>
           <div
             v-if="showWarning"
             class="collapsed-file-warning gl-p-7 gl-bg-orange-50 gl-text-center gl-rounded-bottom-left-base gl-rounded-bottom-right-base"

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

@@ -134,12 +134,6 @@ export default {
   interopRightAttributes(props) {
     return getInteropNewSideAttributes(props.line.right);
   },
-  conflictText: memoize(
-    (line) => {
-      return line.type === CONFLICT_MARKER_THEIR ? 'HEAD//our changes' : 'origin//their changes';
-    },
-    (line) => line.type,
-  ),
   lineContent: (line) => {
     if (line.isConflictMarker) {
       return line.type === CONFLICT_MARKER_THEIR ? 'HEAD//our changes' : 'origin//their changes';

app/assets/javascripts/diffs/i18n.js

@@ -25,3 +25,25 @@ export const SETTINGS_DROPDOWN = {
   fileByFile: __('Show one file at a time'),
   preferences: __('Preferences'),
 };
+
+export const CONFLICT_TEXT = {
+  both_modified: __('Conflict: This file was modified in both the source and target branches.'),
+  modified_source_removed_target: __(
+    'Conflict: This file was modified in the source branch, but removed in the target branch.',
+  ),
+  modified_target_removed_source: __(
+    'Conflict: This file was removed in the source branch, but modified in the target branch.',
+  ),
+  renamed_same_file: __(
+    'Conflict: This file was renamed differently in the source and target branches.',
+  ),
+  removed_source_renamed_target: __(
+    'Conflict: This file was removed in the source branch, but renamed in the target branch.',
+  ),
+  removed_target_renamed_source: __(
+    'Conflict: This file was renamed in the source branch, but removed in the target branch.',
+  ),
+  both_added: __(
+    'Conflict: This file was added both in the source and target branches, but with different contents.',
+  ),
+};

app/assets/javascripts/diffs/index.js

@@ -83,6 +83,9 @@ export default function initDiffsApp(store) {
         showWhitespaceDefault: parseBoolean(dataset.showWhitespaceDefault),
         viewDiffsFileByFile: parseBoolean(dataset.fileByFileDefault),
         defaultSuggestionCommitMessage: dataset.defaultSuggestionCommitMessage,
+        sourceProjectDefaultUrl: dataset.sourceProjectDefaultUrl,
+        sourceProjectFullPath: dataset.sourceProjectFullPath,
+        isForked: parseBoolean(dataset.isForked),
       };
     },
     computed: {
@@ -147,6 +150,9 @@ export default function initDiffsApp(store) {
           fileByFileUserPreference: this.viewDiffsFileByFile,
           defaultSuggestionCommitMessage: this.defaultSuggestionCommitMessage,
           rehydratedMrReviews: getReviewsForMergeRequest(mrPath),
+          sourceProjectDefaultUrl: this.sourceProjectDefaultUrl,
+          sourceProjectFullPath: this.sourceProjectFullPath,
+          isForked: this.isForked,
         },
       });
     },

app/helpers/merge_requests_helper.rb

@@ -186,7 +186,10 @@ module MergeRequestsHelper
       show_suggest_popover: show_suggest_popover?.to_s,
       show_whitespace_default: @show_whitespace_default.to_s,
       file_by_file_default: @file_by_file_default.to_s,
-      default_suggestion_commit_message: default_suggestion_commit_message
+      default_suggestion_commit_message: default_suggestion_commit_message,
+      source_project_default_url: @merge_request.source_project && default_url_to_repo(@merge_request.source_project),
+      source_project_full_path: @merge_request.source_project&.full_path,
+      is_forked: @project.forked?.to_s
     }
   end
 

locale/gitlab.pot

@@ -4514,6 +4514,9 @@ msgstr ""
 msgid "Ask again later"
 msgstr ""
 
+msgid "Ask someone with write access to resolve it."
+msgstr ""
+
 msgid "Ask your group maintainer to set up a group runner."
 msgstr ""
 
@@ -8503,6 +8506,27 @@ msgstr ""
 msgid "Confirmed:"
 msgstr ""
 
+msgid "Conflict: This file was added both in the source and target branches, but with different contents."
+msgstr ""
+
+msgid "Conflict: This file was modified in both the source and target branches."
+msgstr ""
+
+msgid "Conflict: This file was modified in the source branch, but removed in the target branch."
+msgstr ""
+
+msgid "Conflict: This file was removed in the source branch, but modified in the target branch."
+msgstr ""
+
+msgid "Conflict: This file was removed in the source branch, but renamed in the target branch."
+msgstr ""
+
+msgid "Conflict: This file was renamed differently in the source and target branches."
+msgstr ""
+
+msgid "Conflict: This file was renamed in the source branch, but removed in the target branch."
+msgstr ""
+
 msgid "Confluence"
 msgstr ""
 
@@ -37762,6 +37786,12 @@ msgstr ""
 msgid "You are using PostgreSQL %{pg_version_current}, but PostgreSQL %{pg_version_minimum} is required for this version of GitLab. Please upgrade your environment to a supported PostgreSQL version, see %{pg_requirements_url} for details."
 msgstr ""
 
+msgid "You can %{gitlabLinkStart}resolve conflicts on GitLab%{gitlabLinkEnd} or %{resolveLocallyStart}resolve it locally%{resolveLocallyEnd}."
+msgstr ""
+
+msgid "You can %{resolveLocallyStart}resolve it locally%{resolveLocallyEnd}."
+msgstr ""
+
 msgid "You can also create a project from the command line."
 msgstr ""
 

spec/frontend/diffs/components/app_spec.js

@@ -17,7 +17,6 @@ import TreeList from '~/diffs/components/tree_list.vue';
 /* You know what: sometimes alphabetical isn't the best order */
 import CollapsedFilesWarning from '~/diffs/components/collapsed_files_warning.vue';
 import HiddenFilesWarning from '~/diffs/components/hidden_files_warning.vue';
-import MergeConflictWarning from '~/diffs/components/merge_conflict_warning.vue';
 /* eslint-enable import/order */
 
 import axios from '~/lib/utils/axios_utils';
@@ -542,43 +541,6 @@ describe('diffs/components/app', () => {
           expect(getCollapsedFilesWarning(wrapper).exists()).toBe(false);
         });
       });
-
-      describe('merge conflicts', () => {
-        it('should render the merge conflicts banner if viewing the whole changeset and there are conflicts', () => {
-          createComponent({}, ({ state }) => {
-            Object.assign(state.diffs, {
-              latestDiff: true,
-              startVersion: null,
-              hasConflicts: true,
-              canMerge: false,
-              conflictResolutionPath: 'path',
-            });
-          });
-
-          expect(wrapper.find(MergeConflictWarning).exists()).toBe(true);
-        });
-
-        it.each`
-          prop              | value
-          ${'latestDiff'}   | ${false}
-          ${'startVersion'} | ${'notnull'}
-          ${'hasConflicts'} | ${false}
-        `(
-          "should not render if any of the MR properties aren't correct - like $prop: $value",
-          ({ prop, value }) => {
-            createComponent({}, ({ state }) => {
-              Object.assign(state.diffs, {
-                latestDiff: true,
-                startVersion: null,
-                hasConflicts: true,
-                [prop]: value,
-              });
-            });
-
-            expect(wrapper.find(MergeConflictWarning).exists()).toBe(false);
-          },
-        );
-      });
     });
 
     it('should display commit widget if store has a commit', () => {

spec/frontend/diffs/components/diff_file_spec.js

@@ -541,4 +541,34 @@ describe('DiffFile', () => {
 
     expect(findLoader(wrapper).exists()).toBe(true);
   });
+
+  describe('merge conflicts', () => {
+    beforeEach(() => {
+      wrapper.destroy();
+    });
+
+    it('does not render conflict alert', () => {
+      const file = {
+        ...getReadableFile(),
+        conflict_type: null,
+        renderIt: true,
+      };
+
+      ({ wrapper, store } = createComponent({ file }));
+
+      expect(wrapper.find('[data-testid="conflictsAlert"]').exists()).toBe(false);
+    });
+
+    it('renders conflict alert when conflict_type is present', () => {
+      const file = {
+        ...getReadableFile(),
+        conflict_type: 'both_modified',
+        renderIt: true,
+      };
+
+      ({ wrapper, store } = createComponent({ file }));
+
+      expect(wrapper.find('[data-testid="conflictsAlert"]').exists()).toBe(true);
+    });
+  });
 });