Skip to content

G
gitlab-ce
Commit 121597d1 authored by Phil Hughes's avatar Phil Hughes
Browse files
Options

Move merge request widget messages 

Moves the merge request alert messages to be above the rest
of widgets content.
This also makes each of the messages consistent in design.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/324174/
Closes https://gitlab.com/gitlab-org/gitlab/-/issues/324171/
Closes https://gitlab.com/gitlab-org/gitlab/-/issues/324176/
parent fa5a9902
Show whitespace changes
Inline Side-by-side
Showing with 119 additions and 146 deletions
app/assets/javascripts/vue_merge_request_widget/components/mr_widget_alert_message.vue
View file @ 121597d1
<script>
import { GlLink, GlIcon } from '@gitlab/ui';
import { WARNING, DANGER, WARNING_MESSAGE_CLASS, DANGER_MESSAGE_CLASS } from '../constants';
import { GlLink, GlAlert } from '@gitlab/ui';
export default {
name: 'MrWidgetAlertMessage',
name: 'MRWidgetAlertMessage',
components: {
GlAlert,
GlLink,
GlIcon,
},
props: {
type: {
type: String,
required: false,
default: DANGER,
validator: (value) => [WARNING, DANGER].includes(value),
required: true,
},
helpPath: {
type: String,
required: false,
default: undefined,
},
dismissible: {
type: Boolean,
required: false,
default: false,
},
computed: {
messageClass() {
if (this.type === WARNING) {
return WARNING_MESSAGE_CLASS;
} else if (this.type === DANGER) {
return DANGER_MESSAGE_CLASS;
}
return '';
},
data() {
return {
isDismissed: false,
};
},
methods: {
onDismiss() {
this.isDismissed = true;
},
},
};
</script>
<template>
<div class="gl-m-3 gl-ml-7" :class="messageClass">
<gl-alert v-if="!isDismissed" :variant="type" :dismissible="dismissible" @dismiss="onDismiss">
<slot></slot>
<gl-link v-if="helpPath" :href="helpPath" target="_blank">
<gl-icon :size="16" name="question-o" class="align-middle" />
<gl-link v-if="helpPath" :href="helpPath" target="_blank" class="gl-label-link">
<slot name="link-content"></slot>
</gl-link>
</div>
</gl-alert>
</template>
app/assets/javascripts/vue_merge_request_widget/constants.js
View file @ 121597d1
......@@ -5,9 +5,6 @@ export const WARNING = 'warning';
export const DANGER = 'danger';
export const INFO = 'info';
export const WARNING_MESSAGE_CLASS = 'warning_message';
export const DANGER_MESSAGE_CLASS = 'danger_message';
export const MWPS_MERGE_STRATEGY = 'merge_when_pipeline_succeeds';
export const MTWPS_MERGE_STRATEGY = 'add_to_merge_train_when_pipeline_succeeds';
export const MT_MERGE_STRATEGY = 'merge_train';
......
app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
View file @ 121597d1
......@@ -198,6 +198,9 @@ export default {
formattedHumanAccess() {
return (this.mr.humanAccess || '').toLowerCase();
},
hasAlerts() {
return this.mr.mergeError || this.showMergePipelineForkWarning;
},
},
watch: {
state(newVal, oldVal) {
......@@ -432,7 +435,14 @@ export default {
</script>
<template>
<div v-if="isLoaded" class="mr-state-widget gl-mt-3">
<header
class="gl-overflow-hidden gl-rounded-base gl-border-solid gl-border-1 gl-border-gray-100"
>
<mr-widget-alert-message v-if="shouldRenderCollaborationStatus" type="info">
{{ s__('mrWidget|Members who can merge are allowed to add commits.') }}
</mr-widget-alert-message>
<mr-widget-header :mr="mr" />
</header>
<mr-widget-suggest-pipeline
v-if="shouldSuggestPipelines"
data-testid="mr-suggest-pipeline"
......@@ -456,6 +466,25 @@ export default {
:service="service"
/>
<div class="mr-section-container mr-widget-workflow">
<div v-if="hasAlerts" class="gl-overflow-hidden mr-widget-alert-container">
<mr-widget-alert-message v-if="mr.mergeError" type="danger" dismissible>
<span v-safe-html="mergeError"></span>
</mr-widget-alert-message>
<mr-widget-alert-message
v-if="showMergePipelineForkWarning"
type="warning"
:help-path="mr.mergeRequestPipelinesHelpPath"
>
{{
s__(
'mrWidget|If the last pipeline ran in the fork project, it may be inaccurate. Before merge, we advise running a pipeline in this project.',
)
}}
<template #link-content>
{{ __('Learn more') }}
</template>
</mr-widget-alert-message>
</div>
<!-- <extensions-container :mr="mr" /> -->
<grouped-codequality-reports-app
v-if="shouldRenderCodeQuality"
......@@ -492,34 +521,12 @@ export default {
<component :is="componentName" :mr="mr" :service="service" />
<div class="mr-widget-info">
<section v-if="shouldRenderCollaborationStatus" class="mr-info-list mr-links">
<p>
{{ s__('mrWidget|Allows commits from members who can merge to the target branch') }}
</p>
</section>
<mr-widget-related-links
v-if="shouldRenderRelatedLinks"
:state="mr.state"
:related-links="mr.relatedLinks"
/>
<mr-widget-alert-message
v-if="showMergePipelineForkWarning"
type="warning"
:help-path="mr.mergeRequestPipelinesHelpPath"
>
{{
s__(
'mrWidget|If the last pipeline ran in the fork project, it may be inaccurate. Before merge, we advise running a pipeline in this project.',
)
}}
</mr-widget-alert-message>
<mr-widget-alert-message v-if="mr.mergeError" type="danger">
<span v-safe-html="mergeError"></span>
</mr-widget-alert-message>
<source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" />
</div>
</div>
......
app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
View file @ 121597d1
......@@ -44,7 +44,6 @@ export default class MergeRequestStore {
this.sourceBranch = data.source_branch;
this.sourceBranchProtected = data.source_branch_protected;
this.conflictsDocsPath = data.conflicts_docs_path;
this.mergeRequestPipelinesHelpPath = data.merge_request_pipelines_docs_path;
this.mergeTrainWhenPipelineSucceedsDocsPath = data.merge_train_when_pipeline_succeeds_docs_path;
this.commitMessage = data.default_merge_commit_message;
this.shortMergeCommitSha = data.short_merged_commit_sha;
......
app/assets/stylesheets/pages/merge_requests.scss
View file @ 121597d1
......@@ -43,8 +43,6 @@ $tabs-holder-z-index: 250;
}
.mr-widget-section {
border-radius: $border-radius-default $border-radius-default 0 0;
.code-text {
flex: 1;
}
......@@ -88,7 +86,6 @@ $tabs-holder-z-index: 250;
.mr-section-container {
border: 1px solid $border-color;
border-radius: $border-radius-default;
border-top: 0;
background: var(--white, $white);
}
......@@ -110,7 +107,6 @@ $tabs-holder-z-index: 250;
border-radius: $border-radius-default;
}
.mr-widget-section,
.mr-widget-footer {
border-top: solid 1px $border-color;
}
......@@ -525,9 +521,7 @@ $tabs-holder-z-index: 250;
.mr-source-target {
flex-wrap: wrap;
border-radius: $border-radius-default;
padding: $gl-padding;
border: 1px solid $border-color;
background: var(--white, $white);
min-height: $mr-widget-min-height;
......@@ -1027,3 +1021,13 @@ $tabs-holder-z-index: 250;
vertical-align: middle;
}
}
.mr-widget-alert-container {
$radius: $border-radius-default - 1px;
border-radius: $radius $radius 0 0;
.gl-alert:not(:last-child) {
margin-bottom: 1px;
}
}
ee/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.vue
View file @ 121597d1
......@@ -252,7 +252,14 @@ export default {
</script>
<template>
<div v-if="isLoaded" class="mr-state-widget gl-mt-3">
<header
class="gl-overflow-hidden gl-rounded-base gl-border-solid gl-border-1 gl-border-gray-100"
>
<mr-widget-alert-message v-if="shouldRenderCollaborationStatus" type="info">
{{ s__('mrWidget|Members who can merge are allowed to add commits.') }}
</mr-widget-alert-message>
<mr-widget-header :mr="mr" />
</header>
<mr-widget-suggest-pipeline
v-if="shouldSuggestPipelines"
class="mr-widget-workflow"
......@@ -275,6 +282,25 @@ export default {
:service="service"
/>
<div class="mr-section-container mr-widget-workflow">
<div v-if="hasAlerts" class="gl-overflow-hidden mr-widget-alert-container">
<mr-widget-alert-message v-if="mr.mergeError" type="danger" dismissible>
<span v-safe-html="mergeError"></span>
</mr-widget-alert-message>
<mr-widget-alert-message
v-if="showMergePipelineForkWarning"
type="warning"
:help-path="mr.mergeRequestPipelinesHelpPath"
>
{{
s__(
'mrWidget|If the last pipeline ran in the fork project, it may be inaccurate. Before merge, we advise running a pipeline in this project.',
)
}}
<template #link-content>
{{ __('Learn more') }}
</template>
</mr-widget-alert-message>
</div>
<blocking-merge-requests-report :mr="mr" />
<grouped-codequality-reports-app
v-if="shouldRenderCodeQuality"
......@@ -393,34 +419,12 @@ export default {
<div class="mr-widget-section">
<component :is="componentName" :mr="mr" :service="service" />
<div class="mr-widget-info">
<section v-if="mr.allowCollaboration" class="mr-info-list gl-ml-7">
<p>
{{ s__('mrWidget|Allows commits from members who can merge to the target branch') }}
</p>
</section>
<mr-widget-related-links
v-if="shouldRenderRelatedLinks"
:state="mr.state"
:related-links="mr.relatedLinks"
/>
<mr-widget-alert-message
v-if="showMergePipelineForkWarning"
type="warning"
:help-path="mr.mergeRequestPipelinesHelpPath"
>
{{
s__(
'mrWidget|Fork project merge requests do not create merge request pipelines that validate a post merge result unless invoked by a project member.',
)
}}
</mr-widget-alert-message>
<mr-widget-alert-message v-if="mr.mergeError" type="danger">
<span v-safe-html="mergeError"></span>
</mr-widget-alert-message>
<source-branch-removal-status v-if="shouldRenderSourceBranchRemovalStatus" />
</div>
</div>
......
ee/spec/features/merge_request/user_sees_merge_widget_spec.rb
View file @ 121597d1
......@@ -73,12 +73,7 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
it 'shows a warning that fork project merge request does not create merge request pipelines by default', :sidekiq_might_not_need_inline do
visit project_merge_request_path(project, merge_request)
within('.warning_message') do
expect(page)
.to have_content('Fork project merge requests do not create merge' \
' request pipelines that validate a post merge result' \
' unless invoked by a project member.')
end
expect(page).to have_content('If the last pipeline ran in the fork project, it may be inaccurate.')
end
end
end
......
locale/gitlab.pot
View file @ 121597d1
......@@ -38626,9 +38626,6 @@ msgstr ""
msgid "mrWidget|Added to the merge train. There are %{mergeTrainPosition} merge requests waiting to be merged"
msgstr ""
msgid "mrWidget|Allows commits from members who can merge to the target branch"
msgstr ""
msgid "mrWidget|An error occurred while removing your approval."
msgstr ""
......@@ -38698,9 +38695,6 @@ msgstr ""
msgid "mrWidget|Fast-forward merge is not possible. To merge this request, first rebase locally."
msgstr ""
msgid "mrWidget|Fork project merge requests do not create merge request pipelines that validate a post merge result unless invoked by a project member."
msgstr ""
msgid "mrWidget|If the %{missingBranchName} branch exists in your local repository, you can merge this merge request manually using the command line"
msgstr ""
......@@ -38716,6 +38710,9 @@ msgstr ""
msgid "mrWidget|Mark as ready"
msgstr ""
msgid "mrWidget|Members who can merge are allowed to add commits."
msgstr ""
msgid "mrWidget|Mentions"
msgstr ""
......
spec/features/merge_request/user_allows_commits_from_memebers_who_can_merge_spec.rb
View file @ 121597d1
......@@ -32,7 +32,7 @@ RSpec.describe 'create a merge request, allowing commits from members who can me
wait_for_requests
expect(page).to have_content('Allows commits from members who can merge to the target branch')
expect(page).to have_content('Members who can merge are allowed to add commits.')
end
it 'shows a message when one of the projects is private', :sidekiq_might_not_need_inline do
......@@ -59,7 +59,7 @@ RSpec.describe 'create a merge request, allowing commits from members who can me
visit_new_merge_request
expect(page).not_to have_content('Allows commits from members who can merge to the target branch')
expect(page).not_to have_content('The fork project allows commits from members who can write to the target branch.')
end
end
......@@ -81,7 +81,7 @@ RSpec.describe 'create a merge request, allowing commits from members who can me
it 'hides the option from members' do
visit edit_project_merge_request_path(target_project, merge_request)
expect(page).not_to have_content('Allows commits from members who can merge to the target branch')
expect(page).not_to have_content('The fork project allows commits from members who can write to the target branch.')
end
end
end
spec/frontend/vue_mr_widget/components/mr_widget_alert_message_spec.js
View file @ 121597d1
import { GlLink } from '@gitlab/ui';
import { shallowMount, createLocalVue } from '@vue/test-utils';
import { GlLink, GlAlert } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils';
import MrWidgetAlertMessage from '~/vue_merge_request_widget/components/mr_widget_alert_message.vue';
describe('MrWidgetAlertMessage', () => {
let wrapper;
beforeEach(() => {
const localVue = createLocalVue();
let wrapper;
wrapper = shallowMount(localVue.extend(MrWidgetAlertMessage), {
propsData: {},
localVue,
});
function createComponent(propsData = {}) {
wrapper = shallowMount(MrWidgetAlertMessage, {
propsData,
});
}
describe('MrWidgetAlertMessage', () => {
afterEach(() => {
wrapper.destroy();
});
describe('when type is not provided', () => {
it('should render a red message', (done) => {
wrapper.vm.$nextTick(() => {
expect(wrapper.classes()).toContain('danger_message');
expect(wrapper.classes()).not.toContain('warning_message');
done();
});
});
});
it('should render a GlAert', () => {
createComponent({ type: 'danger' });
describe('when type === "danger"', () => {
it('should render a red message', (done) => {
wrapper.setProps({ type: 'danger' });
wrapper.vm.$nextTick(() => {
expect(wrapper.classes()).toContain('danger_message');
expect(wrapper.classes()).not.toContain('warning_message');
done();
});
});
});
describe('when type === "warning"', () => {
it('should render a red message', (done) => {
wrapper.setProps({ type: 'warning' });
wrapper.vm.$nextTick(() => {
expect(wrapper.classes()).toContain('warning_message');
expect(wrapper.classes()).not.toContain('danger_message');
done();
});
});
expect(wrapper.findComponent(GlAlert).exists()).toBe(true);
expect(wrapper.findComponent(GlAlert).props('variant')).toBe('danger');
});
describe('when helpPath is not provided', () => {
it('should not render a help icon/link', (done) => {
wrapper.vm.$nextTick(() => {
const link = wrapper.find(GlLink);
it('should not render a help link', () => {
createComponent({ type: 'info' });
const link = wrapper.findComponent(GlLink);
expect(link.exists()).toBe(false);
done();
});
});
});
describe('when helpPath is provided', () => {
it('should render a help icon/link', (done) => {
wrapper.setProps({ helpPath: '/path/to/help/docs' });
wrapper.vm.$nextTick(() => {
const link = wrapper.find(GlLink);
it('should render a help link', () => {
createComponent({ type: 'info', helpPath: 'https://gitlab.com' });
const link = wrapper.findComponent(GlLink);
expect(link.exists()).toBe(true);
expect(link.attributes().href).toBe('/path/to/help/docs');
done();
});
expect(link.attributes('href')).toBe('https://gitlab.com');
});
});
});
spec/frontend/vue_mr_widget/mr_widget_options_spec.js
View file @ 121597d1
......@@ -26,7 +26,7 @@ describe('MrWidgetOptions', () => {
let wrapper;
let mock;
const COLLABORATION_MESSAGE = 'Allows commits from members who can merge to the target branch';
const COLLABORATION_MESSAGE = 'Members who can merge are allowed to add commits';
beforeEach(() => {
gl.mrWidgetData = { ...mockData };
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7