Add latest changes from gitlab-org/gitlab@master

This commit is contained in:
GitLab Bot 2022-07-27 09:08:34 +00:00
parent 0ab741639e
commit d0a683e342
27 changed files with 267 additions and 228 deletions

View File

@ -133,9 +133,6 @@ export default {
this.model = null; this.model = null;
} }
}, },
helpHtmlConfig: {
ADD_ATTR: ['target'], // allow external links, can be removed after https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1427 is implemented
},
}; };
</script> </script>
@ -147,7 +144,7 @@ export default {
:state="valid" :state="valid"
> >
<template v-if="!isCheckbox" #description> <template v-if="!isCheckbox" #description>
<span v-safe-html:[$options.helpHtmlConfig]="help"></span> <span v-safe-html="help"></span>
</template> </template>
<template v-if="isCheckbox"> <template v-if="isCheckbox">
@ -155,7 +152,7 @@ export default {
<gl-form-checkbox :id="fieldId" v-model="model" :disabled="isInheriting"> <gl-form-checkbox :id="fieldId" v-model="model" :disabled="isInheriting">
{{ checkboxLabel || humanizedTitle }} {{ checkboxLabel || humanizedTitle }}
<template #help> <template #help>
<span v-safe-html:[$options.helpHtmlConfig]="help"></span> <span v-safe-html="help"></span>
</template> </template>
</gl-form-checkbox> </gl-form-checkbox>
</template> </template>

View File

@ -192,11 +192,7 @@ export default {
this.integrationActive = integrationActive; this.integrationActive = integrationActive;
}, },
}, },
descriptionHtmlConfig: {
ADD_ATTR: ['target'], // allow external links, can be removed after https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1427 is implemented
},
helpHtmlConfig: { helpHtmlConfig: {
ADD_ATTR: ['target'], // allow external links, can be removed after https://gitlab.com/gitlab-org/gitlab-ui/-/issues/1427 is implemented
ADD_TAGS: ['use'], // to support icon SVGs ADD_TAGS: ['use'], // to support icon SVGs
FORBID_ATTR: [], // This is trusted input so we can override the default config to allow data-* attributes FORBID_ATTR: [], // This is trusted input so we can override the default config to allow data-* attributes
}, },
@ -254,7 +250,7 @@ export default {
{{ $options.billingPlanNames[section.plan] }} {{ $options.billingPlanNames[section.plan] }}
</gl-badge> </gl-badge>
</h4> </h4>
<p v-safe-html:[$options.descriptionHtmlConfig]="section.description"></p> <p v-safe-html="section.description"></p>
</div> </div>
<div class="col-lg-8"> <div class="col-lg-8">

View File

@ -30,7 +30,6 @@ export default {
return dateInWords(date); return dateInWords(date);
}, },
}, },
safeHtmlConfig: { ADD_ATTR: ['target'] },
}; };
</script> </script>
@ -76,10 +75,7 @@ export default {
{{ packageName }} {{ packageName }}
</gl-badge> </gl-badge>
</div> </div>
<div <div v-safe-html="feature.body" class="gl-pt-3 gl-line-height-20"></div>
v-safe-html:[$options.safeHtmlConfig]="feature.body"
class="gl-pt-3 gl-line-height-20"
></div>
<gl-button <gl-button
:href="feature.url" :href="feature.url"
target="_blank" target="_blank"

View File

@ -43,6 +43,7 @@ export default {
}; };
}, },
update(data) { update(data) {
this.canUpdate = data.workItem.userPermissions.updateWorkItem;
return ( return (
data.workItem.widgets.find((widget) => widget.type === WIDGET_TYPE_HIERARCHY)?.children data.workItem.widgets.find((widget) => widget.type === WIDGET_TYPE_HIERARCHY)?.children
.nodes ?? [] .nodes ?? []
@ -51,6 +52,9 @@ export default {
skip() { skip() {
return !this.issuableId; return !this.issuableId;
}, },
result({ data }) {
this.canUpdate = data.workItem.userPermissions.updateWorkItem;
},
}, },
}, },
data() { data() {
@ -58,6 +62,7 @@ export default {
isShownAddForm: false, isShownAddForm: false,
isOpen: true, isOpen: true,
children: [], children: [],
canUpdate: false,
}; };
}, },
computed: { computed: {
@ -117,7 +122,7 @@ export default {
> >
<h5 class="gl-m-0 gl-line-height-32 gl-flex-grow-1">{{ $options.i18n.title }}</h5> <h5 class="gl-m-0 gl-line-height-32 gl-flex-grow-1">{{ $options.i18n.title }}</h5>
<gl-button <gl-button
v-if="!isShownAddForm" v-if="!isShownAddForm && canUpdate"
category="secondary" category="secondary"
data-testid="toggle-add-form" data-testid="toggle-add-form"
@click="toggleAddForm" @click="toggleAddForm"
@ -173,7 +178,12 @@ export default {
$options.WORK_ITEM_STATUS_TEXT[child.state] $options.WORK_ITEM_STATUS_TEXT[child.state]
}}</span> }}</span>
</gl-badge> </gl-badge>
<work-item-links-menu :work-item-id="child.id" :parent-work-item-id="issuableGid" /> <work-item-links-menu
v-if="canUpdate"
:work-item-id="child.id"
:parent-work-item-id="issuableGid"
data-testid="links-menu"
/>
</div> </div>
</div> </div>
</template> </template>

View File

@ -5,6 +5,9 @@ query workItemQuery($id: WorkItemID!) {
id id
} }
title title
userPermissions {
updateWorkItem
}
widgets { widgets {
type type
... on WorkItemWidgetHierarchy { ... on WorkItemWidgetHierarchy {

View File

@ -58,8 +58,7 @@ class Environment < ApplicationRecord
length: { maximum: 255 }, length: { maximum: 255 },
allow_nil: true allow_nil: true
validates :external_url, addressable_url: true, allow_nil: true, unless: :soft_validation_on_external_url_enabled? validate :safe_external_url
validate :safe_external_url, if: :soft_validation_on_external_url_enabled?
delegate :manual_actions, :other_manual_actions, to: :last_deployment, allow_nil: true delegate :manual_actions, :other_manual_actions, to: :last_deployment, allow_nil: true
delegate :auto_rollback_enabled?, to: :project delegate :auto_rollback_enabled?, to: :project
@ -491,10 +490,6 @@ class Environment < ApplicationRecord
private private
def soft_validation_on_external_url_enabled?
::Feature.enabled?(:soft_validation_on_external_url, project)
end
# We deliberately avoid using AddressableUrlValidator to allow users to update their environments even if they have # We deliberately avoid using AddressableUrlValidator to allow users to update their environments even if they have
# misconfigured `environment:url` keyword. The external URL is presented as a clickable link on UI and not consumed # misconfigured `environment:url` keyword. The external URL is presented as a clickable link on UI and not consumed
# in GitLab internally, thus we sanitize the URL before the persistence to make sure the rendered link is XSS safe. # in GitLab internally, thus we sanitize the URL before the persistence to make sure the rendered link is XSS safe.

View File

@ -26,8 +26,8 @@ module WorkItems
private private
def update(work_item) def before_update(work_item, skip_spam_check: false)
execute_widgets(work_item: work_item, callback: :update, widget_params: @widget_params) execute_widgets(work_item: work_item, callback: :before_update_callback, widget_params: @widget_params)
super super
end end

View File

@ -5,17 +5,18 @@ module WorkItems
class BaseService < ::BaseService class BaseService < ::BaseService
WidgetError = Class.new(StandardError) WidgetError = Class.new(StandardError)
attr_reader :widget, :current_user attr_reader :widget, :work_item, :current_user
def initialize(widget:, current_user:) def initialize(widget:, current_user:)
@widget = widget @widget = widget
@work_item = widget.work_item
@current_user = current_user @current_user = current_user
end end
private private
def can_admin_work_item? def has_permission?(permission)
can?(current_user, :admin_work_item, widget.work_item) can?(current_user, permission, widget.work_item)
end end
end end
end end

View File

@ -4,10 +4,12 @@ module WorkItems
module Widgets module Widgets
module DescriptionService module DescriptionService
class UpdateService < WorkItems::Widgets::BaseService class UpdateService < WorkItems::Widgets::BaseService
def update(params: {}) def before_update_callback(params: {})
return unless params.present? && params[:description] return unless params.present? && params.key?(:description)
return unless has_permission?(:update_work_item)
widget.work_item.description = params[:description] work_item.description = params[:description]
work_item.assign_attributes(last_edited_at: Time.current, last_edited_by: current_user)
end end
end end
end end

View File

@ -29,13 +29,13 @@ module WorkItems
def set_parent(parent) def set_parent(parent)
::WorkItems::ParentLinks::CreateService ::WorkItems::ParentLinks::CreateService
.new(parent, current_user, { target_issuable: widget.work_item }) .new(parent, current_user, { target_issuable: work_item })
.execute .execute
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def remove_parent def remove_parent
link = ::WorkItems::ParentLink.find_by(work_item: widget.work_item) link = ::WorkItems::ParentLink.find_by(work_item: work_item)
return success unless link.present? return success unless link.present?
::WorkItems::ParentLinks::DestroyService.new(link, current_user).execute ::WorkItems::ParentLinks::DestroyService.new(link, current_user).execute
@ -44,12 +44,12 @@ module WorkItems
def update_work_item_children(children) def update_work_item_children(children)
::WorkItems::ParentLinks::CreateService ::WorkItems::ParentLinks::CreateService
.new(widget.work_item, current_user, { issuable_references: children }) .new(work_item, current_user, { issuable_references: children })
.execute .execute
end end
def feature_flag_enabled? def feature_flag_enabled?
Feature.enabled?(:work_items_hierarchy, widget.work_item&.project) Feature.enabled?(:work_items_hierarchy, work_item&.project)
end end
def incompatible_args?(params) def incompatible_args?(params)

View File

@ -7,7 +7,7 @@ module WaitableWorker
# Schedules multiple jobs and waits for them to be completed. # Schedules multiple jobs and waits for them to be completed.
def bulk_perform_and_wait(args_list) def bulk_perform_and_wait(args_list)
# Short-circuit: it's more efficient to do small numbers of jobs inline # Short-circuit: it's more efficient to do small numbers of jobs inline
if args_list.size == 1 && !always_async_project_authorizations_refresh? if args_list.size == 1
return bulk_perform_inline(args_list) return bulk_perform_inline(args_list)
end end
@ -29,10 +29,6 @@ module WaitableWorker
bulk_perform_async(failed) if failed.present? bulk_perform_async(failed) if failed.present?
end end
def always_async_project_authorizations_refresh?
Feature.enabled?(:always_async_project_authorizations_refresh)
end
end end
def perform(*args) def perform(*args)

View File

@ -1,8 +0,0 @@
---
name: always_async_project_authorizations_refresh
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/92333
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/367683
milestone: '15.3'
type: development
group: group::workspace
default_enabled: false

View File

@ -1,8 +0,0 @@
---
name: soft_validation_on_external_url
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/91970
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/367206
milestone: '15.2'
type: development
group: group::release
default_enabled: false

View File

@ -373,7 +373,8 @@ To retry or rollback a deployment:
### Environment URL ### Environment URL
> [Fixed](https://gitlab.com/gitlab-org/gitlab/-/issues/337417) to persist arbitrary URLs in GitLab 15.2 [with a flag](../../administration/feature_flags.md) named `soft_validation_on_external_url`. Disabled by default. > - [Fixed](https://gitlab.com/gitlab-org/gitlab/-/issues/337417) to persist arbitrary URLs in GitLab 15.2 [with a flag](../../administration/feature_flags.md) named `soft_validation_on_external_url`. Disabled by default.
> - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/337417) in GitLab 15.3. [Feature flag `soft_validation_on_external_url`](https://gitlab.com/gitlab-org/gitlab/-/issues/367206) removed.
The [environment URL](../yaml/index.md#environmenturl) is displayed in a few The [environment URL](../yaml/index.md#environmenturl) is displayed in a few
places in GitLab: places in GitLab:

View File

@ -22,7 +22,7 @@ At the highest level, our global nav is workflow-based. Navigation needs to help
The levels under each of the higher workflow-based topics are the names of features. The levels under each of the higher workflow-based topics are the names of features.
For example: For example:
**Use GitLab** (_workflow_) **> Build your application** (_workflow_) **> CI/CD** (_feature_) **> Pipelines** (_feature) **Use GitLab** (_workflow_) **> Build your application** (_workflow_) **> CI/CD** (_feature_) **> Pipelines** (_feature_)
## Choose the right words for your navigation entry ## Choose the right words for your navigation entry
@ -39,20 +39,35 @@ as helpful as **Get started with runners**.
## Add a navigation entry ## Add a navigation entry
All topics should be included in the left nav.
To add a topic to the global nav, edit To add a topic to the global nav, edit
[`navigation.yaml`](https://gitlab.com/gitlab-org/gitlab-docs/blob/main/content/_data/navigation.yaml) [`navigation.yaml`](https://gitlab.com/gitlab-org/gitlab-docs/blob/main/content/_data/navigation.yaml)
and add your item. and add your item.
All new pages need a navigation item. Without a navigation, the page becomes "orphaned." That Without a navigation entry:
is:
- The navigation shuts when the page is opened, and the reader loses their place. - The navigation closes when the page is opened, and the reader loses their place.
- The page doesn't belong in a group with other pages. - The page isn't visible in a group with other pages.
This means the decision to create a new page is a decision to create new navigation item and vice ### Pages you don't need to add
versa.
Exclude these pages from the global nav:
- Legal notices.
- Pages in the `architecture/blueprints` directory.
The following pages should probably be in the global nav, but the technical writers
do not actively work to add them:
- Pages in the `/development` directory.
- Pages authored by the support team, which are under the `doc/administration/troubleshooting` directory.
Sometimes pages for deprecated features are not in the global nav, depending on how long ago the feature was deprecated.
All other pages should be in the global nav.
The technical writing team runs a report to determine which pages are not in the nav.
For now this report is manual, but [an issue exists](https://gitlab.com/gitlab-org/gitlab-docs/-/issues/1212)
to automate it.
### Where to add ### Where to add

View File

@ -204,7 +204,7 @@ describe('DynamicField', () => {
}); });
expect(findGlFormGroup().find('small').html()).toContain( expect(findGlFormGroup().find('small').html()).toContain(
'[<code>1</code> <a>3</a> <a target="_blank" href="foo">4</a>]', '[<code>1</code> <a>3</a> <a href="foo">4</a>]',
); );
}); });
}); });

View File

@ -6,7 +6,11 @@ import createMockApollo from 'helpers/mock_apollo_helper';
import waitForPromises from 'helpers/wait_for_promises'; import waitForPromises from 'helpers/wait_for_promises';
import WorkItemLinks from '~/work_items/components/work_item_links/work_item_links.vue'; import WorkItemLinks from '~/work_items/components/work_item_links/work_item_links.vue';
import getWorkItemLinksQuery from '~/work_items/graphql/work_item_links.query.graphql'; import getWorkItemLinksQuery from '~/work_items/graphql/work_item_links.query.graphql';
import { workItemHierarchyResponse, workItemHierarchyEmptyResponse } from '../../mock_data'; import {
workItemHierarchyResponse,
workItemHierarchyEmptyResponse,
workItemHierarchyNoUpdatePermissionResponse,
} from '../../mock_data';
Vue.use(VueApollo); Vue.use(VueApollo);
@ -29,6 +33,7 @@ describe('WorkItemLinks', () => {
const findEmptyState = () => wrapper.findByTestId('links-empty'); const findEmptyState = () => wrapper.findByTestId('links-empty');
const findToggleAddFormButton = () => wrapper.findByTestId('toggle-add-form'); const findToggleAddFormButton = () => wrapper.findByTestId('toggle-add-form');
const findAddLinksForm = () => wrapper.findByTestId('add-links-form'); const findAddLinksForm = () => wrapper.findByTestId('add-links-form');
const findFirstLinksMenu = () => wrapper.findByTestId('links-menu');
beforeEach(async () => { beforeEach(async () => {
await createComponent(); await createComponent();
@ -82,5 +87,20 @@ describe('WorkItemLinks', () => {
expect(children).toHaveLength(4); expect(children).toHaveLength(4);
expect(children.at(0).findComponent(GlBadge).text()).toBe('Open'); expect(children.at(0).findComponent(GlBadge).text()).toBe('Open');
expect(findFirstLinksMenu().exists()).toBe(true);
});
describe('when no permission to update', () => {
beforeEach(async () => {
await createComponent({ response: workItemHierarchyNoUpdatePermissionResponse });
});
it('does not display button to toggle Add form', () => {
expect(findToggleAddFormButton().exists()).toBe(false);
});
it('does not display link menu on children', () => {
expect(findFirstLinksMenu().exists()).toBe(false);
});
}); });
}); });

View File

@ -307,6 +307,9 @@ export const workItemHierarchyEmptyResponse = {
__typename: 'WorkItemType', __typename: 'WorkItemType',
}, },
title: 'New title', title: 'New title',
userPermissions: {
updateWorkItem: false,
},
widgets: [ widgets: [
{ {
type: 'DESCRIPTION', type: 'DESCRIPTION',
@ -327,6 +330,49 @@ export const workItemHierarchyEmptyResponse = {
}, },
}; };
export const workItemHierarchyNoUpdatePermissionResponse = {
data: {
workItem: {
id: 'gid://gitlab/WorkItem/1',
workItemType: {
id: 'gid://gitlab/WorkItems::Type/6',
__typename: 'WorkItemType',
},
title: 'New title',
userPermissions: {
updateWorkItem: false,
},
widgets: [
{
type: 'DESCRIPTION',
__typename: 'WorkItemWidgetDescription',
},
{
type: 'HIERARCHY',
parent: null,
children: {
nodes: [
{
id: 'gid://gitlab/WorkItem/2',
workItemType: {
id: 'gid://gitlab/WorkItems::Type/5',
__typename: 'WorkItemType',
},
title: 'xyz',
state: 'OPEN',
__typename: 'WorkItem',
},
],
__typename: 'WorkItemConnection',
},
__typename: 'WorkItemWidgetHierarchy',
},
],
__typename: 'WorkItem',
},
},
};
export const workItemHierarchyResponse = { export const workItemHierarchyResponse = {
data: { data: {
workItem: { workItem: {
@ -336,6 +382,9 @@ export const workItemHierarchyResponse = {
__typename: 'WorkItemType', __typename: 'WorkItemType',
}, },
title: 'New title', title: 'New title',
userPermissions: {
updateWorkItem: true,
},
widgets: [ widgets: [
{ {
type: 'DESCRIPTION', type: 'DESCRIPTION',

View File

@ -153,6 +153,11 @@ RSpec.describe Gitlab::GitAccess, :aggregate_failures do
end end
it 'logs' do it 'logs' do
allow(Gitlab::AppJsonLogger).to receive(:info).with(
hash_including(
"class" => "AuthorizedProjectUpdate::ProjectRecalculatePerUserWorker"
)
)
expect(Gitlab::AppJsonLogger).to receive(:info).with( expect(Gitlab::AppJsonLogger).to receive(:info).with(
message: 'Actor was :ci', message: 'Actor was :ci',
project_id: project.id project_id: project.id
@ -745,6 +750,11 @@ RSpec.describe Gitlab::GitAccess, :aggregate_failures do
it { expect { pull_access_check }.not_to raise_error } it { expect { pull_access_check }.not_to raise_error }
it 'logs' do it 'logs' do
expect(Gitlab::AppJsonLogger).to receive(:info).with(
hash_including(
"class" => "AuthorizedProjectUpdate::ProjectRecalculatePerUserWorker"
)
).once
expect(Gitlab::AppJsonLogger).to receive(:info).with( expect(Gitlab::AppJsonLogger).to receive(:info).with(
message: 'Actor was :ci', message: 'Actor was :ci',
project_id: project.id project_id: project.id

View File

@ -82,50 +82,6 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do
end end
end end
end end
context 'when soft_validation_on_external_url feature flag is disabled' do
before do
stub_feature_flags(soft_validation_on_external_url: false)
end
where(:source_external_url, :expected_error_message) do
nil | nil
'http://example.com' | nil
'example.com' | 'is blocked: Only allowed schemes are http, https'
'www.example.io' | 'is blocked: Only allowed schemes are http, https'
'http://$URL' | 'is blocked: Hostname or IP address invalid'
'http://$(URL)' | 'is blocked: Hostname or IP address invalid'
'custom://example.com' | 'is blocked: Only allowed schemes are http, https'
'1.1.1.1' | 'is blocked: Only allowed schemes are http, https'
'$BASE_URL/${CI_COMMIT_REF_NAME}' | 'is blocked: Only allowed schemes are http, https'
'$ENVIRONMENT_URL' | 'is blocked: Only allowed schemes are http, https'
'https://$SUB.$MAIN' | 'is blocked: Hostname or IP address invalid'
'https://$SUB-$REGION.$MAIN' | 'is blocked: Hostname or IP address invalid'
'https://example.com?param={()}' | nil
'http://XSS?x=<script>alert(1)</script>' | nil
'https://user:${VARIABLE}@example.io' | nil
'https://example.com/test?param={data}' | nil
'http://${URL}' | 'is blocked: URI is invalid'
'https://${URL}.example/test' | 'is blocked: URI is invalid'
'http://test${CI_MERGE_REQUEST_IID}.example.com' | 'is blocked: URI is invalid'
'javascript:alert("hello")' | 'is blocked: Only allowed schemes are http, https'
end
with_them do
it 'sets an external URL or an error' do
environment.external_url = source_external_url
environment.valid?
if expected_error_message
expect(environment.errors[:external_url].first).to eq(expected_error_message)
else
expect(environment.errors[:external_url]).to be_empty,
"There were unexpected errors: #{environment.errors.full_messages}"
expect(environment.external_url).to eq(source_external_url)
end
end
end
end
end end
describe '.before_save' do describe '.before_save' do

View File

@ -34,10 +34,7 @@ RSpec.describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_
context 'cache clearing' do context 'cache clearing' do
it 'clears the cache for older diffs on the merge request' do it 'clears the cache for older diffs on the merge request' do
redis = instance_double(Redis) expect_any_instance_of(Redis).to receive(:del).once.and_call_original
expect(Gitlab::Redis::Cache).to receive(:with).and_yield(redis)
expect(redis).to receive(:del).once
expect(Rails.cache).to receive(:delete).once.and_call_original expect(Rails.cache).to receive(:delete).once.and_call_original
subject.execute subject.execute

View File

@ -4,7 +4,8 @@ require 'spec_helper'
RSpec.describe WorkItems::UpdateService do RSpec.describe WorkItems::UpdateService do
let_it_be(:developer) { create(:user) } let_it_be(:developer) { create(:user) }
let_it_be(:project) { create(:project).tap { |proj| proj.add_developer(developer) } } let_it_be(:guest) { create(:user) }
let_it_be(:project) { create(:project) }
let_it_be(:parent) { create(:work_item, project: project) } let_it_be(:parent) { create(:work_item, project: project) }
let_it_be_with_reload(:work_item) { create(:work_item, project: project, assignees: [developer]) } let_it_be_with_reload(:work_item) { create(:work_item, project: project, assignees: [developer]) }
@ -13,6 +14,11 @@ RSpec.describe WorkItems::UpdateService do
let(:opts) { {} } let(:opts) { {} }
let(:current_user) { developer } let(:current_user) { developer }
before do
project.add_developer(developer)
project.add_guest(guest)
end
describe '#execute' do describe '#execute' do
let(:service) do let(:service) do
described_class.new( described_class.new(
@ -102,7 +108,7 @@ RSpec.describe WorkItems::UpdateService do
let(:supported_widgets) do let(:supported_widgets) do
[ [
{ klass: WorkItems::Widgets::DescriptionService::UpdateService, callback: :update, params: { description: 'foo' } }, { klass: WorkItems::Widgets::DescriptionService::UpdateService, callback: :before_update_callback, params: { description: 'foo' } },
{ klass: WorkItems::Widgets::HierarchyService::UpdateService, callback: :before_update_in_transaction, params: { parent: parent } } { klass: WorkItems::Widgets::HierarchyService::UpdateService, callback: :before_update_in_transaction, params: { parent: parent } }
] ]
end end
@ -126,7 +132,7 @@ RSpec.describe WorkItems::UpdateService do
before do before do
allow_next_instance_of(widget_service_class) do |instance| allow_next_instance_of(widget_service_class) do |instance|
allow(instance) allow(instance)
.to receive(:update) .to receive(:before_update_callback)
.with(params: { description: 'changed' }).and_return(nil) .with(params: { description: 'changed' }).and_return(nil)
end end
end end
@ -143,6 +149,28 @@ RSpec.describe WorkItems::UpdateService do
expect(work_item.description).to eq('changed') expect(work_item.description).to eq('changed')
end end
context 'with mentions', :mailer, :sidekiq_might_not_need_inline do
shared_examples 'creates the todo and sends email' do |attribute|
it 'creates a todo and sends email' do
expect { perform_enqueued_jobs { update_work_item } }.to change(Todo, :count).by(1)
expect(work_item.reload.attributes[attribute.to_s]).to eq("mention #{guest.to_reference}")
should_email(guest)
end
end
context 'when description contains a user mention' do
let(:widget_params) { { description_widget: { description: "mention #{guest.to_reference}" } } }
it_behaves_like 'creates the todo and sends email', :description
end
context 'when title contains a user mention' do
let(:opts) { { title: "mention #{guest.to_reference}" } }
it_behaves_like 'creates the todo and sends email', :title
end
end
context 'when work item validation fails' do context 'when work item validation fails' do
let(:opts) { { title: '' } } let(:opts) { { title: '' } }

View File

@ -3,32 +3,102 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe WorkItems::Widgets::DescriptionService::UpdateService do RSpec.describe WorkItems::Widgets::DescriptionService::UpdateService do
let_it_be(:user) { create(:user) } let_it_be(:random_user) { create(:user) }
let_it_be(:project) { create(:project) } let_it_be(:author) { create(:user) }
let_it_be_with_reload(:work_item) { create(:work_item, project: project, description: 'old description') } let_it_be(:guest) { create(:user) }
let_it_be(:reporter) { create(:user) }
let_it_be(:project) { create(:project, :public) }
let(:params) { { description: 'updated description' } }
let(:current_user) { author }
let(:work_item) do
create(:work_item, author: author, project: project, description: 'old description',
last_edited_at: Date.yesterday, last_edited_by: random_user
)
end
let(:widget) { work_item.widgets.find {|widget| widget.is_a?(WorkItems::Widgets::Description) } } let(:widget) { work_item.widgets.find {|widget| widget.is_a?(WorkItems::Widgets::Description) } }
describe '#update' do describe '#update' do
subject { described_class.new(widget: widget, current_user: user).update(params: params) } # rubocop:disable Rails/SaveBang subject { described_class.new(widget: widget, current_user: current_user).before_update_callback(params: params) }
context 'when description param is present' do
let(:params) { { description: 'updated description' } }
shared_examples 'sets work item description' do
it 'correctly sets work item description value' do it 'correctly sets work item description value' do
subject subject
expect(work_item.description).to eq('updated description') expect(work_item.description).to eq(params[:description])
expect(work_item.last_edited_by).to eq(current_user)
expect(work_item.last_edited_at).to be_within(2.seconds).of(Time.current)
end end
end end
context 'when description param is not present' do shared_examples 'does not set work item description' do
let(:params) { {} }
it 'does not change work item description value' do it 'does not change work item description value' do
subject subject
expect(work_item.description).to eq('old description') expect(work_item.description).to eq('old description')
expect(work_item.last_edited_by).to eq(random_user)
expect(work_item.last_edited_at).to eq(Date.yesterday)
end
end
context 'when user has permission to update description' do
context 'when user is work item author' do
let(:current_user) { author }
it_behaves_like 'sets work item description'
end
context 'when user is a project reporter' do
let(:current_user) { reporter }
before do
project.add_reporter(reporter)
end
it_behaves_like 'sets work item description'
end
context 'when description is nil' do
let(:current_user) { author }
let(:params) { { description: nil } }
it_behaves_like 'sets work item description'
end
context 'when description is empty' do
let(:current_user) { author }
let(:params) { { description: '' } }
it_behaves_like 'sets work item description'
end
context 'when description param is not present' do
let(:params) { {} }
it_behaves_like 'does not set work item description'
end
end
context 'when user does not have permission to update description' do
context 'when user is a project guest' do
let(:current_user) { guest }
before do
project.add_guest(guest)
end
it_behaves_like 'does not set work item description'
end
context 'with private project' do
let_it_be(:project) { create(:project) }
context 'when user is work item author' do
let(:current_user) { author }
it_behaves_like 'does not set work item description'
end
end end
end end
end end

View File

@ -208,7 +208,6 @@ RSpec.configure do |config|
include StubFeatureFlags include StubFeatureFlags
include StubSnowplow include StubSnowplow
include StubMember
if ENV['CI'] || ENV['RETRIES'] if ENV['CI'] || ENV['RETRIES']
# This includes the first try, i.e. tests will be run 4 times before failing. # This includes the first try, i.e. tests will be run 4 times before failing.

View File

@ -1,8 +0,0 @@
# frozen_string_literal: true
module StubMember
def self.included(base)
GroupMember.prepend(StubbedMember::GroupMember)
ProjectMember.prepend(StubbedMember::ProjectMember)
end
end

View File

@ -1,64 +0,0 @@
# frozen_string_literal: true
# Extend the ProjectMember & GroupMember class with the ability to
# to run project_authorizations refresh jobs inline.
# This is needed so that calls like `group.add_member(user)` or `create(:project_member)`
# in the specs can be run without including `:sidekiq_inline` trait.
module StubbedMember
extend ActiveSupport::Concern
module ClearDeduplicationData
private
def clear_deduplication_data!
Gitlab::Redis::Queues.with do |redis|
redis.scan_each(match: '*duplicate*').each do |key|
redis.del(key)
end
end
end
end
module GroupMember
include ClearDeduplicationData
private
def refresh_member_authorized_projects(blocking:)
return super unless blocking
# First, we remove all the keys associated with deduplication from Redis.
# We can't perform a full flush with `Gitlab::Redis::Queues.with(&:flushdb)`
# because that is going to remove other, unrelated enqueued jobs as well,
# and that is going to fail some specs.
clear_deduplication_data!
# then we run `super`, which will enqueue a project authorizations refresh job
super
# then we drain (run) the jobs that were enqueued, but only for the worker class we are interested in.
AuthorizedProjectsWorker.drain
ensure
clear_deduplication_data!
end
end
module ProjectMember
include ClearDeduplicationData
private
def refresh_member_authorized_projects(blocking:)
return super unless blocking
clear_deduplication_data!
super
AuthorizedProjectUpdate::ProjectRecalculatePerUserWorker.drain
ensure
clear_deduplication_data!
end
end
end

View File

@ -30,33 +30,19 @@ RSpec.describe WaitableWorker do
describe '.bulk_perform_and_wait' do describe '.bulk_perform_and_wait' do
context '1 job' do context '1 job' do
it 'runs the jobs asynchronously' do it 'inlines the job' do
arguments = [[1]] args_list = [[1]]
expect(worker).to receive(:bulk_perform_inline).with(args_list).and_call_original
expect(Gitlab::AppJsonLogger).to(
receive(:info).with(a_hash_including('message' => 'running inline',
'class' => 'Gitlab::Foo::Bar::DummyWorker',
'job_status' => 'running',
'queue' => 'foo_bar_dummy'))
.once)
expect(worker).to receive(:bulk_perform_async).with(arguments) worker.bulk_perform_and_wait(args_list)
worker.bulk_perform_and_wait(arguments) expect(worker.counter).to eq(1)
end
context 'when the feature flag `always_async_project_authorizations_refresh` is turned off' do
before do
stub_feature_flags(always_async_project_authorizations_refresh: false)
end
it 'inlines the job' do
args_list = [[1]]
expect(worker).to receive(:bulk_perform_inline).with(args_list).and_call_original
expect(Gitlab::AppJsonLogger).to(
receive(:info).with(a_hash_including('message' => 'running inline',
'class' => 'Gitlab::Foo::Bar::DummyWorker',
'job_status' => 'running',
'queue' => 'foo_bar_dummy'))
.once)
worker.bulk_perform_and_wait(args_list)
expect(worker.counter).to eq(1)
end
end end
end end