Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2022-07-27 12:08:34 +0300
committerGitLab Bot <gitlab-bot@gitlab.com>2022-07-27 12:08:34 +0300
commitd0a683e342c9e59bec8bb83879f0fc959b79224e (patch)
treefac5bd8be0faee2016b6ad67c25206ccf42c3447
parent0ab741639e9d9a83833661925167a98d96751135 (diff)
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--app/assets/javascripts/integrations/edit/components/dynamic_field.vue7
-rw-r--r--app/assets/javascripts/integrations/edit/components/integration_form.vue6
-rw-r--r--app/assets/javascripts/whats_new/components/feature.vue6
-rw-r--r--app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue14
-rw-r--r--app/assets/javascripts/work_items/graphql/work_item_links.query.graphql3
-rw-r--r--app/models/environment.rb7
-rw-r--r--app/services/work_items/update_service.rb4
-rw-r--r--app/services/work_items/widgets/base_service.rb7
-rw-r--r--app/services/work_items/widgets/description_service/update_service.rb8
-rw-r--r--app/services/work_items/widgets/hierarchy_service/base_service.rb8
-rw-r--r--app/workers/concerns/waitable_worker.rb6
-rw-r--r--config/feature_flags/development/always_async_project_authorizations_refresh.yml8
-rw-r--r--config/feature_flags/development/soft_validation_on_external_url.yml8
-rw-r--r--doc/ci/environments/index.md3
-rw-r--r--doc/development/documentation/site_architecture/global_nav.md33
-rw-r--r--spec/frontend/integrations/edit/components/dynamic_field_spec.js2
-rw-r--r--spec/frontend/work_items/components/work_item_links/work_item_links_spec.js22
-rw-r--r--spec/frontend/work_items/mock_data.js49
-rw-r--r--spec/lib/gitlab/git_access_spec.rb10
-rw-r--r--spec/models/environment_spec.rb44
-rw-r--r--spec/services/merge_requests/reload_diffs_service_spec.rb5
-rw-r--r--spec/services/work_items/update_service_spec.rb34
-rw-r--r--spec/services/work_items/widgets/description_service/update_service_spec.rb92
-rw-r--r--spec/spec_helper.rb1
-rw-r--r--spec/support/helpers/stub_member.rb8
-rw-r--r--spec/support/helpers/stubbed_member.rb64
-rw-r--r--spec/workers/concerns/waitable_worker_spec.rb40
27 files changed, 269 insertions, 230 deletions
diff --git a/app/assets/javascripts/integrations/edit/components/dynamic_field.vue b/app/assets/javascripts/integrations/edit/components/dynamic_field.vue
index b4ceec22822..fe687ea9767 100644
--- a/app/assets/javascripts/integrations/edit/components/dynamic_field.vue
+++ b/app/assets/javascripts/integrations/edit/components/dynamic_field.vue
@@ -133,9 +133,6 @@ export default {
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>
@@ -147,7 +144,7 @@ export default {
:state="valid"
>
<template v-if="!isCheckbox" #description>
- <span v-safe-html:[$options.helpHtmlConfig]="help"></span>
+ <span v-safe-html="help"></span>
</template>
<template v-if="isCheckbox">
@@ -155,7 +152,7 @@ export default {
<gl-form-checkbox :id="fieldId" v-model="model" :disabled="isInheriting">
{{ checkboxLabel || humanizedTitle }}
<template #help>
- <span v-safe-html:[$options.helpHtmlConfig]="help"></span>
+ <span v-safe-html="help"></span>
</template>
</gl-form-checkbox>
</template>
diff --git a/app/assets/javascripts/integrations/edit/components/integration_form.vue b/app/assets/javascripts/integrations/edit/components/integration_form.vue
index f1f574c6424..7a6f1a953a8 100644
--- a/app/assets/javascripts/integrations/edit/components/integration_form.vue
+++ b/app/assets/javascripts/integrations/edit/components/integration_form.vue
@@ -192,11 +192,7 @@ export default {
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: {
- 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
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] }}
</gl-badge>
</h4>
- <p v-safe-html:[$options.descriptionHtmlConfig]="section.description"></p>
+ <p v-safe-html="section.description"></p>
</div>
<div class="col-lg-8">
diff --git a/app/assets/javascripts/whats_new/components/feature.vue b/app/assets/javascripts/whats_new/components/feature.vue
index 90f6230ef72..9380a77c1fb 100644
--- a/app/assets/javascripts/whats_new/components/feature.vue
+++ b/app/assets/javascripts/whats_new/components/feature.vue
@@ -30,7 +30,6 @@ export default {
return dateInWords(date);
},
},
- safeHtmlConfig: { ADD_ATTR: ['target'] },
};
</script>
@@ -76,10 +75,7 @@ export default {
{{ packageName }}
</gl-badge>
</div>
- <div
- v-safe-html:[$options.safeHtmlConfig]="feature.body"
- class="gl-pt-3 gl-line-height-20"
- ></div>
+ <div v-safe-html="feature.body" class="gl-pt-3 gl-line-height-20"></div>
<gl-button
:href="feature.url"
target="_blank"
diff --git a/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue b/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue
index 89f086cfca5..e65f0cfb1f2 100644
--- a/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue
+++ b/app/assets/javascripts/work_items/components/work_item_links/work_item_links.vue
@@ -43,6 +43,7 @@ export default {
};
},
update(data) {
+ this.canUpdate = data.workItem.userPermissions.updateWorkItem;
return (
data.workItem.widgets.find((widget) => widget.type === WIDGET_TYPE_HIERARCHY)?.children
.nodes ?? []
@@ -51,6 +52,9 @@ export default {
skip() {
return !this.issuableId;
},
+ result({ data }) {
+ this.canUpdate = data.workItem.userPermissions.updateWorkItem;
+ },
},
},
data() {
@@ -58,6 +62,7 @@ export default {
isShownAddForm: false,
isOpen: true,
children: [],
+ canUpdate: false,
};
},
computed: {
@@ -117,7 +122,7 @@ export default {
>
<h5 class="gl-m-0 gl-line-height-32 gl-flex-grow-1">{{ $options.i18n.title }}</h5>
<gl-button
- v-if="!isShownAddForm"
+ v-if="!isShownAddForm && canUpdate"
category="secondary"
data-testid="toggle-add-form"
@click="toggleAddForm"
@@ -173,7 +178,12 @@ export default {
$options.WORK_ITEM_STATUS_TEXT[child.state]
}}</span>
</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>
</template>
diff --git a/app/assets/javascripts/work_items/graphql/work_item_links.query.graphql b/app/assets/javascripts/work_items/graphql/work_item_links.query.graphql
index c2496f53cc8..921f75ccb0a 100644
--- a/app/assets/javascripts/work_items/graphql/work_item_links.query.graphql
+++ b/app/assets/javascripts/work_items/graphql/work_item_links.query.graphql
@@ -5,6 +5,9 @@ query workItemQuery($id: WorkItemID!) {
id
}
title
+ userPermissions {
+ updateWorkItem
+ }
widgets {
type
... on WorkItemWidgetHierarchy {
diff --git a/app/models/environment.rb b/app/models/environment.rb
index b93f1158a90..855fb281d81 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -58,8 +58,7 @@ class Environment < ApplicationRecord
length: { maximum: 255 },
allow_nil: true
- validates :external_url, addressable_url: true, allow_nil: true, unless: :soft_validation_on_external_url_enabled?
- validate :safe_external_url, if: :soft_validation_on_external_url_enabled?
+ validate :safe_external_url
delegate :manual_actions, :other_manual_actions, to: :last_deployment, allow_nil: true
delegate :auto_rollback_enabled?, to: :project
@@ -491,10 +490,6 @@ class Environment < ApplicationRecord
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
# 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.
diff --git a/app/services/work_items/update_service.rb b/app/services/work_items/update_service.rb
index 98818fda263..2deb8c82741 100644
--- a/app/services/work_items/update_service.rb
+++ b/app/services/work_items/update_service.rb
@@ -26,8 +26,8 @@ module WorkItems
private
- def update(work_item)
- execute_widgets(work_item: work_item, callback: :update, widget_params: @widget_params)
+ def before_update(work_item, skip_spam_check: false)
+ execute_widgets(work_item: work_item, callback: :before_update_callback, widget_params: @widget_params)
super
end
diff --git a/app/services/work_items/widgets/base_service.rb b/app/services/work_items/widgets/base_service.rb
index c695651361e..37ed2bf4b05 100644
--- a/app/services/work_items/widgets/base_service.rb
+++ b/app/services/work_items/widgets/base_service.rb
@@ -5,17 +5,18 @@ module WorkItems
class BaseService < ::BaseService
WidgetError = Class.new(StandardError)
- attr_reader :widget, :current_user
+ attr_reader :widget, :work_item, :current_user
def initialize(widget:, current_user:)
@widget = widget
+ @work_item = widget.work_item
@current_user = current_user
end
private
- def can_admin_work_item?
- can?(current_user, :admin_work_item, widget.work_item)
+ def has_permission?(permission)
+ can?(current_user, permission, widget.work_item)
end
end
end
diff --git a/app/services/work_items/widgets/description_service/update_service.rb b/app/services/work_items/widgets/description_service/update_service.rb
index e63b6b2ee6c..fe591ba605e 100644
--- a/app/services/work_items/widgets/description_service/update_service.rb
+++ b/app/services/work_items/widgets/description_service/update_service.rb
@@ -4,10 +4,12 @@ module WorkItems
module Widgets
module DescriptionService
class UpdateService < WorkItems::Widgets::BaseService
- def update(params: {})
- return unless params.present? && params[:description]
+ def before_update_callback(params: {})
+ 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
diff --git a/app/services/work_items/widgets/hierarchy_service/base_service.rb b/app/services/work_items/widgets/hierarchy_service/base_service.rb
index 05625cb5240..ca49597d7d6 100644
--- a/app/services/work_items/widgets/hierarchy_service/base_service.rb
+++ b/app/services/work_items/widgets/hierarchy_service/base_service.rb
@@ -29,13 +29,13 @@ module WorkItems
def set_parent(parent)
::WorkItems::ParentLinks::CreateService
- .new(parent, current_user, { target_issuable: widget.work_item })
+ .new(parent, current_user, { target_issuable: work_item })
.execute
end
# rubocop: disable CodeReuse/ActiveRecord
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?
::WorkItems::ParentLinks::DestroyService.new(link, current_user).execute
@@ -44,12 +44,12 @@ module WorkItems
def update_work_item_children(children)
::WorkItems::ParentLinks::CreateService
- .new(widget.work_item, current_user, { issuable_references: children })
+ .new(work_item, current_user, { issuable_references: children })
.execute
end
def feature_flag_enabled?
- Feature.enabled?(:work_items_hierarchy, widget.work_item&.project)
+ Feature.enabled?(:work_items_hierarchy, work_item&.project)
end
def incompatible_args?(params)
diff --git a/app/workers/concerns/waitable_worker.rb b/app/workers/concerns/waitable_worker.rb
index 9300c2a5790..336d60d46ac 100644
--- a/app/workers/concerns/waitable_worker.rb
+++ b/app/workers/concerns/waitable_worker.rb
@@ -7,7 +7,7 @@ module WaitableWorker
# Schedules multiple jobs and waits for them to be completed.
def bulk_perform_and_wait(args_list)
# 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)
end
@@ -29,10 +29,6 @@ module WaitableWorker
bulk_perform_async(failed) if failed.present?
end
-
- def always_async_project_authorizations_refresh?
- Feature.enabled?(:always_async_project_authorizations_refresh)
- end
end
def perform(*args)
diff --git a/config/feature_flags/development/always_async_project_authorizations_refresh.yml b/config/feature_flags/development/always_async_project_authorizations_refresh.yml
deleted file mode 100644
index 233be4d930e..00000000000
--- a/config/feature_flags/development/always_async_project_authorizations_refresh.yml
+++ /dev/null
@@ -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
diff --git a/config/feature_flags/development/soft_validation_on_external_url.yml b/config/feature_flags/development/soft_validation_on_external_url.yml
deleted file mode 100644
index 3df63a7bb96..00000000000
--- a/config/feature_flags/development/soft_validation_on_external_url.yml
+++ /dev/null
@@ -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
diff --git a/doc/ci/environments/index.md b/doc/ci/environments/index.md
index 9238f783004..7747f5e9b78 100644
--- a/doc/ci/environments/index.md
+++ b/doc/ci/environments/index.md
@@ -373,7 +373,8 @@ To retry or rollback a deployment:
### 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
places in GitLab:
diff --git a/doc/development/documentation/site_architecture/global_nav.md b/doc/development/documentation/site_architecture/global_nav.md
index e1e0da03abc..5ec4415a206 100644
--- a/doc/development/documentation/site_architecture/global_nav.md
+++ b/doc/development/documentation/site_architecture/global_nav.md
@@ -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.
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
@@ -39,20 +39,35 @@ as helpful as **Get started with runners**.
## Add a navigation entry
-All topics should be included in the left nav.
-
To add a topic to the global nav, edit
[`navigation.yaml`](https://gitlab.com/gitlab-org/gitlab-docs/blob/main/content/_data/navigation.yaml)
and add your item.
-All new pages need a navigation item. Without a navigation, the page becomes "orphaned." That
-is:
+Without a navigation entry:
+
+- The navigation closes when the page is opened, and the reader loses their place.
+- The page isn't visible in a group with other pages.
+
+### Pages you don't need to add
+
+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.
-- The navigation shuts when the page is opened, and the reader loses their place.
-- The page doesn't belong in a group with other pages.
+All other pages should be in the global nav.
-This means the decision to create a new page is a decision to create new navigation item and vice
-versa.
+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
diff --git a/spec/frontend/integrations/edit/components/dynamic_field_spec.js b/spec/frontend/integrations/edit/components/dynamic_field_spec.js
index ee2f6541b03..5af0e272285 100644
--- a/spec/frontend/integrations/edit/components/dynamic_field_spec.js
+++ b/spec/frontend/integrations/edit/components/dynamic_field_spec.js
@@ -204,7 +204,7 @@ describe('DynamicField', () => {
});
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>]',
);
});
});
diff --git a/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js b/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js
index 2ec9b1ec0ac..3b780bcc5bc 100644
--- a/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js
+++ b/spec/frontend/work_items/components/work_item_links/work_item_links_spec.js
@@ -6,7 +6,11 @@ import createMockApollo from 'helpers/mock_apollo_helper';
import waitForPromises from 'helpers/wait_for_promises';
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 { workItemHierarchyResponse, workItemHierarchyEmptyResponse } from '../../mock_data';
+import {
+ workItemHierarchyResponse,
+ workItemHierarchyEmptyResponse,
+ workItemHierarchyNoUpdatePermissionResponse,
+} from '../../mock_data';
Vue.use(VueApollo);
@@ -29,6 +33,7 @@ describe('WorkItemLinks', () => {
const findEmptyState = () => wrapper.findByTestId('links-empty');
const findToggleAddFormButton = () => wrapper.findByTestId('toggle-add-form');
const findAddLinksForm = () => wrapper.findByTestId('add-links-form');
+ const findFirstLinksMenu = () => wrapper.findByTestId('links-menu');
beforeEach(async () => {
await createComponent();
@@ -82,5 +87,20 @@ describe('WorkItemLinks', () => {
expect(children).toHaveLength(4);
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);
+ });
});
});
diff --git a/spec/frontend/work_items/mock_data.js b/spec/frontend/work_items/mock_data.js
index a76407931d4..aa0f7317bd0 100644
--- a/spec/frontend/work_items/mock_data.js
+++ b/spec/frontend/work_items/mock_data.js
@@ -307,6 +307,9 @@ export const workItemHierarchyEmptyResponse = {
__typename: 'WorkItemType',
},
title: 'New title',
+ userPermissions: {
+ updateWorkItem: false,
+ },
widgets: [
{
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 = {
data: {
workItem: {
@@ -336,6 +382,9 @@ export const workItemHierarchyResponse = {
__typename: 'WorkItemType',
},
title: 'New title',
+ userPermissions: {
+ updateWorkItem: true,
+ },
widgets: [
{
type: 'DESCRIPTION',
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 8577cad1011..31a5bc737ba 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -153,6 +153,11 @@ RSpec.describe Gitlab::GitAccess, :aggregate_failures do
end
it 'logs' do
+ allow(Gitlab::AppJsonLogger).to receive(:info).with(
+ hash_including(
+ "class" => "AuthorizedProjectUpdate::ProjectRecalculatePerUserWorker"
+ )
+ )
expect(Gitlab::AppJsonLogger).to receive(:info).with(
message: 'Actor was :ci',
project_id: project.id
@@ -746,6 +751,11 @@ RSpec.describe Gitlab::GitAccess, :aggregate_failures do
it 'logs' do
expect(Gitlab::AppJsonLogger).to receive(:info).with(
+ hash_including(
+ "class" => "AuthorizedProjectUpdate::ProjectRecalculatePerUserWorker"
+ )
+ ).once
+ expect(Gitlab::AppJsonLogger).to receive(:info).with(
message: 'Actor was :ci',
project_id: project.id
).once
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index 26883fc57b8..16e56f92b6a 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -82,50 +82,6 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do
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
describe '.before_save' do
diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb
index ed89a02b570..3d5b65207e6 100644
--- a/spec/services/merge_requests/reload_diffs_service_spec.rb
+++ b/spec/services/merge_requests/reload_diffs_service_spec.rb
@@ -34,10 +34,7 @@ RSpec.describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_
context 'cache clearing' do
it 'clears the cache for older diffs on the merge request' do
- redis = instance_double(Redis)
- expect(Gitlab::Redis::Cache).to receive(:with).and_yield(redis)
-
- expect(redis).to receive(:del).once
+ expect_any_instance_of(Redis).to receive(:del).once.and_call_original
expect(Rails.cache).to receive(:delete).once.and_call_original
subject.execute
diff --git a/spec/services/work_items/update_service_spec.rb b/spec/services/work_items/update_service_spec.rb
index 9db4139c6d5..dd5924e7434 100644
--- a/spec/services/work_items/update_service_spec.rb
+++ b/spec/services/work_items/update_service_spec.rb
@@ -4,7 +4,8 @@ require 'spec_helper'
RSpec.describe WorkItems::UpdateService do
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_with_reload(:work_item) { create(:work_item, project: project, assignees: [developer]) }
@@ -13,6 +14,11 @@ RSpec.describe WorkItems::UpdateService do
let(:opts) { {} }
let(:current_user) { developer }
+ before do
+ project.add_developer(developer)
+ project.add_guest(guest)
+ end
+
describe '#execute' do
let(:service) do
described_class.new(
@@ -102,7 +108,7 @@ RSpec.describe WorkItems::UpdateService 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 } }
]
end
@@ -126,7 +132,7 @@ RSpec.describe WorkItems::UpdateService do
before do
allow_next_instance_of(widget_service_class) do |instance|
allow(instance)
- .to receive(:update)
+ .to receive(:before_update_callback)
.with(params: { description: 'changed' }).and_return(nil)
end
end
@@ -143,6 +149,28 @@ RSpec.describe WorkItems::UpdateService do
expect(work_item.description).to eq('changed')
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
let(:opts) { { title: '' } }
diff --git a/spec/services/work_items/widgets/description_service/update_service_spec.rb b/spec/services/work_items/widgets/description_service/update_service_spec.rb
index a2eceb97f09..37b3f03fc6a 100644
--- a/spec/services/work_items/widgets/description_service/update_service_spec.rb
+++ b/spec/services/work_items/widgets/description_service/update_service_spec.rb
@@ -3,32 +3,102 @@
require 'spec_helper'
RSpec.describe WorkItems::Widgets::DescriptionService::UpdateService do
- let_it_be(:user) { create(:user) }
- let_it_be(:project) { create(:project) }
- let_it_be_with_reload(:work_item) { create(:work_item, project: project, description: 'old description') }
+ let_it_be(:random_user) { create(:user) }
+ let_it_be(:author) { create(:user) }
+ 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) } }
describe '#update' do
- subject { described_class.new(widget: widget, current_user: user).update(params: params) } # rubocop:disable Rails/SaveBang
-
- context 'when description param is present' do
- let(:params) { { description: 'updated description' } }
+ subject { described_class.new(widget: widget, current_user: current_user).before_update_callback(params: params) }
+ shared_examples 'sets work item description' do
it 'correctly sets work item description value' do
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
- context 'when description param is not present' do
- let(:params) { {} }
-
+ shared_examples 'does not set work item description' do
it 'does not change work item description value' do
subject
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
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 921d6503099..47cd78873f8 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -208,7 +208,6 @@ RSpec.configure do |config|
include StubFeatureFlags
include StubSnowplow
- include StubMember
if ENV['CI'] || ENV['RETRIES']
# This includes the first try, i.e. tests will be run 4 times before failing.
diff --git a/spec/support/helpers/stub_member.rb b/spec/support/helpers/stub_member.rb
deleted file mode 100644
index b8fb7bce234..00000000000
--- a/spec/support/helpers/stub_member.rb
+++ /dev/null
@@ -1,8 +0,0 @@
-# frozen_string_literal: true
-
-module StubMember
- def self.included(base)
- GroupMember.prepend(StubbedMember::GroupMember)
- ProjectMember.prepend(StubbedMember::ProjectMember)
- end
-end
diff --git a/spec/support/helpers/stubbed_member.rb b/spec/support/helpers/stubbed_member.rb
deleted file mode 100644
index bf700229656..00000000000
--- a/spec/support/helpers/stubbed_member.rb
+++ /dev/null
@@ -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
diff --git a/spec/workers/concerns/waitable_worker_spec.rb b/spec/workers/concerns/waitable_worker_spec.rb
index bf156c3b8cb..f6d4cc4679d 100644
--- a/spec/workers/concerns/waitable_worker_spec.rb
+++ b/spec/workers/concerns/waitable_worker_spec.rb
@@ -30,33 +30,19 @@ RSpec.describe WaitableWorker do
describe '.bulk_perform_and_wait' do
context '1 job' do
- it 'runs the jobs asynchronously' do
- arguments = [[1]]
-
- expect(worker).to receive(:bulk_perform_async).with(arguments)
-
- worker.bulk_perform_and_wait(arguments)
- 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
+ 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