Commit 30a6f52a authored by Phil Hughes's avatar Phil Hughes

Added warning to diff files when there are conflicts

This adds an alert component to each diff file that has conflicts.

https://gitlab.com/gitlab-org/gitlab/-/issues/281171
parent 65b4b26f
......@@ -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';
......@@ -51,7 +53,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';
......@@ -64,7 +65,6 @@ export default {
DiffFile,
NoChanges,
HiddenFilesWarning,
MergeConflictWarning,
CollapsedFilesWarning,
CommitWidget,
TreeList,
......@@ -76,6 +76,7 @@ export default {
DynamicScrollerItem,
PreRenderer,
VirtualScrollerScrollSync,
MrWidgetHowToMergeModal,
},
alerts: {
ALERT_OVERFLOW_HIDDEN,
......@@ -163,6 +164,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',
......@@ -591,6 +609,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>
......@@ -610,12 +631,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"
......@@ -728,6 +743,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>
<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"
......
......@@ -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';
......
......@@ -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.',
),
};
......@@ -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,
},
});
},
......
......@@ -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
......
......@@ -4508,6 +4508,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 ""
......@@ -37714,6 +37738,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 ""
......
......@@ -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', () => {
......
......@@ -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);
});
});
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment