Commit 03a8f65e authored by GitLab Bot's avatar GitLab Bot

Automatic merge of gitlab-org/gitlab master

parents 5fe2ffc8 c44be2dc
<script>
import { GlIcon, GlTooltipDirective } from '@gitlab/ui';
import { GlIcon, GlToggle, GlTooltipDirective } from '@gitlab/ui';
import { __ } from '~/locale';
import Tracking from '~/tracking';
import toggleButton from '~/vue_shared/components/toggle_button.vue';
import eventHub from '../../event_hub';
const ICON_ON = 'notifications';
......@@ -16,7 +15,7 @@ export default {
},
components: {
GlIcon,
toggleButton,
GlToggle,
},
mixins: [Tracking.mixin({ label: 'right_sidebar' })],
props: {
......@@ -106,7 +105,7 @@ export default {
</script>
<template>
<div>
<div class="gl-display-flex gl-justify-content-space-between">
<span
ref="tooltip"
v-gl-tooltip.viewport.left
......@@ -116,13 +115,13 @@ export default {
>
<gl-icon :name="notificationIcon" :size="16" class="sidebar-item-icon is-active" />
</span>
<span class="issuable-header-text hide-collapsed float-left"> {{ notificationText }} </span>
<toggle-button
<span class="hide-collapsed" data-testid="subscription-title"> {{ notificationText }} </span>
<gl-toggle
v-if="!projectEmailsDisabled"
ref="toggleButton"
:is-loading="showLoadingState"
:value="subscribed"
class="float-right hide-collapsed js-issuable-subscribe-button"
class="hide-collapsed"
data-testid="subscription-toggle"
@change="toggleSubscription"
/>
</div>
......
......@@ -190,11 +190,7 @@ class IssuableBaseService < BaseService
change_additional_attributes(issuable)
old_associations = associations_before_update(issuable)
label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids)
if labels_changing?(issuable.label_ids, label_ids)
params[:label_ids] = label_ids
issuable.touch
end
assign_requested_labels(issuable)
if issuable.changed? || params.present?
issuable.assign_attributes(params)
......@@ -297,10 +293,6 @@ class IssuableBaseService < BaseService
update_task(issuable)
end
def labels_changing?(old_label_ids, new_label_ids)
old_label_ids.sort != new_label_ids.sort
end
def has_title_or_description_changed?(issuable)
issuable.title_changed? || issuable.description_changed?
end
......@@ -349,6 +341,20 @@ class IssuableBaseService < BaseService
end
# rubocop: enable CodeReuse/ActiveRecord
def assign_requested_labels(issuable)
label_ids = process_label_ids(params, existing_label_ids: issuable.label_ids)
return unless ids_changing?(issuable.label_ids, label_ids)
params[:label_ids] = label_ids
issuable.touch
end
# Arrays of ids are used, but we should really use sets of ids, so
# let's have an helper to properly check if some ids are changing
def ids_changing?(old_array, new_array)
old_array.sort != new_array.sort
end
def toggle_award(issuable)
award = params.delete(:emoji_award)
AwardEmojis::ToggleService.new(issuable, award, current_user).execute if award
......
......@@ -16,30 +16,17 @@ module MergeRequests
merge_request.source_project = find_source_project
merge_request.target_project = find_target_project
# Source project sets the default source branch removal setting
merge_request.merge_params['force_remove_source_branch'] =
if params.key?(:force_remove_source_branch)
params.delete(:force_remove_source_branch)
else
merge_request.source_project.remove_source_branch_after_merge?
end
# Force remove the source branch?
merge_request.merge_params['force_remove_source_branch'] = force_remove_source_branch
# Only assign merge requests params that are allowed
self.params = assign_allowed_merge_params(merge_request, params)
# Filter out params that are either not allowed or invalid
filter_params(merge_request)
# merge_request.assign_attributes(...) below is a Rails
# method that only work if all the params it is passed have
# corresponding fields in the database. As there are no fields
# in the database for :add_label_ids and :remove_label_ids, we
# need to remove them from the params before the call to
# merge_request.assign_attributes(...)
#
# IssuableBaseService#process_label_ids takes care
# of the removal.
params[:label_ids] = process_label_ids(params, extra_label_ids: merge_request.label_ids.to_a)
merge_request.assign_attributes(params.to_h.compact)
# Filter out :add_label_ids and :remove_label_ids params
filter_label_id_params
merge_request.compare_commits = []
set_merge_request_target_branch
......@@ -74,6 +61,29 @@ module MergeRequests
:errors,
to: :merge_request
def force_remove_source_branch
if params.key?(:force_remove_source_branch)
params.delete(:force_remove_source_branch)
else
merge_request.source_project.remove_source_branch_after_merge?
end
end
def filter_label_id_params
# merge_request.assign_attributes(...) below is a Rails
# method that only work if all the params it is passed have
# corresponding fields in the database. As there are no fields
# in the database for :add_label_ids and :remove_label_ids, we
# need to remove them from the params before the call to
# merge_request.assign_attributes(...)
#
# IssuableBaseService#process_label_ids takes care
# of the removal.
params[:label_ids] = process_label_ids(params, extra_label_ids: merge_request.label_ids.to_a)
merge_request.assign_attributes(params.to_h.compact)
end
def find_source_project
source_project = project_from_params(:source_project)
return source_project if source_project.present? && can?(current_user, :create_merge_request_from, source_project)
......
---
title: Code extraction - refactoring of MR services classes
merge_request: 49827
author:
type: changed
---
title: Migrate toggle button in subscription to GitLab UI component
merge_request: 52717
author:
type: changed
---
redirect_to: '../user/group/index.md#user-contribution-analysis'
---
This document was moved to [another location](../user/group/index.md#user-contribution-analysis)
<!-- This redirect file can be deleted after February 1, 2021. -->
<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page -->
---
redirect_to: '../pipelines/job_artifacts.md'
---
This document was moved to [pipelines/job_artifacts.md](../pipelines/job_artifacts.md).
<!-- This redirect file can be deleted after February 1, 2021. -->
<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page -->
---
redirect_to: '../../user/permissions.md#gitlab-cicd-permissions'
---
This document was moved to [user/permissions.md](../../user/permissions.md#gitlab-cicd-permissions).
<!-- This redirect file can be deleted after February 1, 2021. -->
<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page -->
---
redirect_to: '../user/admin_area/license.md'
---
This document was moved to [another location](../user/admin_area/license.md).
<!-- This redirect file can be deleted after February 1, 2021. -->
<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page -->
---
redirect_to: 'https://docs.gitlab.com'
---
Visit our [documentation page](https://docs.gitlab.com) for information about GitLab.
---
redirect_to: '../../../install/aws/index.md'
---
This document was moved to [another location](../../../install/aws/index.md).
<!-- This redirect file can be deleted after February 1, 2021. -->
<!-- Before deletion, see: https://docs.gitlab.com/ee/development/documentation/#move-or-rename-a-page -->
---
redirect_to: 'https://docs.gitlab.com'
---
Visit our [documentation page](https://docs.gitlab.com) for information about GitLab.
---
redirect_to: 'https://docs.gitlab.com'
---
Visit our [documentation page](https://docs.gitlab.com) for information about GitLab.
......@@ -411,7 +411,7 @@ plugins:
enabled: true
```
This adds SonarJava to the `plugins:` section of the [default `.codeclimate.yml`](https://gitlab.com/gitlab-org/ci-cd/codequality/-/blob/master/codeclimate_defaults/.codeclimate.yml)
This adds SonarJava to the `plugins:` section of the [default `.codeclimate.yml`](https://gitlab.com/gitlab-org/ci-cd/codequality/-/blob/master/codeclimate_defaults/.codeclimate.yml.template)
included in your project.
Changes to the `plugins:` section do not affect the `exclude_patterns` section of the
......@@ -428,7 +428,7 @@ Here's [an example project](https://gitlab.com/jheimbuck_gl/jh_java_example_proj
A common issue is that the terms `Code Quality` (GitLab specific) and `Code Climate`
(Engine used by GitLab) are very similar. You must add a **`.codeclimate.yml`** file
to change the default configuration, **not** a `.codequality.yml` file. If you use
the wrong filename, the [default `.codeclimate.yml`](https://gitlab.com/gitlab-org/ci-cd/codequality/-/blob/master/codeclimate_defaults/.codeclimate.yml)
the wrong filename, the [default `.codeclimate.yml`](https://gitlab.com/gitlab-org/ci-cd/codequality/-/blob/master/codeclimate_defaults/.codeclimate.yml.template)
is still used.
### No Code Quality report is displayed in a Merge Request
......@@ -444,15 +444,15 @@ This can be due to multiple reasons:
- The [`artifacts:expire_in`](../../../ci/yaml/README.md#artifactsexpire_in) CI/CD
setting can cause the Code Quality artifact(s) to expire faster than desired.
- If you use the [`REPORT_STDOUT` environment variable](https://gitlab.com/gitlab-org/ci-cd/codequality#environment-variables), no report file is generated and nothing displays in the merge request.
- Large `codeclimate.json` files (esp. >10 MB) are [known to prevent the report from being displayed](https://gitlab.com/gitlab-org/gitlab/-/issues/2737).
- Large `gl-code-quality-report.json` files (esp. >10 MB) are [known to prevent the report from being displayed](https://gitlab.com/gitlab-org/gitlab/-/issues/2737).
As a work-around, try removing [properties](https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#data-types)
that are [ignored by GitLab](#implementing-a-custom-tool). You can:
- Configure the Code Quality tool to not output those types.
- Use `sed`, `awk` or similar commands in the `.gitlab-ci.yml` script to
edit the `codeclimate.json` before the job completes.
edit the `gl-code-quality-report.json` before the job completes.
### Only a single Code Quality report is displayed, but more are defined
GitLab only uses the Code Quality artifact from the latest created job (with the largest job ID).
If multiple jobs in a pipeline generate a code quality artifact, those of earlier jobs are ignored.
To avoid confusion, configure only one job to generate a `codeclimate.json`.
To avoid confusion, configure only one job to generate a `gl-code-quality-report.json`.
import Vue from 'vue';
import { GlToggle } from '@gitlab/ui';
import { mount } from '@vue/test-utils';
import SidebarSubscription from 'ee/epic/components/sidebar_items/sidebar_subscription.vue';
import createStore from 'ee/epic/store';
import { mountComponentWithStore } from 'helpers/vue_mount_component_helper';
import { mockEpicMeta, mockEpicData } from '../../mock_data';
import { extendedWrapper } from 'helpers/vue_test_utils_helper';
describe('SidebarSubscriptionComponent', () => {
let vm;
let store;
let wrapper;
beforeEach(() => {
const Component = Vue.extend(SidebarSubscription);
store = createStore();
store.dispatch('setEpicMeta', mockEpicMeta);
store.dispatch('setEpicData', mockEpicData);
vm = mountComponentWithStore(Component, {
store,
props: { sidebarCollapsed: false },
});
wrapper = extendedWrapper(
mount(SidebarSubscription, {
store: createStore(),
propsData: { sidebarCollapsed: false },
}),
);
});
afterEach(() => {
vm.$destroy();
wrapper.destroy();
wrapper = null;
});
describe('template', () => {
it('renders subscription toggle element container', () => {
expect(vm.$el.classList.contains('block')).toBe(true);
expect(vm.$el.classList.contains('subscription')).toBe(true);
expect(wrapper.classes('block')).toBe(true);
expect(wrapper.classes('subscription')).toBe(true);
});
it('renders toggle title text', () => {
expect(vm.$el.querySelector('.issuable-header-text').innerText.trim()).toBe('Notifications');
expect(wrapper.findByTestId('subscription-title').text()).toBe('Notifications');
});
it('renders toggle button element', () => {
expect(vm.$el.querySelector('.js-issuable-subscribe-button button')).not.toBeNull();
expect(wrapper.findComponent(GlToggle).exists()).toBe(true);
});
});
});
......@@ -57,7 +57,7 @@ fi
# Do not use 'README.md', instead use 'index.md'
# Number of 'README.md's as of 2020-10-13
NUMBER_READMES=36
NUMBER_READMES=28
FIND_READMES=$(find doc/ -name "README.md" | wc -l)
echo '=> Checking for new README.md files...'
echo
......
......@@ -411,10 +411,10 @@ RSpec.describe 'Issue Boards', :js do
wait_for_requests
page.within('.subscriptions') do
find('.js-issuable-subscribe-button button:not(.is-checked)').click
find('[data-testid="subscription-toggle"] button:not(.is-checked)').click
wait_for_requests
expect(page).to have_css('.js-issuable-subscribe-button button.is-checked')
expect(page).to have_css('[data-testid="subscription-toggle"] button.is-checked')
end
end
......@@ -427,10 +427,10 @@ RSpec.describe 'Issue Boards', :js do
wait_for_requests
page.within('.subscriptions') do
find('.js-issuable-subscribe-button button.is-checked').click
find('[data-testid="subscription-toggle"] button.is-checked').click
wait_for_requests
expect(page).to have_css('.js-issuable-subscribe-button button:not(.is-checked)')
expect(page).to have_css('[data-testid="subscription-toggle"] button:not(.is-checked)')
end
end
end
......
......@@ -15,13 +15,13 @@ RSpec.describe "User toggles subscription", :js do
end
it "unsubscribes from issue" do
subscription_button = find(".js-issuable-subscribe-button")
subscription_button = find('[data-testid="subscription-toggle"]')
# Check we're subscribed.
expect(subscription_button).to have_css("button.is-checked")
# Toggle subscription.
find(".js-issuable-subscribe-button button").click
find('[data-testid="subscription-toggle"]').click
wait_for_requests
# Check we're unsubscribed.
......@@ -33,7 +33,7 @@ RSpec.describe "User toggles subscription", :js do
it 'is disabled' do
expect(page).to have_content('Notifications have been disabled by the project or group owner')
expect(page).not_to have_selector('.js-issuable-subscribe-button')
expect(page).not_to have_selector('[data-testid="subscription-toggle"]')
end
end
end
......@@ -15,7 +15,7 @@ RSpec.describe 'User manages subscription', :js do
end
it 'toggles subscription' do
page.within('.js-issuable-subscribe-button') do
page.within('[data-testid="subscription-toggle"]') do
wait_for_requests
expect(page).to have_css 'button:not(.is-checked)'
......
import { GlToggle } from '@gitlab/ui';
import { shallowMount } from '@vue/test-utils';
import { extendedWrapper } from 'helpers/vue_test_utils_helper';
import Subscriptions from '~/sidebar/components/subscriptions/subscriptions.vue';
import eventHub from '~/sidebar/event_hub';
import ToggleButton from '~/vue_shared/components/toggle_button.vue';
describe('Subscriptions', () => {
let wrapper;
const findToggleButton = () => wrapper.find(ToggleButton);
const findToggleButton = () => wrapper.findComponent(GlToggle);
const mountComponent = (propsData) =>
shallowMount(Subscriptions, {
propsData,
});
extendedWrapper(
shallowMount(Subscriptions, {
propsData,
}),
);
afterEach(() => {
wrapper.destroy();
......@@ -24,7 +27,7 @@ describe('Subscriptions', () => {
subscribed: undefined,
});
expect(findToggleButton().attributes('isloading')).toBe('true');
expect(findToggleButton().props('isLoading')).toBe(true);
});
it('is toggled "off" when currently not subscribed', () => {
......@@ -32,7 +35,7 @@ describe('Subscriptions', () => {
subscribed: false,
});
expect(findToggleButton().attributes('value')).toBeFalsy();
expect(findToggleButton().props('value')).toBe(false);
});
it('is toggled "on" when currently subscribed', () => {
......@@ -40,7 +43,7 @@ describe('Subscriptions', () => {
subscribed: true,
});
expect(findToggleButton().attributes('value')).toBe('true');
expect(findToggleButton().props('value')).toBe(true);
});
it('toggleSubscription method emits `toggleSubscription` event on eventHub and Component', () => {
......@@ -93,14 +96,16 @@ describe('Subscriptions', () => {
});
it('sets the correct display text', () => {
expect(wrapper.find('.issuable-header-text').text()).toContain(subscribeDisabledDescription);
expect(wrapper.findByTestId('subscription-title').text()).toContain(
subscribeDisabledDescription,
);
expect(wrapper.find({ ref: 'tooltip' }).attributes('title')).toBe(
subscribeDisabledDescription,
);
});
it('does not render the toggle button', () => {
expect(wrapper.find('.js-issuable-subscribe-button').exists()).toBe(false);
expect(findToggleButton().exists()).toBe(false);
});
});
});
......@@ -21,6 +21,7 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" }
let(:deleted_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 #{Gitlab::Git::BLANK_SHA} refs/heads/#{source_branch}" }
let(:default_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{project.default_branch}" }
let(:error_mr_required) { "A merge_request.create push option is required to create a merge request for branch #{source_branch}" }
shared_examples_for 'a service that can create a merge request' do
subject(:last_mr) { MergeRequest.last }
......@@ -176,11 +177,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it 'adds an error to the service' do
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
service.execute
expect(service.errors).to include(error)
expect(service.errors).to include(error_mr_required)
end
context 'when coupled with the `create` push option' do
......@@ -197,11 +196,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it 'adds an error to the service' do
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
service.execute
expect(service.errors).to include(error)
expect(service.errors).to include(error_mr_required)
end
context 'when coupled with the `create` push option' do
......@@ -263,11 +260,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it 'adds an error to the service' do
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
service.execute
expect(service.errors).to include(error)
expect(service.errors).to include(error_mr_required)
end
context 'when coupled with the `create` push option' do
......@@ -308,11 +303,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it 'adds an error to the service' do
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
service.execute
expect(service.errors).to include(error)
expect(service.errors).to include(error_mr_required)
end
context 'when coupled with the `create` push option' do
......@@ -329,11 +322,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it 'adds an error to the service' do
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
service.execute
expect(service.errors).to include(error)
expect(service.errors).to include(error_mr_required)
end
context 'when coupled with the `create` push option' do
......@@ -374,11 +365,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it 'adds an error to the service' do
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
service.execute
expect(service.errors).to include(error)
expect(service.errors).to include(error_mr_required)
end
context 'when coupled with the `create` push option' do
......@@ -395,11 +384,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it 'adds an error to the service' do
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
service.execute
expect(service.errors).to include(error)
expect(service.errors).to include(error_mr_required)
end
context 'when coupled with the `create` push option' do
......@@ -440,11 +427,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it 'adds an error to the service' do
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
service.execute
expect(service.errors).to include(error)
expect(service.errors).to include(error_mr_required)
end
context 'when coupled with the `create` push option' do
......@@ -461,11 +446,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it 'adds an error to the service' do
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
service.execute
expect(service.errors).to include(error)
expect(service.errors).to include(error_mr_required)
end
context 'when coupled with the `create` push option' do
......@@ -506,11 +489,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it 'adds an error to the service' do
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
service.execute
expect(service.errors).to include(error)
expect(service.errors).to include(error_mr_required)
end
context 'when coupled with the `create` push option' do
......@@ -527,11 +508,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do
it_behaves_like 'a service that does not create a merge request'
it 'adds an error to the service' do
error = "A merge_request.create push option is required to create a merge request for branch #{source_branch}"
service.execute
expect(service.errors).to include(error)
expect(service.errors).to include(error_mr_required)
end
context 'when coupled with the `create` push option' do
......
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