diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-10-22 18:19:11 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-10-22 18:19:11 +0300 |
commit | 56c37d6e4bcb8cc1629318648269cc6c8ba78790 (patch) | |
tree | b8db5fa32e7b9405c0a7740bf1fdcdf63c211db4 | |
parent | faec73b0fe1b2f19646363e4359f302fc1b9414c (diff) |
Add latest changes from gitlab-org/gitlab@master
64 files changed, 823 insertions, 121 deletions
diff --git a/app/assets/javascripts/pipeline_editor/components/file_nav/branch_switcher.vue b/app/assets/javascripts/pipeline_editor/components/file_nav/branch_switcher.vue index 68065cc3c73..c1739f51f6b 100644 --- a/app/assets/javascripts/pipeline_editor/components/file_nav/branch_switcher.vue +++ b/app/assets/javascripts/pipeline_editor/components/file_nav/branch_switcher.vue @@ -43,14 +43,25 @@ export default { }, inject: ['projectFullPath', 'totalBranches'], props: { + hasUnsavedChanges: { + type: Boolean, + required: false, + default: false, + }, paginationLimit: { type: Number, required: false, default: BRANCH_PAGINATION_LIMIT, }, + shouldLoadNewBranch: { + type: Boolean, + required: false, + default: false, + }, }, data() { return { + branchSelected: null, availableBranches: [], filteredBranches: [], isSearchingBranches: false, @@ -105,6 +116,13 @@ export default { return this.branches.length > 0 || this.searchTerm.length > 0; }, }, + watch: { + shouldLoadNewBranch(flag) { + if (flag) { + this.changeBranch(this.branchSelected); + } + }, + }, methods: { availableBranchesQueryVars(varsOverride = {}) { if (this.searchTerm.length > 0) { @@ -149,11 +167,7 @@ export default { }) .catch(this.showFetchError); }, - async selectBranch(newBranch) { - if (newBranch === this.currentBranch) { - return; - } - + async changeBranch(newBranch) { this.updateCurrentBranch(newBranch); const updatedPath = setUrlParams({ branch_name: newBranch }); historyPushState(updatedPath); @@ -164,6 +178,19 @@ export default { await this.$nextTick(); this.$emit('refetchContent'); }, + selectBranch(newBranch) { + if (newBranch !== this.currentBranch) { + // If there are unsaved changes, we want to show the user + // a modal to confirm what to do with these before changing + // branches. + if (this.hasUnsavedChanges) { + this.branchSelected = newBranch; + this.$emit('select-branch', newBranch); + } else { + this.changeBranch(newBranch); + } + } + }, async setSearchTerm(newSearchTerm) { this.pageCounter = 0; this.searchTerm = newSearchTerm.trim(); diff --git a/app/assets/javascripts/pipeline_editor/components/file_nav/pipeline_editor_file_nav.vue b/app/assets/javascripts/pipeline_editor/components/file_nav/pipeline_editor_file_nav.vue index 551a0430fbf..83b074dd55c 100644 --- a/app/assets/javascripts/pipeline_editor/components/file_nav/pipeline_editor_file_nav.vue +++ b/app/assets/javascripts/pipeline_editor/components/file_nav/pipeline_editor_file_nav.vue @@ -5,10 +5,26 @@ export default { components: { BranchSwitcher, }, + props: { + hasUnsavedChanges: { + type: Boolean, + required: false, + default: false, + }, + shouldLoadNewBranch: { + type: Boolean, + required: false, + default: false, + }, + }, }; </script> <template> <div class="gl-mb-4"> - <branch-switcher v-on="$listeners" /> + <branch-switcher + :has-unsaved-changes="hasUnsavedChanges" + :should-load-new-branch="shouldLoadNewBranch" + v-on="$listeners" + /> </div> </template> diff --git a/app/assets/javascripts/pipeline_editor/components/header/pipeline_editor_header.vue b/app/assets/javascripts/pipeline_editor/components/header/pipeline_editor_header.vue index fcc31f087ff..ec6ee52b6b2 100644 --- a/app/assets/javascripts/pipeline_editor/components/header/pipeline_editor_header.vue +++ b/app/assets/javascripts/pipeline_editor/components/header/pipeline_editor_header.vue @@ -63,6 +63,7 @@ export default { v-if="showPipelineStatus" :commit-sha="commitSha" :class="$options.pipelineStatusClasses" + v-on="$listeners" /> <validation-segment :class="validationStyling" :ci-config="ciConfigData" /> </div> diff --git a/app/assets/javascripts/pipeline_editor/components/header/pipeline_editor_mini_graph.vue b/app/assets/javascripts/pipeline_editor/components/header/pipeline_editor_mini_graph.vue index 75b1398a3c2..25a78aab933 100644 --- a/app/assets/javascripts/pipeline_editor/components/header/pipeline_editor_mini_graph.vue +++ b/app/assets/javascripts/pipeline_editor/components/header/pipeline_editor_mini_graph.vue @@ -1,17 +1,52 @@ <script> +import { __ } from '~/locale'; import PipelineMiniGraph from '~/pipelines/components/pipelines_list/pipeline_mini_graph.vue'; +import getLinkedPipelinesQuery from '~/projects/commit_box/info/graphql/queries/get_linked_pipelines.query.graphql'; +import { PIPELINE_FAILURE } from '../../constants'; export default { + i18n: { + linkedPipelinesFetchError: __('Unable to fetch upstream and downstream pipelines.'), + }, components: { PipelineMiniGraph, + LinkedPipelinesMiniList: () => + import('ee_component/vue_shared/components/linked_pipelines_mini_list.vue'), }, + inject: ['projectFullPath'], props: { pipeline: { type: Object, required: true, }, }, + apollo: { + linkedPipelines: { + query: getLinkedPipelinesQuery, + variables() { + return { + fullPath: this.projectFullPath, + iid: this.pipeline.iid, + }; + }, + skip() { + return !this.pipeline.iid; + }, + update({ project }) { + return project?.pipeline; + }, + error() { + this.$emit('showError', { + type: PIPELINE_FAILURE, + reasons: [this.$options.i18n.linkedPipelinesFetchError], + }); + }, + }, + }, computed: { + downstreamPipelines() { + return this.linkedPipelines?.downstream?.nodes || []; + }, pipelinePath() { return this.pipeline.detailedStatus?.detailsPath || ''; }, @@ -38,12 +73,29 @@ export default { }; }); }, + showDownstreamPipelines() { + return this.downstreamPipelines.length > 0; + }, + upstreamPipeline() { + return this.linkedPipelines?.upstream; + }, }, }; </script> <template> <div v-if="pipelineStages.length > 0" class="stage-cell gl-mr-5"> + <linked-pipelines-mini-list + v-if="upstreamPipeline" + :triggered-by="[upstreamPipeline]" + data-testid="pipeline-editor-mini-graph-upstream" + /> <pipeline-mini-graph class="gl-display-inline" :stages="pipelineStages" /> + <linked-pipelines-mini-list + v-if="showDownstreamPipelines" + :triggered="downstreamPipelines" + :pipeline-path="pipelinePath" + data-testid="pipeline-editor-mini-graph-downstream" + /> </div> </template> diff --git a/app/assets/javascripts/pipeline_editor/components/header/pipeline_status.vue b/app/assets/javascripts/pipeline_editor/components/header/pipeline_status.vue index a1fa2147994..f8eda561b49 100644 --- a/app/assets/javascripts/pipeline_editor/components/header/pipeline_status.vue +++ b/app/assets/javascripts/pipeline_editor/components/header/pipeline_status.vue @@ -59,11 +59,12 @@ export default { }; }, update(data) { - const { id, commitPath = '', detailedStatus = {}, stages, status } = + const { id, iid, commitPath = '', detailedStatus = {}, stages, status } = data.project?.pipeline || {}; return { id, + iid, commitPath, detailedStatus, stages, @@ -159,6 +160,7 @@ export default { <pipeline-editor-mini-graph v-if="glFeatures.pipelineEditorMiniGraph" :pipeline="pipeline" + v-on="$listeners" /> <gl-button class="gl-mt-2 gl-md-mt-0" diff --git a/app/assets/javascripts/pipeline_editor/components/ui/pipeline_editor_messages.vue b/app/assets/javascripts/pipeline_editor/components/ui/pipeline_editor_messages.vue index 091b202e10b..7206f19d060 100644 --- a/app/assets/javascripts/pipeline_editor/components/ui/pipeline_editor_messages.vue +++ b/app/assets/javascripts/pipeline_editor/components/ui/pipeline_editor_messages.vue @@ -8,6 +8,7 @@ import { DEFAULT_FAILURE, DEFAULT_SUCCESS, LOAD_FAILURE_UNKNOWN, + PIPELINE_FAILURE, } from '../../constants'; import CodeSnippetAlert from '../code_snippet_alert/code_snippet_alert.vue'; import { @@ -24,6 +25,7 @@ export default { [COMMIT_FAILURE]: s__('Pipelines|The GitLab CI configuration could not be updated.'), [DEFAULT_FAILURE]: __('Something went wrong on our end.'), [LOAD_FAILURE_UNKNOWN]: s__('Pipelines|The CI configuration was not loaded, please try again.'), + [PIPELINE_FAILURE]: s__('Pipelines|There was a problem with loading the pipeline data.'), }, successTexts: { [COMMIT_SUCCESS]: __('Your changes have been successfully committed.'), @@ -74,6 +76,11 @@ export default { text: this.$options.errorTexts[COMMIT_FAILURE], variant: 'danger', }; + case PIPELINE_FAILURE: + return { + text: this.$options.errorTexts[PIPELINE_FAILURE], + variant: 'danger', + }; default: return { text: this.$options.errorTexts[DEFAULT_FAILURE], diff --git a/app/assets/javascripts/pipeline_editor/constants.js b/app/assets/javascripts/pipeline_editor/constants.js index fee9053f208..e15fce794af 100644 --- a/app/assets/javascripts/pipeline_editor/constants.js +++ b/app/assets/javascripts/pipeline_editor/constants.js @@ -16,6 +16,7 @@ export const COMMIT_SUCCESS = 'COMMIT_SUCCESS'; export const DEFAULT_FAILURE = 'DEFAULT_FAILURE'; export const DEFAULT_SUCCESS = 'DEFAULT_SUCCESS'; export const LOAD_FAILURE_UNKNOWN = 'LOAD_FAILURE_UNKNOWN'; +export const PIPELINE_FAILURE = 'PIPELINE_FAILURE'; export const CREATE_TAB = 'CREATE_TAB'; export const LINT_TAB = 'LINT_TAB'; diff --git a/app/assets/javascripts/pipeline_editor/index.js b/app/assets/javascripts/pipeline_editor/index.js index 89b9091e6f9..68e4cb8d1f8 100644 --- a/app/assets/javascripts/pipeline_editor/index.js +++ b/app/assets/javascripts/pipeline_editor/index.js @@ -93,6 +93,7 @@ export const initPipelineEditor = (selector = '#js-pipeline-editor') => { ciExamplesHelpPagePath, ciHelpPagePath, configurationPaths, + dataMethod: 'graphql', defaultBranch, emptyStateIllustrationPath, helpPaths, diff --git a/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue b/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue index e70417145ab..a31e508059c 100644 --- a/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue +++ b/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue @@ -325,8 +325,9 @@ export default { <pipeline-editor-home :ci-config-data="ciConfigData" :ci-file-content="currentCiFileContent" - :is-new-ci-config-file="isNewCiConfigFile" :commit-sha="commitSha" + :has-unsaved-changes="hasUnsavedChanges" + :is-new-ci-config-file="isNewCiConfigFile" @commit="updateOnCommit" @resetContent="resetContent" @showError="showErrorAlert" diff --git a/app/assets/javascripts/pipeline_editor/pipeline_editor_home.vue b/app/assets/javascripts/pipeline_editor/pipeline_editor_home.vue index 5f9686834ea..b82e9faee53 100644 --- a/app/assets/javascripts/pipeline_editor/pipeline_editor_home.vue +++ b/app/assets/javascripts/pipeline_editor/pipeline_editor_home.vue @@ -1,4 +1,6 @@ <script> +import { GlModal } from '@gitlab/ui'; +import { __ } from '~/locale'; import CommitSection from './components/commit/commit_section.vue'; import PipelineEditorDrawer from './components/drawer/pipeline_editor_drawer.vue'; import PipelineEditorFileNav from './components/file_nav/pipeline_editor_file_nav.vue'; @@ -7,8 +9,23 @@ import PipelineEditorTabs from './components/pipeline_editor_tabs.vue'; import { CREATE_TAB } from './constants'; export default { + commitSectionRef: 'commitSectionRef', + modal: { + switchBranch: { + title: __('You have unsaved changes'), + body: __('Uncommitted changes will be lost if you change branches. Do you want to continue?'), + actionPrimary: { + text: __('Switch Branches'), + }, + actionSecondary: { + text: __('Cancel'), + attributes: { variant: 'default' }, + }, + }, + }, components: { CommitSection, + GlModal, PipelineEditorDrawer, PipelineEditorFileNav, PipelineEditorHeader, @@ -28,6 +45,11 @@ export default { required: false, default: '', }, + hasUnsavedChanges: { + type: Boolean, + required: false, + default: false, + }, isNewCiConfigFile: { type: Boolean, required: true, @@ -36,6 +58,8 @@ export default { data() { return { currentTab: CREATE_TAB, + shouldLoadNewBranch: false, + showSwitchBranchModal: false, }; }, computed: { @@ -44,6 +68,16 @@ export default { }, }, methods: { + closeBranchModal() { + this.showSwitchBranchModal = false; + }, + handleConfirmSwitchBranch() { + this.showSwitchBranchModal = true; + }, + switchBranch() { + this.showSwitchBranchModal = false; + this.shouldLoadNewBranch = true; + }, setCurrentTab(tabName) { this.currentTab = tabName; }, @@ -53,11 +87,31 @@ export default { <template> <div class="gl-pr-9 gl-transition-medium gl-w-full"> - <pipeline-editor-file-nav v-on="$listeners" /> + <gl-modal + v-if="showSwitchBranchModal" + visible + modal-id="switchBranchModal" + :title="$options.modal.switchBranch.title" + :action-primary="$options.modal.switchBranch.actionPrimary" + :action-secondary="$options.modal.switchBranch.actionSecondary" + @primary="switchBranch" + @secondary="closeBranchModal" + @cancel="closeBranchModal" + @hide="closeBranchModal" + > + {{ $options.modal.switchBranch.body }} + </gl-modal> + <pipeline-editor-file-nav + :has-unsaved-changes="hasUnsavedChanges" + :should-load-new-branch="shouldLoadNewBranch" + @select-branch="handleConfirmSwitchBranch" + v-on="$listeners" + /> <pipeline-editor-header :ci-config-data="ciConfigData" :commit-sha="commitSha" :is-new-ci-config-file="isNewCiConfigFile" + v-on="$listeners" /> <pipeline-editor-tabs :ci-config-data="ciConfigData" @@ -68,6 +122,7 @@ export default { /> <commit-section v-if="showCommitForm" + :ref="$options.commitSectionRef" :ci-file-content="ciFileContent" :commit-sha="commitSha" v-on="$listeners" diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index 774f81b0a5e..f0632780838 100644 --- a/app/services/groups/transfer_service.rb +++ b/app/services/groups/transfer_service.rb @@ -191,7 +191,7 @@ module Groups def localized_error_messages { database_not_supported: s_('TransferGroup|Database is not supported.'), - namespace_with_same_path: s_('TransferGroup|The parent group already has a subgroup with the same path.'), + namespace_with_same_path: s_('TransferGroup|The parent group already has a subgroup or a project with the same path.'), group_is_already_root: s_('TransferGroup|Group is already a root group.'), same_parent_as_current: s_('TransferGroup|Group is already associated to the parent group.'), invalid_policies: s_("TransferGroup|You don't have enough permissions."), diff --git a/app/views/layouts/_snowplow.html.haml b/app/views/layouts/_snowplow.html.haml index fc3b12acc46..7e242fb4a8e 100644 --- a/app/views/layouts/_snowplow.html.haml +++ b/app/views/layouts/_snowplow.html.haml @@ -10,5 +10,6 @@ window.snowplowOptions = #{Gitlab::Tracking.options(@group).to_json} gl = window.gl || {}; - gl.snowplowStandardContext = #{Gitlab::Tracking::StandardContext.new.to_context.to_json.to_json} + gl.snowplowStandardContext = #{Gitlab::Tracking::StandardContext.new(namespace: @group || @project&.namespace, + project: @project, user: current_user).to_context.to_json.to_json} gl.snowplowPseudonymizedPageUrl = #{masked_page_url.to_json}; diff --git a/doc/ci/services/index.md b/doc/ci/services/index.md index 4114833d1c8..9b3e9e62e8a 100644 --- a/doc/ci/services/index.md +++ b/doc/ci/services/index.md @@ -260,6 +260,7 @@ test: | `entrypoint` | no | 9.4 |Command or script to execute as the container's entrypoint. It's translated to Docker's `--entrypoint` option while creating the container. The syntax is similar to [`Dockerfile`'s `ENTRYPOINT`](https://docs.docker.com/engine/reference/builder/#entrypoint) directive, where each shell token is a separate string in the array. | | `command` | no | 9.4 |Command or script that should be used as the container's command. It's translated to arguments passed to Docker after the image's name. The syntax is similar to [`Dockerfile`'s `CMD`](https://docs.docker.com/engine/reference/builder/#cmd) directive, where each shell token is a separate string in the array. | | `alias` (1) | no | 9.4 |Additional alias that can be used to access the service from the job's container. Read [Accessing the services](#accessing-the-services) for more information. | +| `variables` | no | 14.5 | Additional environment variables that are passed exclusively to the service. The syntax is the same as [Job Variables](../variables/index.md). | (1) Alias support for the Kubernetes executor was [introduced](https://gitlab.com/gitlab-org/gitlab-runner/-/issues/2229) in GitLab Runner 12.8, and is only available for Kubernetes version 1.7 or later. diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index 73e26cdee39..8876b496aff 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -598,6 +598,11 @@ include: All [nested includes](#nested-includes) execute without context as a public user, so you can only `include` public projects or templates. +NOTE: +Be careful when including a remote CI/CD configuration file. No pipelines or notifications +trigger when external CI/CD configuration files change. From a security perspective, +this is similar to pulling a third party dependency. + #### `include:template` > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/issues/53445) in GitLab 11.7. diff --git a/doc/development/service_ping/implement.md b/doc/development/service_ping/implement.md index 34a845147dc..fdf02b5e2ce 100644 --- a/doc/development/service_ping/implement.md +++ b/doc/development/service_ping/implement.md @@ -295,19 +295,22 @@ Examples of implementation: - Using Redis methods [`INCR`](https://redis.io/commands/incr), [`GET`](https://redis.io/commands/get), and [`Gitlab::UsageDataCounters::WikiPageCounter`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data_counters/wiki_page_counter.rb) - Using Redis methods [`HINCRBY`](https://redis.io/commands/hincrby), [`HGETALL`](https://redis.io/commands/hgetall), and [`Gitlab::UsageCounters::PodLogs`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_counters/pod_logs.rb) -##### UsageData API tracking +##### `UsageData` API -<!-- There's nearly identical content in `##### Adding new events`. If you fix errors here, you may need to fix the same errors in the other location. --> +You can use the `UsageData` API to track events. +To track events, the `usage_data_api` feature flag must +be enabled (set to `default_enabled: true`). +Enabled by default in GitLab 13.7 and later. -1. Track event using `UsageData` API +##### UsageData API tracking - Increment event count using ordinary Redis counter, for given event name. +1. Track events using the [`UsageData` API](#usagedata-api). - Tracking events using the `UsageData` API requires the `usage_data_api` feature flag to be enabled, which is enabled by default. + Increment event count using an ordinary Redis counter, for a given event name. API requests are protected by checking for a valid CSRF token. - To be able to increment the values, the related feature `usage_data_<event_name>` should be enabled. + To increment the values, the related feature `usage_data_<event_name>` must be enabled. ```plaintext POST /usage_data/increment_counter @@ -315,18 +318,18 @@ Examples of implementation: | Attribute | Type | Required | Description | | :-------- | :--- | :------- | :---------- | - | `event` | string | yes | The event name it should be tracked | + | `event` | string | yes | The event name to track. | Response: - - `200` if event was tracked - - `400 Bad request` if event parameter is missing - - `401 Unauthorized` if user is not authenticated - - `403 Forbidden` for invalid CSRF token provided + - `200` if the event was tracked. + - `400 Bad request` if the event parameter is missing. + - `401 Unauthorized` if the user is not authenticated. + - `403 Forbidden` if an invalid CSRF token is provided. -1. Track events using JavaScript/Vue API helper which calls the API above +1. Track events using the JavaScript/Vue API helper which calls the [`UsageData` API](#usagedata-api). - Note that `usage_data_api` and `usage_data_#{event_name}` should be enabled to be able to track events + To track events, `usage_data_api` and `usage_data_#{event_name}` must be enabled. ```javascript import api from '~/api'; @@ -460,14 +463,10 @@ Implemented using Redis methods [PFADD](https://redis.io/commands/pfadd) and [PF track_usage_event(:incident_management_incident_created, current_user.id) ``` - - Using the `UsageData` API. - <!-- There's nearly identical content in `##### UsageData API Tracking`. If you find / fix errors here, you may need to fix errors in that section too. --> + - Using the [`UsageData` API](#usagedata-api). Increment unique users count using Redis HLL, for a given event name. - To track events using the `UsageData` API, ensure the `usage_data_api` feature flag - is set to `default_enabled: true`. Enabled by default in GitLab 13.7 and later. - API requests are protected by checking for a valid CSRF token. ```plaintext @@ -485,10 +484,7 @@ Implemented using Redis methods [PFADD](https://redis.io/commands/pfadd) and [PF - `401 Unauthorized` if the user is not authenticated. - `403 Forbidden` if an invalid CSRF token is provided. - - Using the JavaScript/Vue API helper, which calls the `UsageData` API. - - To track events using the `UsageData` API, ensure the `usage_data_api` feature flag - is set to `default_enabled: true`. Enabled by default in GitLab 13.7 and later. + - Using the JavaScript/Vue API helper, which calls the [`UsageData` API](#usagedata-api). Example for an existing event already defined in [known events](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data_counters/known_events/): diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md index e6288d254d3..3f84dffc99c 100644 --- a/doc/development/sidekiq_style_guide.md +++ b/doc/development/sidekiq_style_guide.md @@ -255,6 +255,11 @@ module Ci end ``` +Also, you can pass `if_deduplicated: :reschedule_once` option to re-run a job once after +the currently running job finished and deduplication happened at least once. +This ensures that the latest result is always produced even if a race condition +happened. See [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/342123) for more information. + #### Scheduling jobs in the future GitLab doesn't skip jobs scheduled in the future, as we assume that diff --git a/doc/subscriptions/bronze_starter.md b/doc/subscriptions/bronze_starter.md index 00d568ecb59..44af774e860 100644 --- a/doc/subscriptions/bronze_starter.md +++ b/doc/subscriptions/bronze_starter.md @@ -96,7 +96,7 @@ the tiers are no longer mentioned in GitLab documentation: - [Trigger pull mirroring from the API](../user/project/repository/mirror/pull.md#trigger-an-update-by-using-the-api) - [Mirror only protected branches](../user/project/repository/mirror/index.md#mirror-only-protected-branches) - [Bidirectional mirroring](../user/project/repository/mirror/bidirectional.md) - - [Mirror with Perforce Helix via Git Fusion](../user/project/repository/mirror/bidirectional.md#mirror-with-perforce-helix-via-git-fusion) + - [Mirror with Perforce Helix with Git Fusion](../user/project/repository/mirror/bidirectional.md#mirror-with-perforce-helix-with-git-fusion) - Runners: - Run pipelines in the parent project [for merge requests from a forked project](../ci/pipelines/merge_request_pipelines.md#run-pipelines-in-the-parent-project-for-merge-requests-from-a-forked-project) - [Shared runners pipeline minutes quota](../user/admin_area/settings/continuous_integration.md#shared-runners-pipeline-minutes-quota) diff --git a/doc/user/markdown.md b/doc/user/markdown.md index 5dfeab36b0d..e4141799ff7 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -216,15 +216,15 @@ If you're new to this, don't be :fearful:. You can join the emoji :family:. All Consult the [Emoji Cheat Sheet](https://www.emojicopy.com) for a list of all supported emoji codes. :thumbsup: ``` -Sometimes you want to <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/app/assets/images/emoji/monkey.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> around a bit and add some <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/app/assets/images/emoji/star2.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> to your <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/app/assets/images/emoji/speech_balloon.png" width="20px" height="20px" style="display:inline;margin:0;border: 0">. Well we have a gift for you: +Sometimes you want to <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/public/-/emojis/2/monkey.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> around a bit and add some <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/public/-/emojis/2/star2.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> to your <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/public/-/emojis/2/speech_balloon.png" width="20px" height="20px" style="display:inline;margin:0;border: 0">. Well we have a gift for you: -<img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/app/assets/images/emoji/zap.png" width="20px" height="20px" style="display:inline;margin:0;border: 0">You can use emoji anywhere GitLab Flavored Markdown is supported. <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/app/assets/images/emoji/v.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> +<img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/public/-/emojis/2/zap.png" width="20px" height="20px" style="display:inline;margin:0;border: 0">You can use emoji anywhere GitLab Flavored Markdown is supported. <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/public/-/emojis/2/v.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> -You can use it to point out a<img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/app/assets/images/emoji/bug.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> or warn about <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/app/assets/images/emoji/speak_no_evil.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> patches. If someone improves your really <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/app/assets/images/emoji/snail.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> code, send them some <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/app/assets/images/emoji/birthday.png" width="20px" height="20px" style="display:inline;margin:0;border: 0">. People <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/app/assets/images/emoji/heart.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> you for that. +You can use it to point out a<img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/public/-/emojis/2/bug.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> or warn about <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/public/-/emojis/2/speak_no_evil.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> patches. If someone improves your really <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/public/-/emojis/2/snail.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> code, send them some <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/public/-/emojis/2/birthday.png" width="20px" height="20px" style="display:inline;margin:0;border: 0">. People <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/public/-/emojis/2/heart.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> you for that. -If you're new to this, don't be <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/app/assets/images/emoji/fearful.png" width="20px" height="20px" style="display:inline;margin:0;border: 0">. You can join the emoji <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/app/assets/images/emoji/family.png" width="20px" height="20px" style="display:inline;margin:0;border: 0">. All you need to do is to look up one of the supported codes. +If you're new to this, don't be <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/public/-/emojis/2/fearful.png" width="20px" height="20px" style="display:inline;margin:0;border: 0">. You can join the emoji <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/public/-/emojis/2/family.png" width="20px" height="20px" style="display:inline;margin:0;border: 0">. All you need to do is to look up one of the supported codes. -Consult the [Emoji Cheat Sheet](https://www.webfx.com/tools/emoji-cheat-sheet/) for a list of all supported emoji codes. <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/app/assets/images/emoji/thumbsup.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> +Consult the [Emoji Cheat Sheet](https://www.webfx.com/tools/emoji-cheat-sheet/) for a list of all supported emoji codes. <img src="https://gitlab.com/gitlab-org/gitlab-foss/raw/master/public/-/emojis/2/thumbsup.png" width="20px" height="20px" style="display:inline;margin:0;border: 0"> #### Emojis and your operating system diff --git a/doc/user/project/import/perforce.md b/doc/user/project/import/perforce.md index f3843396b79..aa256e07b30 100644 --- a/doc/user/project/import/perforce.md +++ b/doc/user/project/import/perforce.md @@ -61,3 +61,7 @@ creating small and efficient Git pack files. So it might be a good idea to spend time and CPU to properly repack your repository before sending it for the first time to your GitLab server. See [this StackOverflow question](https://stackoverflow.com/questions/28720151/git-gc-aggressive-vs-git-repack/). + +## Related topics + +- [Mirror with Perforce Helix with Git Fusion](../repository/mirror/bidirectional.md#mirror-with-perforce-helix-with-git-fusion) diff --git a/doc/user/project/merge_requests/approvals/settings.md b/doc/user/project/merge_requests/approvals/settings.md index 3c67097e8c6..c5440c3358c 100644 --- a/doc/user/project/merge_requests/approvals/settings.md +++ b/doc/user/project/merge_requests/approvals/settings.md @@ -39,7 +39,7 @@ By default, the author of a merge request cannot approve it. To change this sett 1. Go to your project and select **Settings > General**. 1. Expand **Merge request (MR) approvals**. -1. Clear the **Prevent MR approval by the author** checkbox. +1. Clear the **Prevent approval by author** checkbox. 1. Select **Save changes**. Authors can edit the approval rule in an individual merge request and override @@ -64,7 +64,7 @@ their own. To do this: 1. Go to your project and select **Settings > General**. 1. Expand **Merge request (MR) approvals**. -1. Select the **Prevent MR approvals from users who make commits to the MR** checkbox. +1. Select the **Prevent approvals by users who add commits** checkbox. If this checkbox is cleared, an administrator has disabled it [at the instance level](../../../admin_area/merge_requests_approvals.md), and it can't be changed at the project level. @@ -84,7 +84,7 @@ on merge requests, you can disable this setting: 1. Go to your project and select **Settings > General**. 1. Expand **Merge request (MR) approvals**. -1. Select the **Prevent users from modifying MR approval rules in merge requests** checkbox. +1. Select the **Prevent editing approval rules in merge requests** checkbox. 1. Select **Save changes**. This change affects all open merge requests. @@ -102,7 +102,7 @@ permission enables an electronic signature for approvals, such as the one define [sign-in restrictions documentation](../../../admin_area/settings/sign_in_restrictions.md#password-authentication-enabled). 1. Go to your project and select **Settings > General**. 1. Expand **Merge request (MR) approvals**. -1. Select the **Require user password for approvals** checkbox. +1. Select the **Require user password to approve** checkbox. 1. Select **Save changes**. ## Remove all approvals when commits are added to the source branch @@ -113,7 +113,7 @@ when more changes are added to it: 1. Go to your project and select **Settings > General**. 1. Expand **Merge request (MR) approvals**. -1. Select the **Require new approvals when new commits are added to an MR** checkbox. +1. Select the **Remove all approvals when commits are added to the source branch** checkbox. 1. Select **Save changes**. Approvals aren't reset when a merge request is [rebased from the UI](../fast_forward_merge.md) diff --git a/doc/user/project/repository/mirror/bidirectional.md b/doc/user/project/repository/mirror/bidirectional.md index c4254655fcc..340d7b48a47 100644 --- a/doc/user/project/repository/mirror/bidirectional.md +++ b/doc/user/project/repository/mirror/bidirectional.md @@ -139,7 +139,7 @@ This sample has a few limitations: - The script circumvents the Git hook quarantine environment because the update of `$TARGET_REPO` is seen as a ref update, and Git displays warnings about it. -## Mirror with Perforce Helix via Git Fusion **(PREMIUM)** +## Mirror with Perforce Helix with Git Fusion **(PREMIUM)** > Moved to GitLab Premium in 13.9. diff --git a/lib/api/entities/ci/job_request/service.rb b/lib/api/entities/ci/job_request/service.rb index f89b95c1d5c..0dae5d5a933 100644 --- a/lib/api/entities/ci/job_request/service.rb +++ b/lib/api/entities/ci/job_request/service.rb @@ -6,6 +6,7 @@ module API module JobRequest class Service < Entities::Ci::JobRequest::Image expose :alias, :command + expose :variables end end end diff --git a/lib/api/repositories.rb b/lib/api/repositories.rb index 1aa76906b3d..c6cedb9f060 100644 --- a/lib/api/repositories.rb +++ b/lib/api/repositories.rb @@ -42,6 +42,26 @@ module API not_found! 'Blob' unless @blob end + + def fetch_target_project(current_user, user_project, params) + return user_project unless params[:from_project_id].present? + + MergeRequestTargetProjectFinder + .new(current_user: current_user, source_project: user_project, project_feature: :repository) + .execute(include_routes: true).find_by_id(params[:from_project_id]) + end + + def compare_cache_key(current_user, user_project, target_project, params) + [ + user_project, + target_project, + current_user, + :repository_compare, + target_project.repository.commit(params[:from]), + user_project.repository.commit(params[:to]), + params + ] + end end desc 'Get a project repository tree' do @@ -126,20 +146,15 @@ module API end get ':id/repository/compare' do ff_enabled = Feature.enabled?(:api_caching_rate_limit_repository_compare, user_project, default_enabled: :yaml) + target_project = fetch_target_project(current_user, user_project, params) - cache_action_if(ff_enabled, [user_project, :repository_compare, current_user, declared_params], expires_in: 1.minute) do - if params[:from_project_id].present? - target_project = MergeRequestTargetProjectFinder - .new(current_user: current_user, source_project: user_project, project_feature: :repository) - .execute(include_routes: true).find_by_id(params[:from_project_id]) + if target_project.blank? + render_api_error!("Target project id:#{params[:from_project_id]} is not a fork of project id:#{params[:id]}", 400) + end - if target_project.blank? - render_api_error!("Target project id:#{params[:from_project_id]} is not a fork of project id:#{params[:id]}", 400) - end - else - target_project = user_project - end + cache_key = compare_cache_key(current_user, user_project, target_project, declared_params) + cache_action_if(ff_enabled, cache_key, expires_in: 1.minute) do compare = CompareService.new(user_project, params[:to]).execute(target_project, params[:from], straight: params[:straight]) if compare diff --git a/lib/gitlab/ci/build/image.rb b/lib/gitlab/ci/build/image.rb index 1d7bfba75cd..8ddcf1d523e 100644 --- a/lib/gitlab/ci/build/image.rb +++ b/lib/gitlab/ci/build/image.rb @@ -4,7 +4,7 @@ module Gitlab module Ci module Build class Image - attr_reader :alias, :command, :entrypoint, :name, :ports + attr_reader :alias, :command, :entrypoint, :name, :ports, :variables class << self def from_image(job) @@ -33,6 +33,7 @@ module Gitlab @entrypoint = image[:entrypoint] @name = image[:name] @ports = build_ports(image).select(&:valid?) + @variables = build_variables(image) end end @@ -45,6 +46,12 @@ module Gitlab def build_ports(image) image[:ports].to_a.map { |port| ::Gitlab::Ci::Build::Port.new(port) } end + + def build_variables(image) + image[:variables].to_a.map do |key, value| + { key: key, value: value.to_s } + end + end end end end diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index f867189d521..75bbe2ccb1b 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -14,10 +14,10 @@ module Gitlab ALLOWED_KEYS = %i[tags script type image services start_in artifacts cache dependencies before_script after_script environment coverage retry parallel interruptible timeout - release dast_configuration secrets].freeze + release].freeze validations do - validates :config, allowed_keys: ALLOWED_KEYS + PROCESSABLE_ALLOWED_KEYS + validates :config, allowed_keys: Gitlab::Ci::Config::Entry::Job.allowed_keys + PROCESSABLE_ALLOWED_KEYS validates :script, presence: true with_options allow_nil: true do @@ -178,6 +178,10 @@ module Gitlab allow_failure_defined? ? static_allow_failure : manual_action? end + def self.allowed_keys + ALLOWED_KEYS + end + private def allow_failure_criteria diff --git a/lib/gitlab/ci/config/entry/service.rb b/lib/gitlab/ci/config/entry/service.rb index 247bf930d3b..f27dca4986e 100644 --- a/lib/gitlab/ci/config/entry/service.rb +++ b/lib/gitlab/ci/config/entry/service.rb @@ -15,7 +15,7 @@ module Gitlab include ::Gitlab::Config::Entry::Attributable include ::Gitlab::Config::Entry::Configurable - ALLOWED_KEYS = %i[name entrypoint command alias ports].freeze + ALLOWED_KEYS = %i[name entrypoint command alias ports variables].freeze validations do validates :config, hash_or_string: true @@ -32,6 +32,10 @@ module Gitlab entry :ports, Entry::Ports, description: 'Ports used to expose the service' + entry :variables, ::Gitlab::Ci::Config::Entry::Variables, + description: 'Environment variables available for this service.', + inherit: false + attributes :ports def alias diff --git a/lib/gitlab/sidekiq_logging/deduplication_logger.rb b/lib/gitlab/sidekiq_logging/deduplication_logger.rb index c5654819ffb..f4f6cb2a306 100644 --- a/lib/gitlab/sidekiq_logging/deduplication_logger.rb +++ b/lib/gitlab/sidekiq_logging/deduplication_logger.rb @@ -6,7 +6,7 @@ module Gitlab include Singleton include LogsJobs - def log(job, deduplication_type, deduplication_options = {}) + def deduplicated_log(job, deduplication_type, deduplication_options = {}) payload = parse_job(job) payload['job_status'] = 'deduplicated' payload['message'] = "#{base_message(payload)}: deduplicated: #{deduplication_type}" @@ -17,6 +17,14 @@ module Gitlab Sidekiq.logger.info payload end + + def rescheduled_log(job) + payload = parse_job(job) + payload['job_status'] = 'rescheduled' + payload['message'] = "#{base_message(payload)}: rescheduled" + + Sidekiq.logger.info payload + end end end end diff --git a/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb b/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb index e63164efc94..5b02efdbf6f 100644 --- a/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb +++ b/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job.rb @@ -24,6 +24,7 @@ module Gitlab MAX_REDIS_RETRIES = 5 DEFAULT_STRATEGY = :until_executing STRATEGY_NONE = :none + DEDUPLICATED_FLAG_VALUE = 1 LUA_SET_WAL_SCRIPT = <<~EOS local key, wal, offset, ttl = KEYS[1], ARGV[1], tonumber(ARGV[2]), ARGV[3] @@ -110,12 +111,18 @@ module Gitlab def delete! Sidekiq.redis do |redis| redis.multi do |multi| - multi.del(idempotency_key) + multi.del(idempotency_key, deduplicated_flag_key) delete_wal_locations!(multi) end end end + def reschedule + Gitlab::SidekiqLogging::DeduplicationLogger.instance.rescheduled_log(job) + + worker_klass.perform_async(*arguments) + end + def scheduled? scheduled_at.present? end @@ -126,6 +133,22 @@ module Gitlab jid != existing_jid end + def set_deduplicated_flag!(expiry = DUPLICATE_KEY_TTL) + return unless reschedulable? + + Sidekiq.redis do |redis| + redis.set(deduplicated_flag_key, DEDUPLICATED_FLAG_VALUE, ex: expiry, nx: true) + end + end + + def should_reschedule? + return false unless reschedulable? + + Sidekiq.redis do |redis| + redis.get(deduplicated_flag_key).present? + end + end + def scheduled_at job['at'] end @@ -216,6 +239,10 @@ module Gitlab @idempotency_key ||= job['idempotency_key'] || "#{namespace}:#{idempotency_hash}" end + def deduplicated_flag_key + "#{idempotency_key}:deduplicate_flag" + end + def idempotency_hash Digest::SHA256.hexdigest(idempotency_string) end @@ -235,6 +262,10 @@ module Gitlab def preserve_wal_location? Feature.enabled?(:preserve_latest_wal_locations_for_idempotent_jobs, default_enabled: :yaml) end + + def reschedulable? + !scheduled? && options[:if_deduplicated] == :reschedule_once + end end end end diff --git a/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/deduplicates_when_scheduling.rb b/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/deduplicates_when_scheduling.rb index b0da85b74a6..72f06b856d8 100644 --- a/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/deduplicates_when_scheduling.rb +++ b/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/deduplicates_when_scheduling.rb @@ -6,6 +6,7 @@ module Gitlab module Strategies class DeduplicatesWhenScheduling < Base extend ::Gitlab::Utils::Override + include ::Gitlab::Utils::StrongMemoize override :initialize def initialize(duplicate_job) @@ -19,8 +20,9 @@ module Gitlab if duplicate_job.idempotent? duplicate_job.update_latest_wal_location! + duplicate_job.set_deduplicated_flag!(expiry) - Gitlab::SidekiqLogging::DeduplicationLogger.instance.log( + Gitlab::SidekiqLogging::DeduplicationLogger.instance.deduplicated_log( job, "dropped #{strategy_name}", duplicate_job.options) return false end @@ -49,11 +51,13 @@ module Gitlab end def expiry - return DuplicateJob::DUPLICATE_KEY_TTL unless duplicate_job.scheduled? + strong_memoize(:expiry) do + next DuplicateJob::DUPLICATE_KEY_TTL unless duplicate_job.scheduled? - time_diff = duplicate_job.scheduled_at.to_i - Time.now.to_i + time_diff = duplicate_job.scheduled_at.to_i - Time.now.to_i - time_diff > 0 ? time_diff : DuplicateJob::DUPLICATE_KEY_TTL + time_diff > 0 ? time_diff : DuplicateJob::DUPLICATE_KEY_TTL + end end end end diff --git a/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed.rb b/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed.rb index 25f1b8b7c51..8c7e15364f8 100644 --- a/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed.rb +++ b/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed.rb @@ -14,7 +14,10 @@ module Gitlab yield + should_reschedule = duplicate_job.should_reschedule? + # Deleting before rescheduling to make sure we don't deduplicate again. duplicate_job.delete! + duplicate_job.reschedule if should_reschedule end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 23d729a63b7..6fd8eb053c2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -25249,6 +25249,9 @@ msgstr "" msgid "Pipelines|There are currently no pipelines." msgstr "" +msgid "Pipelines|There was a problem with loading the pipeline data." +msgstr "" + msgid "Pipelines|There was an error fetching the pipelines. Try again in a few moments or contact your support team." msgstr "" @@ -33154,6 +33157,9 @@ msgstr "" msgid "Survey Response" msgstr "" +msgid "Switch Branches" +msgstr "" + msgid "Switch branch" msgstr "" @@ -35924,7 +35930,7 @@ msgstr "" msgid "TransferGroup|Group is already associated to the parent group." msgstr "" -msgid "TransferGroup|The parent group already has a subgroup with the same path." +msgid "TransferGroup|The parent group already has a subgroup or a project with the same path." msgstr "" msgid "TransferGroup|Transfer failed: %{error_message}" @@ -36292,6 +36298,9 @@ msgstr "" msgid "Unable to fetch branches list, please close the form and try again" msgstr "" +msgid "Unable to fetch upstream and downstream pipelines." +msgstr "" + msgid "Unable to fetch vulnerable projects" msgstr "" @@ -36376,6 +36385,9 @@ msgstr "" msgid "Unauthenticated web rate limit period in seconds" msgstr "" +msgid "Uncommitted changes will be lost if you change branches. Do you want to continue?" +msgstr "" + msgid "Undo" msgstr "" @@ -39165,6 +39177,9 @@ msgstr "" msgid "You have successfully purchased a %{plan} plan subscription for %{seats}. You’ll receive a receipt via email." msgstr "" +msgid "You have unsaved changes" +msgstr "" + msgid "You left the \"%{membershipable_human_name}\" %{source_type}." msgstr "" diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 1108c606df3..a705cf26845 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -443,7 +443,7 @@ FactoryBot.define do options do { image: { name: 'ruby:2.7', entrypoint: '/bin/sh' }, - services: ['postgres', { name: 'docker:stable-dind', entrypoint: '/bin/sh', command: 'sleep 30', alias: 'docker' }], + services: ['postgres', { name: 'docker:stable-dind', entrypoint: '/bin/sh', command: 'sleep 30', alias: 'docker' }, { name: 'mysql:latest', variables: { MYSQL_ROOT_PASSWORD: 'root123.' } }], script: %w(echo), after_script: %w(ls date), artifacts: { diff --git a/spec/frontend/pipeline_editor/components/file-nav/branch_switcher_spec.js b/spec/frontend/pipeline_editor/components/file-nav/branch_switcher_spec.js index b5881790b0b..2f9f52c0304 100644 --- a/spec/frontend/pipeline_editor/components/file-nav/branch_switcher_spec.js +++ b/spec/frontend/pipeline_editor/components/file-nav/branch_switcher_spec.js @@ -36,8 +36,9 @@ describe('Pipeline editor branch switcher', () => { let mockLastCommitBranchQuery; const createComponent = ( - { currentBranch, isQueryLoading, mountFn, options } = { + { currentBranch, isQueryLoading, mountFn, options, props } = { currentBranch: mockDefaultBranch, + hasUnsavedChanges: false, isQueryLoading: false, mountFn: shallowMount, options: {}, @@ -45,6 +46,7 @@ describe('Pipeline editor branch switcher', () => { ) => { wrapper = mountFn(BranchSwitcher, { propsData: { + ...props, paginationLimit: mockBranchPaginationLimit, }, provide: { @@ -70,7 +72,7 @@ describe('Pipeline editor branch switcher', () => { }); }; - const createComponentWithApollo = (mountFn = shallowMount) => { + const createComponentWithApollo = ({ mountFn = shallowMount, props = {} } = {}) => { const handlers = [[getAvailableBranchesQuery, mockAvailableBranchQuery]]; const resolvers = { Query: { @@ -86,6 +88,7 @@ describe('Pipeline editor branch switcher', () => { createComponent({ mountFn, + props, options: { localVue, apolloProvider: mockApollo, @@ -149,7 +152,7 @@ describe('Pipeline editor branch switcher', () => { availableBranches: mockProjectBranches, currentBranch: mockDefaultBranch, }); - createComponentWithApollo(mount); + createComponentWithApollo({ mountFn: mount }); await waitForPromises(); }); @@ -201,7 +204,7 @@ describe('Pipeline editor branch switcher', () => { availableBranches: mockProjectBranches, currentBranch: mockDefaultBranch, }); - createComponentWithApollo(mount); + createComponentWithApollo({ mountFn: mount }); await waitForPromises(); }); @@ -247,6 +250,23 @@ describe('Pipeline editor branch switcher', () => { expect(wrapper.emitted('refetchContent')).toBeUndefined(); }); + + describe('with unsaved changes', () => { + beforeEach(async () => { + createComponentWithApollo({ mountFn: mount, props: { hasUnsavedChanges: true } }); + await waitForPromises(); + }); + + it('emits `select-branch` event and does not switch branch', async () => { + expect(wrapper.emitted('select-branch')).toBeUndefined(); + + const branch = findDropdownItems().at(1); + await branch.vm.$emit('click'); + + expect(wrapper.emitted('select-branch')).toEqual([[branch.text()]]); + expect(wrapper.emitted('refetchContent')).toBeUndefined(); + }); + }); }); describe('when searching', () => { @@ -255,7 +275,7 @@ describe('Pipeline editor branch switcher', () => { availableBranches: mockProjectBranches, currentBranch: mockDefaultBranch, }); - createComponentWithApollo(mount); + createComponentWithApollo({ mountFn: mount }); await waitForPromises(); }); @@ -429,7 +449,7 @@ describe('Pipeline editor branch switcher', () => { availableBranches: mockProjectBranches, currentBranch: mockDefaultBranch, }); - createComponentWithApollo(mount); + createComponentWithApollo({ mountFn: mount }); await waitForPromises(); await createNewBranch(); }); diff --git a/spec/frontend/pipeline_editor/components/header/pipline_editor_mini_graph_spec.js b/spec/frontend/pipeline_editor/components/header/pipline_editor_mini_graph_spec.js index 3d7c3c839da..a09c3d0e315 100644 --- a/spec/frontend/pipeline_editor/components/header/pipline_editor_mini_graph_spec.js +++ b/spec/frontend/pipeline_editor/components/header/pipline_editor_mini_graph_spec.js @@ -1,22 +1,58 @@ -import { shallowMount } from '@vue/test-utils'; +import { shallowMount, createLocalVue } from '@vue/test-utils'; +import VueApollo from 'vue-apollo'; +import waitForPromises from 'helpers/wait_for_promises'; +import createMockApollo from 'helpers/mock_apollo_helper'; import PipelineEditorMiniGraph from '~/pipeline_editor/components/header/pipeline_editor_mini_graph.vue'; import PipelineMiniGraph from '~/pipelines/components/pipelines_list/pipeline_mini_graph.vue'; -import { mockProjectPipeline } from '../../mock_data'; +import getLinkedPipelinesQuery from '~/projects/commit_box/info/graphql/queries/get_linked_pipelines.query.graphql'; +import { PIPELINE_FAILURE } from '~/pipeline_editor/constants'; +import { mockLinkedPipelines, mockProjectFullPath, mockProjectPipeline } from '../../mock_data'; + +const localVue = createLocalVue(); +localVue.use(VueApollo); describe('Pipeline Status', () => { let wrapper; + let mockApollo; + let mockLinkedPipelinesQuery; - const createComponent = ({ hasStages = true } = {}) => { + const createComponent = ({ hasStages = true, options } = {}) => { wrapper = shallowMount(PipelineEditorMiniGraph, { + provide: { + dataMethod: 'graphql', + projectFullPath: mockProjectFullPath, + }, propsData: { pipeline: mockProjectPipeline({ hasStages }).pipeline, }, + ...options, + }); + }; + + const createComponentWithApollo = (hasStages = true) => { + const handlers = [[getLinkedPipelinesQuery, mockLinkedPipelinesQuery]]; + mockApollo = createMockApollo(handlers); + + createComponent({ + hasStages, + options: { + localVue, + apolloProvider: mockApollo, + }, }); }; const findPipelineMiniGraph = () => wrapper.findComponent(PipelineMiniGraph); + const findUpstream = () => wrapper.find('[data-testid="pipeline-editor-mini-graph-upstream"]'); + const findDownstream = () => + wrapper.find('[data-testid="pipeline-editor-mini-graph-downstream"]'); + + beforeEach(() => { + mockLinkedPipelinesQuery = jest.fn(); + }); afterEach(() => { + mockLinkedPipelinesQuery.mockReset(); wrapper.destroy(); }); @@ -39,4 +75,60 @@ describe('Pipeline Status', () => { expect(findPipelineMiniGraph().exists()).toBe(false); }); }); + + describe('when querying upstream and downstream pipelines', () => { + describe('when query succeeds', () => { + beforeEach(() => { + mockLinkedPipelinesQuery.mockResolvedValue(mockLinkedPipelines()); + createComponentWithApollo(); + }); + + it('should call the query with the correct variables', () => { + expect(mockLinkedPipelinesQuery).toHaveBeenCalledTimes(1); + expect(mockLinkedPipelinesQuery).toHaveBeenCalledWith({ + fullPath: mockProjectFullPath, + iid: mockProjectPipeline().pipeline.iid, + }); + }); + + describe('linked pipeline rendering based on given data', () => { + it.each` + hasDownstream | hasUpstream | downstreamRenderAction | upstreamRenderAction + ${true} | ${true} | ${'renders'} | ${'renders'} + ${true} | ${false} | ${'renders'} | ${'hides'} + ${false} | ${true} | ${'hides'} | ${'renders'} + ${false} | ${false} | ${'hides'} | ${'hides'} + `( + '$downstreamRenderAction downstream and $upstreamRenderAction upstream', + async ({ hasDownstream, hasUpstream }) => { + mockLinkedPipelinesQuery.mockResolvedValue( + mockLinkedPipelines({ hasDownstream, hasUpstream }), + ); + createComponentWithApollo(); + await waitForPromises(); + + expect(findUpstream().exists()).toBe(hasUpstream); + expect(findDownstream().exists()).toBe(hasDownstream); + }, + ); + }); + }); + + describe('when query fails', () => { + beforeEach(() => { + mockLinkedPipelinesQuery.mockRejectedValue(new Error()); + createComponentWithApollo(); + }); + + it('should emit an error event when query fails', async () => { + expect(wrapper.emitted('showError')).toHaveLength(1); + expect(wrapper.emitted('showError')[0]).toEqual([ + { + type: PIPELINE_FAILURE, + reasons: [wrapper.vm.$options.i18n.linkedPipelinesFetchError], + }, + ]); + }); + }); + }); }); diff --git a/spec/frontend/pipeline_editor/components/ui/pipeline_editor_messages_spec.js b/spec/frontend/pipeline_editor/components/ui/pipeline_editor_messages_spec.js index 9f910ed4f9c..a55176ccd79 100644 --- a/spec/frontend/pipeline_editor/components/ui/pipeline_editor_messages_spec.js +++ b/spec/frontend/pipeline_editor/components/ui/pipeline_editor_messages_spec.js @@ -11,6 +11,7 @@ import { DEFAULT_FAILURE, DEFAULT_SUCCESS, LOAD_FAILURE_UNKNOWN, + PIPELINE_FAILURE, } from '~/pipeline_editor/constants'; beforeEach(() => { @@ -65,6 +66,7 @@ describe('Pipeline Editor messages', () => { failureType | message | expectedFailureType ${COMMIT_FAILURE} | ${'failed commit'} | ${COMMIT_FAILURE} ${LOAD_FAILURE_UNKNOWN} | ${'loading failure'} | ${LOAD_FAILURE_UNKNOWN} + ${PIPELINE_FAILURE} | ${'pipeline failure'} | ${PIPELINE_FAILURE} ${'random'} | ${'error without a specified type'} | ${DEFAULT_FAILURE} `('shows a message for $message', ({ failureType, expectedFailureType }) => { createComponent({ failureType, showFailure: true }); diff --git a/spec/frontend/pipeline_editor/mock_data.js b/spec/frontend/pipeline_editor/mock_data.js index 0b0ff14486e..9ad604a63ee 100644 --- a/spec/frontend/pipeline_editor/mock_data.js +++ b/spec/frontend/pipeline_editor/mock_data.js @@ -290,6 +290,62 @@ export const mockProjectPipeline = ({ hasStages = true } = {}) => { }; }; +export const mockLinkedPipelines = ({ hasDownstream = true, hasUpstream = true } = {}) => { + let upstream = null; + let downstream = { + nodes: [], + __typename: 'PipelineConnection', + }; + + if (hasDownstream) { + downstream = { + nodes: [ + { + id: 'gid://gitlab/Ci::Pipeline/612', + path: '/root/job-log-sections/-/pipelines/612', + project: { name: 'job-log-sections', __typename: 'Project' }, + detailedStatus: { + group: 'success', + icon: 'status_success', + label: 'passed', + __typename: 'DetailedStatus', + }, + __typename: 'Pipeline', + }, + ], + __typename: 'PipelineConnection', + }; + } + + if (hasUpstream) { + upstream = { + id: 'gid://gitlab/Ci::Pipeline/610', + path: '/root/trigger-downstream/-/pipelines/610', + project: { name: 'trigger-downstream', __typename: 'Project' }, + detailedStatus: { + group: 'success', + icon: 'status_success', + label: 'passed', + __typename: 'DetailedStatus', + }, + __typename: 'Pipeline', + }; + } + + return { + data: { + project: { + pipeline: { + path: '/root/ci-project/-/pipelines/790', + downstream, + upstream, + }, + __typename: 'Project', + }, + }, + }; +}; + export const mockLintResponse = { valid: true, mergedYaml: mockCiYml, diff --git a/spec/frontend/pipeline_editor/pipeline_editor_home_spec.js b/spec/frontend/pipeline_editor/pipeline_editor_home_spec.js index 4234e4d6a9b..11e6aadad5f 100644 --- a/spec/frontend/pipeline_editor/pipeline_editor_home_spec.js +++ b/spec/frontend/pipeline_editor/pipeline_editor_home_spec.js @@ -1,9 +1,10 @@ import { shallowMount } from '@vue/test-utils'; import { nextTick } from 'vue'; - +import { GlModal } from '@gitlab/ui'; import CommitSection from '~/pipeline_editor/components/commit/commit_section.vue'; import PipelineEditorDrawer from '~/pipeline_editor/components/drawer/pipeline_editor_drawer.vue'; import PipelineEditorFileNav from '~/pipeline_editor/components/file_nav/pipeline_editor_file_nav.vue'; +import BranchSwitcher from '~/pipeline_editor/components/file_nav/branch_switcher.vue'; import PipelineEditorHeader from '~/pipeline_editor/components/header/pipeline_editor_header.vue'; import PipelineEditorTabs from '~/pipeline_editor/components/pipeline_editor_tabs.vue'; import { MERGED_TAB, VISUALIZE_TAB, CREATE_TAB, LINT_TAB } from '~/pipeline_editor/constants'; @@ -11,11 +12,14 @@ import PipelineEditorHome from '~/pipeline_editor/pipeline_editor_home.vue'; import { mockLintResponse, mockCiYml } from './mock_data'; +jest.mock('~/lib/utils/common_utils'); + describe('Pipeline editor home wrapper', () => { let wrapper; - const createComponent = ({ props = {}, glFeatures = {} } = {}) => { + const createComponent = ({ props = {}, glFeatures = {}, data = {}, stubs = {} } = {}) => { wrapper = shallowMount(PipelineEditorHome, { + data: () => data, propsData: { ciConfigData: mockLintResponse, ciFileContent: mockCiYml, @@ -24,15 +28,20 @@ describe('Pipeline editor home wrapper', () => { ...props, }, provide: { + projectFullPath: '', + totalBranches: 19, glFeatures: { ...glFeatures, }, }, + stubs, }); }; + const findBranchSwitcher = () => wrapper.findComponent(BranchSwitcher); const findCommitSection = () => wrapper.findComponent(CommitSection); const findFileNav = () => wrapper.findComponent(PipelineEditorFileNav); + const findModal = () => wrapper.findComponent(GlModal); const findPipelineEditorDrawer = () => wrapper.findComponent(PipelineEditorDrawer); const findPipelineEditorHeader = () => wrapper.findComponent(PipelineEditorHeader); const findPipelineEditorTabs = () => wrapper.findComponent(PipelineEditorTabs); @@ -67,6 +76,46 @@ describe('Pipeline editor home wrapper', () => { }); }); + describe('modal when switching branch', () => { + describe('when `showSwitchBranchModal` value is false', () => { + beforeEach(() => { + createComponent(); + }); + + it('is not visible', () => { + expect(findModal().exists()).toBe(false); + }); + }); + describe('when `showSwitchBranchModal` value is true', () => { + beforeEach(() => { + createComponent({ + data: { showSwitchBranchModal: true }, + stubs: { PipelineEditorFileNav }, + }); + }); + + it('is visible', () => { + expect(findModal().exists()).toBe(true); + }); + + it('pass down `shouldLoadNewBranch` to the branch switcher when primary is selected', async () => { + expect(findBranchSwitcher().props('shouldLoadNewBranch')).toBe(false); + + await findModal().vm.$emit('primary'); + + expect(findBranchSwitcher().props('shouldLoadNewBranch')).toBe(true); + }); + + it('closes the modal when secondary action is selected', async () => { + expect(findModal().exists()).toBe(true); + + await findModal().vm.$emit('secondary'); + + expect(findModal().exists()).toBe(false); + }); + }); + }); + describe('commit form toggle', () => { beforeEach(() => { createComponent(); diff --git a/spec/graphql/mutations/customer_relations/contacts/create_spec.rb b/spec/graphql/mutations/customer_relations/contacts/create_spec.rb index 21a1aa2741a..3fd4a57acd6 100644 --- a/spec/graphql/mutations/customer_relations/contacts/create_spec.rb +++ b/spec/graphql/mutations/customer_relations/contacts/create_spec.rb @@ -29,7 +29,7 @@ RSpec.describe Mutations::CustomerRelations::Contacts::Create do it 'raises an error' do expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) - .with_message("The resource that you are attempting to access does not exist or you don't have permission to perform this action") + .with_message(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) end end diff --git a/spec/graphql/mutations/customer_relations/contacts/update_spec.rb b/spec/graphql/mutations/customer_relations/contacts/update_spec.rb index 93bc6f53cf9..00e71aeb7c6 100644 --- a/spec/graphql/mutations/customer_relations/contacts/update_spec.rb +++ b/spec/graphql/mutations/customer_relations/contacts/update_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Mutations::CustomerRelations::Contacts::Update do let(:last_name) { 'Smith' } let(:email) { 'ls@gitlab.com' } let(:description) { 'VIP' } - let(:does_not_exist_or_no_permission) { "The resource that you are attempting to access does not exist or you don't have permission to perform this action" } + let(:does_not_exist_or_no_permission) { Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR } let(:contact) { create(:contact, group: group) } let(:attributes) do { diff --git a/spec/graphql/mutations/customer_relations/organizations/create_spec.rb b/spec/graphql/mutations/customer_relations/organizations/create_spec.rb index 738a8d724ab..33f78958bb9 100644 --- a/spec/graphql/mutations/customer_relations/organizations/create_spec.rb +++ b/spec/graphql/mutations/customer_relations/organizations/create_spec.rb @@ -30,7 +30,7 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Create do it 'raises an error' do expect { resolve_mutation }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) - .with_message("The resource that you are attempting to access does not exist or you don't have permission to perform this action") + .with_message(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) end end diff --git a/spec/graphql/mutations/customer_relations/organizations/update_spec.rb b/spec/graphql/mutations/customer_relations/organizations/update_spec.rb index 0bc6f184fe3..5bfa009252d 100644 --- a/spec/graphql/mutations/customer_relations/organizations/update_spec.rb +++ b/spec/graphql/mutations/customer_relations/organizations/update_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Mutations::CustomerRelations::Organizations::Update do let(:name) { 'GitLab' } let(:default_rate) { 1000.to_f } let(:description) { 'VIP' } - let(:does_not_exist_or_no_permission) { "The resource that you are attempting to access does not exist or you don't have permission to perform this action" } + let(:does_not_exist_or_no_permission) { Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR } let(:organization) { create(:organization, group: group) } let(:attributes) do { diff --git a/spec/graphql/mutations/discussions/toggle_resolve_spec.rb b/spec/graphql/mutations/discussions/toggle_resolve_spec.rb index b03c6cb094f..1ec38e3bc0a 100644 --- a/spec/graphql/mutations/discussions/toggle_resolve_spec.rb +++ b/spec/graphql/mutations/discussions/toggle_resolve_spec.rb @@ -26,7 +26,7 @@ RSpec.describe Mutations::Discussions::ToggleResolve do it 'raises an error if the resource is not accessible to the user' do expect { subject }.to raise_error( Gitlab::Graphql::Errors::ResourceNotAvailable, - "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR ) end end @@ -40,7 +40,7 @@ RSpec.describe Mutations::Discussions::ToggleResolve do it 'raises an error' do expect { subject }.to raise_error( Gitlab::Graphql::Errors::ResourceNotAvailable, - "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR ) end end diff --git a/spec/graphql/mutations/environments/canary_ingress/update_spec.rb b/spec/graphql/mutations/environments/canary_ingress/update_spec.rb index 2715a908f85..48e55828a6b 100644 --- a/spec/graphql/mutations/environments/canary_ingress/update_spec.rb +++ b/spec/graphql/mutations/environments/canary_ingress/update_spec.rb @@ -60,7 +60,7 @@ RSpec.describe Mutations::Environments::CanaryIngress::Update do let(:user) { reporter } it 'raises an error' do - expect { subject }.to raise_error("The resource that you are attempting to access does not exist or you don't have permission to perform this action") + expect { subject }.to raise_error(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) end end end diff --git a/spec/graphql/mutations/notes/reposition_image_diff_note_spec.rb b/spec/graphql/mutations/notes/reposition_image_diff_note_spec.rb index e78f755d5c7..39794a070c6 100644 --- a/spec/graphql/mutations/notes/reposition_image_diff_note_spec.rb +++ b/spec/graphql/mutations/notes/reposition_image_diff_note_spec.rb @@ -29,7 +29,7 @@ RSpec.describe Mutations::Notes::RepositionImageDiffNote do it 'raises an error if the resource is not accessible to the user' do expect { subject }.to raise_error( Gitlab::Graphql::Errors::ResourceNotAvailable, - "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR ) end end diff --git a/spec/graphql/mutations/releases/delete_spec.rb b/spec/graphql/mutations/releases/delete_spec.rb index d97f839ce87..9934aea0031 100644 --- a/spec/graphql/mutations/releases/delete_spec.rb +++ b/spec/graphql/mutations/releases/delete_spec.rb @@ -28,7 +28,7 @@ RSpec.describe Mutations::Releases::Delete do shared_examples 'unauthorized or not found error' do it 'raises a Gitlab::Graphql::Errors::ResourceNotAvailable error' do - expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable, "The resource that you are attempting to access does not exist or you don't have permission to perform this action") + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable, Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) end end diff --git a/spec/graphql/mutations/releases/update_spec.rb b/spec/graphql/mutations/releases/update_spec.rb index 5ee63ac4dc2..9fae703b85a 100644 --- a/spec/graphql/mutations/releases/update_spec.rb +++ b/spec/graphql/mutations/releases/update_spec.rb @@ -232,7 +232,7 @@ RSpec.describe Mutations::Releases::Update do let(:mutation_arguments) { super().merge(project_path: 'not/a/real/path') } it 'raises an error' do - expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable, "The resource that you are attempting to access does not exist or you don't have permission to perform this action") + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable, Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) end end end @@ -242,7 +242,7 @@ RSpec.describe Mutations::Releases::Update do let(:current_user) { reporter } it 'raises an error' do - expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable, "The resource that you are attempting to access does not exist or you don't have permission to perform this action") + expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable, Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) end end end diff --git a/spec/lib/gitlab/sidekiq_logging/deduplication_logger_spec.rb b/spec/lib/gitlab/sidekiq_logging/deduplication_logger_spec.rb index 82f927fe481..f44a1e8b6ba 100644 --- a/spec/lib/gitlab/sidekiq_logging/deduplication_logger_spec.rb +++ b/spec/lib/gitlab/sidekiq_logging/deduplication_logger_spec.rb @@ -23,11 +23,37 @@ RSpec.describe Gitlab::SidekiqLogging::DeduplicationLogger do } expect(Sidekiq.logger).to receive(:info).with(a_hash_including(expected_payload)).and_call_original - described_class.instance.log(job, "a fancy strategy", { foo: :bar }) + described_class.instance.deduplicated_log(job, "a fancy strategy", { foo: :bar }) end it "does not modify the job" do - expect { described_class.instance.log(job, "a fancy strategy") } + expect { described_class.instance.deduplicated_log(job, "a fancy strategy") } + .not_to change { job } + end + end + + describe '#rescheduled_log' do + let(:job) do + { + 'class' => 'TestWorker', + 'args' => [1234, 'hello', { 'key' => 'value' }], + 'jid' => 'da883554ee4fe414012f5f42', + 'correlation_id' => 'cid' + } + end + + it 'logs a rescheduled message to the sidekiq logger' do + expected_payload = { + 'job_status' => 'rescheduled', + 'message' => "#{job['class']} JID-#{job['jid']}: rescheduled" + } + expect(Sidekiq.logger).to receive(:info).with(a_hash_including(expected_payload)).and_call_original + + described_class.instance.rescheduled_log(job) + end + + it 'does not modify the job' do + expect { described_class.instance.rescheduled_log(job) } .not_to change { job } end end diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb index 5083ac514db..f1744d245ab 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb @@ -24,6 +24,10 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi "#{Gitlab::Redis::Queues::SIDEKIQ_NAMESPACE}:duplicate:#{queue}:#{hash}" end + let(:deduplicated_flag_key) do + "#{idempotency_key}:deduplicate_flag" + end + describe '#schedule' do shared_examples 'scheduling with deduplication class' do |strategy_class| it 'calls schedule on the strategy' do @@ -270,6 +274,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi context 'when the key exists in redis' do before do set_idempotency_key(idempotency_key, 'existing-jid') + set_idempotency_key(deduplicated_flag_key, 1) wal_locations.each do |config_name, location| set_idempotency_key(existing_wal_location_key(idempotency_key, config_name), location) set_idempotency_key(wal_location_key(idempotency_key, config_name), location) @@ -299,6 +304,11 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi let(:from_value) { 'existing-jid' } end + it_behaves_like 'deleting keys from redis', 'deduplication counter key' do + let(:key) { deduplicated_flag_key } + let(:from_value) { '1' } + end + it_behaves_like 'deleting keys from redis', 'existing wal location keys for main database' do let(:key) { existing_wal_location_key(idempotency_key, :main) } let(:from_value) { wal_locations[:main] } @@ -390,6 +400,103 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi end end + describe '#reschedule' do + it 'reschedules the current job' do + fake_logger = instance_double(Gitlab::SidekiqLogging::DeduplicationLogger) + expect(Gitlab::SidekiqLogging::DeduplicationLogger).to receive(:instance).and_return(fake_logger) + expect(fake_logger).to receive(:rescheduled_log).with(a_hash_including({ 'jid' => '123' })) + expect(AuthorizedProjectsWorker).to receive(:perform_async).with(1).once + + duplicate_job.reschedule + end + end + + describe '#should_reschedule?' do + subject { duplicate_job.should_reschedule? } + + context 'when the job is reschedulable' do + before do + allow(duplicate_job).to receive(:reschedulable?) { true } + end + + it { is_expected.to eq(false) } + + context 'with deduplicated flag' do + before do + duplicate_job.set_deduplicated_flag! + end + + it { is_expected.to eq(true) } + end + end + + context 'when the job is not reschedulable' do + before do + allow(duplicate_job).to receive(:reschedulable?) { false } + end + + it { is_expected.to eq(false) } + + context 'with deduplicated flag' do + before do + duplicate_job.set_deduplicated_flag! + end + + it { is_expected.to eq(false) } + end + end + end + + describe '#set_deduplicated_flag!' do + context 'when the job is reschedulable' do + before do + allow(duplicate_job).to receive(:reschedulable?) { true } + end + + it 'sets the key in Redis' do + duplicate_job.set_deduplicated_flag! + + flag = Sidekiq.redis { |redis| redis.get(deduplicated_flag_key) } + + expect(flag).to eq(described_class::DEDUPLICATED_FLAG_VALUE.to_s) + end + + it 'sets, gets and cleans up the deduplicated flag' do + expect(duplicate_job.should_reschedule?).to eq(false) + + duplicate_job.set_deduplicated_flag! + expect(duplicate_job.should_reschedule?).to eq(true) + + duplicate_job.delete! + expect(duplicate_job.should_reschedule?).to eq(false) + end + end + + context 'when the job is not reschedulable' do + before do + allow(duplicate_job).to receive(:reschedulable?) { false } + end + + it 'does not set the key in Redis' do + duplicate_job.set_deduplicated_flag! + + flag = Sidekiq.redis { |redis| redis.get(deduplicated_flag_key) } + + expect(flag).to be_nil + end + + it 'does not set the deduplicated flag' do + expect(duplicate_job.should_reschedule?).to eq(false) + + duplicate_job.set_deduplicated_flag! + expect(duplicate_job.should_reschedule?).to eq(false) + + duplicate_job.delete! + expect(duplicate_job.should_reschedule?).to eq(false) + end + end + end + describe '#duplicate?' do it "raises an error if the check wasn't performed" do expect { duplicate_job.duplicate? }.to raise_error /Call `#check!` first/ diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed_spec.rb index 9772255fc50..963301bc001 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed_spec.rb @@ -9,6 +9,9 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecut before do allow(fake_duplicate_job).to receive(:latest_wal_locations).and_return( {} ) + allow(fake_duplicate_job).to receive(:scheduled?) { false } + allow(fake_duplicate_job).to receive(:options) { {} } + allow(fake_duplicate_job).to receive(:should_reschedule?) { false } end it 'deletes the lock after executing' do @@ -19,6 +22,28 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecut proc.call end end + + it 'does not reschedule the job even if deduplication happened' do + expect(fake_duplicate_job).to receive(:delete!) + expect(fake_duplicate_job).not_to receive(:reschedule) + + strategy.perform({}) do + proc.call + end + end + + context 'when job is reschedulable' do + it 'reschedules the job if deduplication happened' do + allow(fake_duplicate_job).to receive(:should_reschedule?) { true } + + expect(fake_duplicate_job).to receive(:delete!) + expect(fake_duplicate_job).to receive(:reschedule).once + + strategy.perform({}) do + proc.call + end + end + end end end end diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index c3fbef9be48..fdf1a278d4c 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -218,9 +218,11 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(json_response['git_info']).to eq(expected_git_info) expect(json_response['image']).to eq({ 'name' => 'ruby:2.7', 'entrypoint' => '/bin/sh', 'ports' => [] }) expect(json_response['services']).to eq([{ 'name' => 'postgres', 'entrypoint' => nil, - 'alias' => nil, 'command' => nil, 'ports' => [] }, + 'alias' => nil, 'command' => nil, 'ports' => [], 'variables' => nil }, { 'name' => 'docker:stable-dind', 'entrypoint' => '/bin/sh', - 'alias' => 'docker', 'command' => 'sleep 30', 'ports' => [] }]) + 'alias' => 'docker', 'command' => 'sleep 30', 'ports' => [], 'variables' => [] }, + { 'name' => 'mysql:latest', 'entrypoint' => nil, + 'alias' => nil, 'command' => nil, 'ports' => [], 'variables' => [{ 'key' => 'MYSQL_ROOT_PASSWORD', 'value' => 'root123.' }] }]) expect(json_response['steps']).to eq(expected_steps) expect(json_response['artifacts']).to eq(expected_artifacts) expect(json_response['cache']).to eq(expected_cache) diff --git a/spec/requests/api/graphql/mutations/ci/runners_registration_token/reset_spec.rb b/spec/requests/api/graphql/mutations/ci/runners_registration_token/reset_spec.rb index 0fd8fdc3f59..322706be119 100644 --- a/spec/requests/api/graphql/mutations/ci/runners_registration_token/reset_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/runners_registration_token/reset_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'RunnersRegistrationTokenReset' do subject expect(graphql_errors).not_to be_empty - expect(graphql_errors).to include(a_hash_including('message' => "The resource that you are attempting to access does not exist or you don't have permission to perform this action")) + expect(graphql_errors).to include(a_hash_including('message' => Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR)) expect(mutation_response).to be_nil end end diff --git a/spec/requests/api/graphql/mutations/issues/move_spec.rb b/spec/requests/api/graphql/mutations/issues/move_spec.rb index 5bbaff61edd..20ed16879f6 100644 --- a/spec/requests/api/graphql/mutations/issues/move_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/move_spec.rb @@ -33,7 +33,7 @@ RSpec.describe 'Moving an issue' do context 'when the user is not allowed to read source project' do it 'returns an error' do - error = "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + error = Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR post_graphql_mutation(mutation, current_user: user) expect(response).to have_gitlab_http_status(:success) diff --git a/spec/requests/api/graphql/mutations/issues/set_confidential_spec.rb b/spec/requests/api/graphql/mutations/issues/set_confidential_spec.rb index 3f804a46992..12ab504da14 100644 --- a/spec/requests/api/graphql/mutations/issues/set_confidential_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/set_confidential_spec.rb @@ -36,7 +36,7 @@ RSpec.describe 'Setting an issue as confidential' do end it 'returns an error if the user is not allowed to update the issue' do - error = "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + error = Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR post_graphql_mutation(mutation, current_user: create(:user)) expect(graphql_errors).to include(a_hash_including('message' => error)) diff --git a/spec/requests/api/graphql/mutations/issues/set_due_date_spec.rb b/spec/requests/api/graphql/mutations/issues/set_due_date_spec.rb index 72e47a98373..8e223b6fdaf 100644 --- a/spec/requests/api/graphql/mutations/issues/set_due_date_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/set_due_date_spec.rb @@ -36,7 +36,7 @@ RSpec.describe 'Setting Due Date of an issue' do end it 'returns an error if the user is not allowed to update the issue' do - error = "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + error = Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR post_graphql_mutation(mutation, current_user: create(:user)) expect(graphql_errors).to include(a_hash_including('message' => error)) diff --git a/spec/requests/api/graphql/mutations/issues/set_severity_spec.rb b/spec/requests/api/graphql/mutations/issues/set_severity_spec.rb index 41997f151a2..cd9d695bd2c 100644 --- a/spec/requests/api/graphql/mutations/issues/set_severity_spec.rb +++ b/spec/requests/api/graphql/mutations/issues/set_severity_spec.rb @@ -35,7 +35,7 @@ RSpec.describe 'Setting severity level of an incident' do context 'when the user is not allowed to update the incident' do it 'returns an error' do - error = "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + error = Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR post_graphql_mutation(mutation, current_user: user) expect(response).to have_gitlab_http_status(:success) diff --git a/spec/requests/api/graphql/mutations/releases/create_spec.rb b/spec/requests/api/graphql/mutations/releases/create_spec.rb index a4918cd560c..86995c10f10 100644 --- a/spec/requests/api/graphql/mutations/releases/create_spec.rb +++ b/spec/requests/api/graphql/mutations/releases/create_spec.rb @@ -342,7 +342,7 @@ RSpec.describe 'Creation of a new release' do end context "when the current user doesn't have access to create releases" do - expected_error_message = "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + expected_error_message = Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR context 'when the current user is a Reporter' do let(:current_user) { reporter } diff --git a/spec/requests/api/graphql/mutations/releases/delete_spec.rb b/spec/requests/api/graphql/mutations/releases/delete_spec.rb index 40063156609..eb4f0b594ea 100644 --- a/spec/requests/api/graphql/mutations/releases/delete_spec.rb +++ b/spec/requests/api/graphql/mutations/releases/delete_spec.rb @@ -50,7 +50,7 @@ RSpec.describe 'Deleting a release' do expect(mutation_response).to be_nil expect(graphql_errors.count).to eq(1) - expect(graphql_errors.first['message']).to eq("The resource that you are attempting to access does not exist or you don't have permission to perform this action") + expect(graphql_errors.first['message']).to eq(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) end end diff --git a/spec/requests/api/graphql/mutations/releases/update_spec.rb b/spec/requests/api/graphql/mutations/releases/update_spec.rb index c9a6c3abd57..0fa3d7de299 100644 --- a/spec/requests/api/graphql/mutations/releases/update_spec.rb +++ b/spec/requests/api/graphql/mutations/releases/update_spec.rb @@ -218,13 +218,13 @@ RSpec.describe 'Updating an existing release' do context 'when the project does not exist' do let(:mutation_arguments) { super().merge(projectPath: 'not/a/real/path') } - it_behaves_like 'top-level error with message', "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + it_behaves_like 'top-level error with message', Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR end end end context "when the current user doesn't have access to update releases" do - expected_error_message = "The resource that you are attempting to access does not exist or you don't have permission to perform this action" + expected_error_message = Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR context 'when the current user is a Reporter' do let(:current_user) { reporter } diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index f05f125c974..35e8ff2eeee 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -495,6 +495,43 @@ RSpec.describe API::Repositories do expect(response).to have_gitlab_http_status(:not_found) end + + it "returns a newly created commit", :use_clean_rails_redis_caching do + # Parse the commits ourselves because json_response is cached + def commit_messages(response) + Gitlab::Json.parse(response.body)["commits"].map do |commit| + commit["message"] + end + end + + # First trigger the rate limit cache + get api(route, current_user), params: { from: 'master', to: 'feature' } + + expect(response).to have_gitlab_http_status(:ok) + expect(commit_messages(response)).not_to include("Cool new commit") + + # Then create a new commit via the API + post api("/projects/#{project.id}/repository/commits", user), params: { + branch: "feature", + commit_message: "Cool new commit", + actions: [ + { + action: "create", + file_path: "foo/bar/baz.txt", + content: "puts 8" + } + ] + } + + expect(response).to have_gitlab_http_status(:created) + + # Now perform the same query as before, but the cache should have expired + # and our new commit should exist + get api(route, current_user), params: { from: 'master', to: 'feature' } + + expect(response).to have_gitlab_http_status(:ok) + expect(commit_messages(response)).to include("Cool new commit") + end end context 'when unauthenticated', 'and project is public' do diff --git a/spec/services/groups/transfer_service_spec.rb b/spec/services/groups/transfer_service_spec.rb index ee38c0fbb44..a75c5bff912 100644 --- a/spec/services/groups/transfer_service_spec.rb +++ b/spec/services/groups/transfer_service_spec.rb @@ -153,7 +153,7 @@ RSpec.describe Groups::TransferService do it 'adds an error on group' do transfer_service.execute(nil) - expect(transfer_service.error).to eq('Transfer failed: The parent group already has a subgroup with the same path.') + expect(transfer_service.error).to eq('Transfer failed: The parent group already has a subgroup or a project with the same path.') end end @@ -241,7 +241,7 @@ RSpec.describe Groups::TransferService do it 'adds an error on group' do transfer_service.execute(new_parent_group) - expect(transfer_service.error).to eq('Transfer failed: The parent group already has a subgroup with the same path.') + expect(transfer_service.error).to eq('Transfer failed: The parent group already has a subgroup or a project with the same path.') end end diff --git a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb index f9ff959fa05..04c6349bf52 100644 --- a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb +++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb @@ -102,6 +102,7 @@ RSpec.describe Projects::LfsPointers::LfsDownloadService do it 'skips read_total_timeout', :aggregate_failures do stub_const('GitLab::HTTP::DEFAULT_READ_TOTAL_TIMEOUT', 0) + expect(ProjectCacheWorker).to receive(:perform_async).once expect(Gitlab::Metrics::System).not_to receive(:monotonic_time) expect(subject.execute).to include(status: :success) end diff --git a/spec/support/helpers/stub_object_storage.rb b/spec/support/helpers/stub_object_storage.rb index 56177d445d6..5e86b08aa45 100644 --- a/spec/support/helpers/stub_object_storage.rb +++ b/spec/support/helpers/stub_object_storage.rb @@ -4,7 +4,6 @@ module StubObjectStorage def stub_dependency_proxy_object_storage(**params) stub_object_storage_uploader(config: ::Gitlab.config.dependency_proxy.object_store, uploader: ::DependencyProxy::FileUploader, - remote_directory: 'dependency_proxy', **params) end @@ -16,7 +15,6 @@ module StubObjectStorage def stub_object_storage_uploader( config:, uploader:, - remote_directory:, enabled: true, proxy_download: false, background_upload: false, @@ -40,7 +38,7 @@ module StubObjectStorage return unless enabled stub_object_storage(connection_params: uploader.object_store_credentials, - remote_directory: remote_directory) + remote_directory: config.remote_directory) end def stub_object_storage(connection_params:, remote_directory:) @@ -60,56 +58,48 @@ module StubObjectStorage def stub_artifacts_object_storage(uploader = JobArtifactUploader, **params) stub_object_storage_uploader(config: Gitlab.config.artifacts.object_store, uploader: uploader, - remote_directory: 'artifacts', **params) end def stub_external_diffs_object_storage(uploader = described_class, **params) stub_object_storage_uploader(config: Gitlab.config.external_diffs.object_store, uploader: uploader, - remote_directory: 'external-diffs', **params) end def stub_lfs_object_storage(**params) stub_object_storage_uploader(config: Gitlab.config.lfs.object_store, uploader: LfsObjectUploader, - remote_directory: 'lfs-objects', **params) end def stub_package_file_object_storage(**params) stub_object_storage_uploader(config: Gitlab.config.packages.object_store, uploader: ::Packages::PackageFileUploader, - remote_directory: 'packages', **params) end def stub_composer_cache_object_storage(**params) stub_object_storage_uploader(config: Gitlab.config.packages.object_store, uploader: ::Packages::Composer::CacheUploader, - remote_directory: 'packages', **params) end def stub_uploads_object_storage(uploader = described_class, **params) stub_object_storage_uploader(config: Gitlab.config.uploads.object_store, uploader: uploader, - remote_directory: 'uploads', **params) end def stub_terraform_state_object_storage(**params) stub_object_storage_uploader(config: Gitlab.config.terraform_state.object_store, uploader: Terraform::StateUploader, - remote_directory: 'terraform', **params) end def stub_pages_object_storage(uploader = described_class, **params) stub_object_storage_uploader(config: Gitlab.config.pages.object_store, uploader: uploader, - remote_directory: 'pages', **params) end diff --git a/spec/support/shared_examples/lib/gitlab/sidekiq_middleware/strategy_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/sidekiq_middleware/strategy_shared_examples.rb index 708bc71ae96..0ba0076e164 100644 --- a/spec/support/shared_examples/lib/gitlab/sidekiq_middleware/strategy_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/sidekiq_middleware/strategy_shared_examples.rb @@ -11,7 +11,7 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| describe '#schedule' do before do - allow(Gitlab::SidekiqLogging::DeduplicationLogger.instance).to receive(:log) + allow(Gitlab::SidekiqLogging::DeduplicationLogger.instance).to receive(:deduplicated_log) end it 'checks for duplicates before yielding' do @@ -40,6 +40,7 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| allow(fake_duplicate_job).to receive(:check!).and_return('the jid') allow(fake_duplicate_job).to receive(:idempotent?).and_return(true) allow(fake_duplicate_job).to receive(:update_latest_wal_location!) + allow(fake_duplicate_job).to receive(:set_deduplicated_flag!) allow(fake_duplicate_job).to receive(:options).and_return({}) job_hash = {} @@ -65,6 +66,7 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| .and_return('the jid')) allow(fake_duplicate_job).to receive(:idempotent?).and_return(true) allow(fake_duplicate_job).to receive(:update_latest_wal_location!) + allow(fake_duplicate_job).to receive(:set_deduplicated_flag!) job_hash = {} expect(fake_duplicate_job).to receive(:duplicate?).and_return(true) @@ -86,6 +88,7 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| receive(:check!).with(time_diff.to_i).and_return('the jid')) allow(fake_duplicate_job).to receive(:idempotent?).and_return(true) allow(fake_duplicate_job).to receive(:update_latest_wal_location!) + allow(fake_duplicate_job).to receive(:set_deduplicated_flag!) job_hash = {} expect(fake_duplicate_job).to receive(:duplicate?).and_return(true) @@ -100,6 +103,26 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| end end + context "when the job is not duplicate" do + before do + allow(fake_duplicate_job).to receive(:scheduled?).and_return(false) + allow(fake_duplicate_job).to receive(:check!).and_return('the jid') + allow(fake_duplicate_job).to receive(:duplicate?).and_return(false) + allow(fake_duplicate_job).to receive(:options).and_return({}) + allow(fake_duplicate_job).to receive(:existing_jid).and_return('the jid') + end + + it 'does not return false nor drop the job' do + schedule_result = nil + + expect(fake_duplicate_job).not_to receive(:set_deduplicated_flag!) + + expect { |b| schedule_result = strategy.schedule({}, &b) }.to yield_control + + expect(schedule_result).to be_nil + end + end + context "when the job is droppable" do before do allow(fake_duplicate_job).to receive(:scheduled?).and_return(false) @@ -109,6 +132,7 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| allow(fake_duplicate_job).to receive(:existing_jid).and_return('the jid') allow(fake_duplicate_job).to receive(:idempotent?).and_return(true) allow(fake_duplicate_job).to receive(:update_latest_wal_location!) + allow(fake_duplicate_job).to receive(:set_deduplicated_flag!) end it 'updates latest wal location' do @@ -117,10 +141,11 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| strategy.schedule({ 'jid' => 'new jid' }) {} end - it 'drops the job' do + it 'returns false to drop the job' do schedule_result = nil expect(fake_duplicate_job).to receive(:idempotent?).and_return(true) + expect(fake_duplicate_job).to receive(:set_deduplicated_flag!).once expect { |b| schedule_result = strategy.schedule({}, &b) }.not_to yield_control expect(schedule_result).to be(false) @@ -130,7 +155,7 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| fake_logger = instance_double(Gitlab::SidekiqLogging::DeduplicationLogger) expect(Gitlab::SidekiqLogging::DeduplicationLogger).to receive(:instance).and_return(fake_logger) - expect(fake_logger).to receive(:log).with(a_hash_including({ 'jid' => 'new jid' }), expected_message, {}) + expect(fake_logger).to receive(:deduplicated_log).with(a_hash_including({ 'jid' => 'new jid' }), expected_message, {}) strategy.schedule({ 'jid' => 'new jid' }) {} end @@ -140,7 +165,7 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| expect(Gitlab::SidekiqLogging::DeduplicationLogger).to receive(:instance).and_return(fake_logger) allow(fake_duplicate_job).to receive(:options).and_return({ foo: :bar }) - expect(fake_logger).to receive(:log).with(a_hash_including({ 'jid' => 'new jid' }), expected_message, { foo: :bar }) + expect(fake_logger).to receive(:deduplicated_log).with(a_hash_including({ 'jid' => 'new jid' }), expected_message, { foo: :bar }) strategy.schedule({ 'jid' => 'new jid' }) {} end @@ -159,6 +184,9 @@ RSpec.shared_examples 'deduplicating jobs when scheduling' do |strategy_name| before do allow(fake_duplicate_job).to receive(:delete!) + allow(fake_duplicate_job).to receive(:scheduled?) { false } + allow(fake_duplicate_job).to receive(:options) { {} } + allow(fake_duplicate_job).to receive(:should_reschedule?) { false } allow(fake_duplicate_job).to receive(:latest_wal_locations).and_return( wal_locations ) end diff --git a/spec/support/shared_examples/requests/api/graphql/mutations/destroy_list_shared_examples.rb b/spec/support/shared_examples/requests/api/graphql/mutations/destroy_list_shared_examples.rb index 0cec67ff541..dca152223fb 100644 --- a/spec/support/shared_examples/requests/api/graphql/mutations/destroy_list_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/graphql/mutations/destroy_list_shared_examples.rb @@ -28,7 +28,7 @@ RSpec.shared_examples 'board lists destroy request' do it 'returns an error' do subject - expect(graphql_errors.first['message']).to include("The resource that you are attempting to access does not exist or you don't have permission to perform this action") + expect(graphql_errors.first['message']).to include(Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR) end end |