diff options
75 files changed, 1521 insertions, 170 deletions
diff --git a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue index eb4150b010d..3280ff48129 100644 --- a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue +++ b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue @@ -28,29 +28,38 @@ export default { { key: 'error', label: __('Error'), - thClass: 'w-70p', - tdClass: 'table-col d-flex align-items-center d-sm-table-cell', + thClass: 'w-60p', + tdClass: 'table-col d-flex d-sm-table-cell px-3', }, { key: 'events', label: __('Events'), - tdClass: 'table-col d-flex align-items-center d-sm-table-cell', + thClass: 'text-right', + tdClass: 'table-col d-flex d-sm-table-cell', }, { key: 'users', label: __('Users'), - tdClass: 'table-col d-flex align-items-center d-sm-table-cell', + thClass: 'text-right', + tdClass: 'table-col d-flex d-sm-table-cell', }, { key: 'lastSeen', label: __('Last seen'), - thClass: 'w-15p', - tdClass: 'table-col d-flex align-items-center d-sm-table-cell', + thClass: '', + tdClass: 'table-col d-flex d-sm-table-cell', }, { key: 'ignore', label: '', - tdClass: 'table-col d-flex align-items-center d-sm-table-cell', + thClass: 'w-3rem', + tdClass: 'table-col d-flex pl-0 d-sm-table-cell', + }, + { + key: 'resolved', + label: '', + thClass: 'w-3rem', + tdClass: 'table-col d-flex pl-0 d-sm-table-cell', }, { key: 'details', @@ -197,9 +206,7 @@ export default { <template> <div class="error-list"> <div v-if="errorTrackingEnabled"> - <div - class="row flex-column flex-sm-row align-items-sm-center row-top m-0 mt-sm-2 mx-sm-1 p-0 p-sm-3" - > + <div class="row flex-column flex-sm-row align-items-sm-center row-top m-0 mt-sm-2 p-0 p-sm-3"> <div class="search-box flex-fill mr-sm-2 my-3 m-sm-0 p-3 p-sm-0"> <div class="filtered-search-box mb-0"> <gl-dropdown @@ -333,6 +340,16 @@ export default { <gl-icon name="eye-slash" :size="12" /> </gl-button> </template> + <template v-slot:resolved="errors"> + <gl-button + ref="resolveError" + v-gl-tooltip + :title="__('Resolve')" + @click="updateIssueStatus(errors.item.id, 'resolved')" + > + <gl-icon name="check-circle" :size="12" /> + </gl-button> + </template> <template v-slot:details="errors"> <gl-button :href="getDetailsLink(errors.item.id)" diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js index 6bbf118d7d1..a03fedcd7e7 100644 --- a/app/assets/javascripts/lib/utils/text_utility.js +++ b/app/assets/javascripts/lib/utils/text_utility.js @@ -21,12 +21,17 @@ export const addDelimiter = text => export const highCountTrim = count => (count > 99 ? '99+' : count); /** - * Converts first char to uppercase and replaces undercores with spaces - * @param {String} string + * Converts first char to uppercase and replaces the given separator with spaces + * @param {String} string - The string to humanize + * @param {String} separator - The separator used to separate words (defaults to "_") * @requires {String} + * @returns {String} */ -export const humanize = string => - string.charAt(0).toUpperCase() + string.replace(/_/g, ' ').slice(1); +export const humanize = (string, separator = '_') => { + const replaceRegex = new RegExp(separator, 'g'); + + return string.charAt(0).toUpperCase() + string.replace(replaceRegex, ' ').slice(1); +}; /** * Replaces underscores with dashes @@ -45,7 +50,11 @@ export const slugify = (str, separator = '-') => { const slug = str .trim() .toLowerCase() - .replace(/[^a-zA-Z0-9_.-]+/g, separator); + .replace(/[^a-zA-Z0-9_.-]+/g, separator) + // Remove any duplicate separators or separator prefixes/suffixes + .split(separator) + .filter(Boolean) + .join(separator); return slug === separator ? '' : slug; }; @@ -160,6 +169,15 @@ export const convertToSentenceCase = string => { }; /** + * Converts a sentence to title case + * e.g. Hello world => Hello World + * + * @param {String} string + * @returns {String} + */ +export const convertToTitleCase = string => string.replace(/\b[a-z]/g, s => s.toUpperCase()); + +/** * Splits camelCase or PascalCase words * e.g. HelloWorld => Hello World * diff --git a/app/assets/javascripts/projects/project_import_gitlab_project.js b/app/assets/javascripts/projects/project_import_gitlab_project.js index fbef3a0b059..4f222438500 100644 --- a/app/assets/javascripts/projects/project_import_gitlab_project.js +++ b/app/assets/javascripts/projects/project_import_gitlab_project.js @@ -1,19 +1,45 @@ import $ from 'jquery'; +import { convertToTitleCase, humanize, slugify } from '../lib/utils/text_utility'; import { getParameterValues } from '../lib/utils/url_utility'; import projectNew from './project_new'; +const prepareParameters = () => { + const name = getParameterValues('name')[0]; + const path = getParameterValues('path')[0]; + + // If the name param exists but the path doesn't then generate it from the name + if (name && !path) { + return { name, path: slugify(name) }; + } + + // If the path param exists but the name doesn't then generate it from the path + if (path && !name) { + return { name: convertToTitleCase(humanize(path, '-')), path }; + } + + return { name, path }; +}; + export default () => { - const pathParam = getParameterValues('path')[0]; - const nameParam = getParameterValues('name')[0]; - const $projectPath = $('.js-path-name'); + let hasUserDefinedProjectName = false; const $projectName = $('.js-project-name'); - - // get the path url and append it in the input - $projectPath.val(pathParam); + const $projectPath = $('.js-path-name'); + const { name, path } = prepareParameters(); // get the project name from the URL and set it as input value - $projectName.val(nameParam); + $projectName.val(name); + + // get the path url and append it in the input + $projectPath.val(path); // generate slug when project name changes - $projectName.keyup(() => projectNew.onProjectNameChange($projectName, $projectPath)); + $projectName.on('keyup', () => { + projectNew.onProjectNameChange($projectName, $projectPath); + hasUserDefinedProjectName = $projectName.val().trim().length > 0; + }); + + // generate project name from the slug if one isn't set + $projectPath.on('keyup', () => + projectNew.onProjectPathChange($projectName, $projectPath, hasUserDefinedProjectName), + ); }; diff --git a/app/assets/javascripts/projects/project_new.js b/app/assets/javascripts/projects/project_new.js index 92c4c05bd87..2aa5f6ec626 100644 --- a/app/assets/javascripts/projects/project_new.js +++ b/app/assets/javascripts/projects/project_new.js @@ -1,14 +1,45 @@ import $ from 'jquery'; import { addSelectOnFocusBehaviour } from '../lib/utils/common_utils'; -import { slugify } from '../lib/utils/text_utility'; +import { convertToTitleCase, humanize, slugify } from '../lib/utils/text_utility'; import { s__ } from '~/locale'; let hasUserDefinedProjectPath = false; +let hasUserDefinedProjectName = false; + +const onProjectNameChange = ($projectNameInput, $projectPathInput) => { + const slug = slugify($projectNameInput.val()); + $projectPathInput.val(slug); +}; + +const onProjectPathChange = ($projectNameInput, $projectPathInput, hasExistingProjectName) => { + const slug = $projectPathInput.val(); + + if (!hasExistingProjectName) { + $projectNameInput.val(convertToTitleCase(humanize(slug, '[-_]'))); + } +}; + +const setProjectNamePathHandlers = ($projectNameInput, $projectPathInput) => { + $projectNameInput.off('keyup change').on('keyup change', () => { + onProjectNameChange($projectNameInput, $projectPathInput); + hasUserDefinedProjectName = $projectNameInput.val().trim().length > 0; + hasUserDefinedProjectPath = $projectPathInput.val().trim().length > 0; + }); + + $projectPathInput.off('keyup change').on('keyup change', () => { + onProjectPathChange($projectNameInput, $projectPathInput, hasUserDefinedProjectName); + hasUserDefinedProjectPath = $projectPathInput.val().trim().length > 0; + }); +}; const deriveProjectPathFromUrl = $projectImportUrl => { + const $currentProjectName = $projectImportUrl + .parents('.toggle-import-form') + .find('#project_name'); const $currentProjectPath = $projectImportUrl .parents('.toggle-import-form') .find('#project_path'); + if (hasUserDefinedProjectPath) { return; } @@ -30,14 +61,10 @@ const deriveProjectPathFromUrl = $projectImportUrl => { const pathMatch = /\/([^/]+)$/.exec(importUrl); if (pathMatch) { $currentProjectPath.val(pathMatch[1]); + onProjectPathChange($currentProjectName, $currentProjectPath, false); } }; -const onProjectNameChange = ($projectNameInput, $projectPathInput) => { - const slug = slugify($projectNameInput.val()); - $projectPathInput.val(slug); -}; - const bindEvents = () => { const $newProjectForm = $('#new_project'); const $projectImportUrl = $('#project_import_url'); @@ -202,10 +229,7 @@ const bindEvents = () => { const $activeTabProjectName = $('.tab-pane.active #project_name'); const $activeTabProjectPath = $('.tab-pane.active #project_path'); $activeTabProjectName.focus(); - $activeTabProjectName.keyup(() => { - onProjectNameChange($activeTabProjectName, $activeTabProjectPath); - hasUserDefinedProjectPath = $activeTabProjectPath.val().trim().length > 0; - }); + setProjectNamePathHandlers($activeTabProjectName, $activeTabProjectPath); } $useTemplateBtn.on('change', chooseTemplate); @@ -220,26 +244,24 @@ const bindEvents = () => { $projectPath.val($projectPath.val().trim()); }); - $projectPath.on('keyup', () => { - hasUserDefinedProjectPath = $projectPath.val().trim().length > 0; - }); - $projectImportUrl.keyup(() => deriveProjectPathFromUrl($projectImportUrl)); $('.js-import-git-toggle-button').on('click', () => { const $projectMirror = $('#project_mirror'); $projectMirror.attr('disabled', !$projectMirror.attr('disabled')); + setProjectNamePathHandlers( + $('.tab-pane.active #project_name'), + $('.tab-pane.active #project_path'), + ); }); - $projectName.on('keyup change', () => { - onProjectNameChange($projectName, $projectPath); - hasUserDefinedProjectPath = $projectPath.val().trim().length > 0; - }); + setProjectNamePathHandlers($projectName, $projectPath); }; export default { bindEvents, deriveProjectPathFromUrl, onProjectNameChange, + onProjectPathChange, }; diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 7521a6491af..dc119b52f4e 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -461,6 +461,7 @@ img.emoji { .w-3rem { width: 3rem; } .w-15p { width: 15%; } .w-30p { width: 30%; } +.w-60p { width: 60%; } .w-70p { width: 70%; } .h-12em { height: 12em; } diff --git a/app/finders/deployments_finder.rb b/app/finders/deployments_finder.rb index b718b55dd68..0bb8ce6b4da 100644 --- a/app/finders/deployments_finder.rb +++ b/app/finders/deployments_finder.rb @@ -17,6 +17,8 @@ class DeploymentsFinder def execute items = init_collection items = by_updated_at(items) + items = by_environment(items) + items = by_status(items) sort(items) end @@ -58,6 +60,24 @@ class DeploymentsFinder items end + def by_environment(items) + if params[:environment].present? + items.for_environment_name(params[:environment]) + else + items + end + end + + def by_status(items) + return items unless params[:status].present? + + unless Deployment.statuses.key?(params[:status]) + raise ArgumentError, "The deployment status #{params[:status]} is invalid" + end + + items.for_status(params[:status]) + end + def sort_params order_by = ALLOWED_SORT_VALUES.include?(params[:order_by]) ? params[:order_by] : DEFAULT_SORT_VALUE order_direction = ALLOWED_SORT_DIRECTIONS.include?(params[:sort]) ? params[:sort] : DEFAULT_SORT_DIRECTION diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 74cc7f93580..e0daf692665 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -30,6 +30,11 @@ class Deployment < ApplicationRecord delegate :name, to: :environment, prefix: true scope :for_environment, -> (environment) { where(environment_id: environment) } + scope :for_environment_name, -> (name) do + joins(:environment).where(environments: { name: name }) + end + + scope :for_status, -> (status) { where(status: status) } scope :visible, -> { where(status: %i[running success failed canceled]) } diff --git a/app/models/group.rb b/app/models/group.rb index 51b4fe4c1ce..b642b177df1 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -57,6 +57,8 @@ class Group < Namespace has_one :import_export_upload + has_many :import_failures, inverse_of: :group + accepts_nested_attributes_for :variables, allow_destroy: true validate :visibility_level_allowed_by_projects diff --git a/app/models/import_failure.rb b/app/models/import_failure.rb index 998572853d3..a1e03218640 100644 --- a/app/models/import_failure.rb +++ b/app/models/import_failure.rb @@ -2,6 +2,8 @@ class ImportFailure < ApplicationRecord belongs_to :project + belongs_to :group - validates :project, presence: true + validates :project, presence: true, unless: :group + validates :group, presence: true, unless: :project end diff --git a/changelogs/unreleased/24305-create-new-project-auto-populate-project-slug-string-to-project-nam.yml b/changelogs/unreleased/24305-create-new-project-auto-populate-project-slug-string-to-project-nam.yml new file mode 100644 index 00000000000..8e4e95540f1 --- /dev/null +++ b/changelogs/unreleased/24305-create-new-project-auto-populate-project-slug-string-to-project-nam.yml @@ -0,0 +1,6 @@ +--- +title: 'Resolve Create new project: Auto-populate project slug string to project name + if name is empty' +merge_request: 22627 +author: +type: changed diff --git a/changelogs/unreleased/29403-migrate-issue-tracker-data.yml b/changelogs/unreleased/29403-migrate-issue-tracker-data.yml new file mode 100644 index 00000000000..9f454640b50 --- /dev/null +++ b/changelogs/unreleased/29403-migrate-issue-tracker-data.yml @@ -0,0 +1,5 @@ +--- +title: Migrate issue trackers data +merge_request: 18639 +author: +type: other diff --git a/changelogs/unreleased/35305-hide-unauthorized-mirroring-actions.yml b/changelogs/unreleased/35305-hide-unauthorized-mirroring-actions.yml new file mode 100644 index 00000000000..09fd9d5cf96 --- /dev/null +++ b/changelogs/unreleased/35305-hide-unauthorized-mirroring-actions.yml @@ -0,0 +1,5 @@ +--- +title: Hide mirror admin actions from developers +merge_request: 21569 +author: +type: fixed diff --git a/changelogs/unreleased/39100-retry-failures-during-import.yml b/changelogs/unreleased/39100-retry-failures-during-import.yml new file mode 100644 index 00000000000..e8ffadd0ca9 --- /dev/null +++ b/changelogs/unreleased/39100-retry-failures-during-import.yml @@ -0,0 +1,5 @@ +--- +title: Add retry logic for failures during import +merge_request: 22265 +author: +type: added diff --git a/changelogs/unreleased/deployments-api-filters.yml b/changelogs/unreleased/deployments-api-filters.yml new file mode 100644 index 00000000000..274a5483ff2 --- /dev/null +++ b/changelogs/unreleased/deployments-api-filters.yml @@ -0,0 +1,5 @@ +--- +title: Filter deployments using the environment & status +merge_request: 22996 +author: +type: added diff --git a/changelogs/unreleased/lm-resolve-sentry-errors-from-list-view.yml b/changelogs/unreleased/lm-resolve-sentry-errors-from-list-view.yml new file mode 100644 index 00000000000..b87f3614ff5 --- /dev/null +++ b/changelogs/unreleased/lm-resolve-sentry-errors-from-list-view.yml @@ -0,0 +1,5 @@ +--- +title: Resolve Sentry errors from error tracking list +merge_request: 23135 +author: +type: added diff --git a/changelogs/unreleased/sh-fix-issue-197464.yml b/changelogs/unreleased/sh-fix-issue-197464.yml new file mode 100644 index 00000000000..05b68b87d17 --- /dev/null +++ b/changelogs/unreleased/sh-fix-issue-197464.yml @@ -0,0 +1,5 @@ +--- +title: Fix analytics tracking for new merge request notes +merge_request: 23273 +author: +type: fixed diff --git a/config/initializers/retriable.rb b/config/initializers/retriable.rb new file mode 100644 index 00000000000..3c673cc7513 --- /dev/null +++ b/config/initializers/retriable.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +Retriable.configure do |config| + config.contexts[:relation_import] = { + tries: ENV.fetch('RELATION_IMPORT_TRIES', 3).to_i, + base_interval: ENV.fetch('RELATION_IMPORT_BASE_INTERVAL', 0.5).to_f, + multiplier: ENV.fetch('RELATION_IMPORT_MULTIPLIER', 1.5).to_f, + rand_factor: ENV.fetch('RELATION_IMPORT_RAND_FACTOR', 0.5).to_f, + on: Gitlab::ImportExport::ImportFailureService::RETRIABLE_EXCEPTIONS + } +end diff --git a/db/migrate/20200115135132_add_retry_count_and_group_id_to_import_failures.rb b/db/migrate/20200115135132_add_retry_count_and_group_id_to_import_failures.rb new file mode 100644 index 00000000000..8f62aaba60e --- /dev/null +++ b/db/migrate/20200115135132_add_retry_count_and_group_id_to_import_failures.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddRetryCountAndGroupIdToImportFailures < ActiveRecord::Migration[5.2] + DOWNTIME = false + + def change + add_column :import_failures, :retry_count, :integer + add_column :import_failures, :group_id, :integer + change_column_null(:import_failures, :project_id, true) + end +end diff --git a/db/migrate/20200115135234_add_group_index_and_fk_to_import_failures.rb b/db/migrate/20200115135234_add_group_index_and_fk_to_import_failures.rb new file mode 100644 index 00000000000..ae0d6d31c42 --- /dev/null +++ b/db/migrate/20200115135234_add_group_index_and_fk_to_import_failures.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class AddGroupIndexAndFkToImportFailures < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + GROUP_INDEX = 'index_import_failures_on_group_id_not_null'.freeze + + disable_ddl_transaction! + + def up + add_concurrent_index(:import_failures, :group_id, where: 'group_id IS NOT NULL', name: GROUP_INDEX) + + add_concurrent_foreign_key(:import_failures, :namespaces, column: :group_id) + end + + def down + remove_foreign_key(:import_failures, column: :group_id) + + remove_concurrent_index_by_name(:import_failures, GROUP_INDEX) + end +end diff --git a/db/migrate/20200117112554_update_project_index_to_import_failures.rb b/db/migrate/20200117112554_update_project_index_to_import_failures.rb new file mode 100644 index 00000000000..1e8347aad44 --- /dev/null +++ b/db/migrate/20200117112554_update_project_index_to_import_failures.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class UpdateProjectIndexToImportFailures < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + PROJECT_INDEX_OLD = 'index_import_failures_on_project_id'.freeze + PROJECT_INDEX_NEW = 'index_import_failures_on_project_id_not_null'.freeze + + disable_ddl_transaction! + + def up + add_concurrent_index(:import_failures, :project_id, where: 'project_id IS NOT NULL', name: PROJECT_INDEX_NEW) + remove_concurrent_index_by_name(:import_failures, PROJECT_INDEX_OLD) + end + + def down + add_concurrent_index(:import_failures, :project_id, name: PROJECT_INDEX_OLD) + remove_concurrent_index_by_name(:import_failures, PROJECT_INDEX_NEW) + end +end diff --git a/db/post_migrate/20190924152703_migrate_issue_trackers_data.rb b/db/post_migrate/20190924152703_migrate_issue_trackers_data.rb new file mode 100644 index 00000000000..93ea501e7e0 --- /dev/null +++ b/db/post_migrate/20190924152703_migrate_issue_trackers_data.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +class MigrateIssueTrackersData < ActiveRecord::Migration[5.1] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INTERVAL = 3.minutes.to_i + BATCH_SIZE = 5_000 + MIGRATION = 'MigrateIssueTrackersSensitiveData' + + disable_ddl_transaction! + + class Service < ActiveRecord::Base + self.table_name = 'services' + self.inheritance_column = :_type_disabled + + include ::EachBatch + end + + def up + relation = Service.where(category: 'issue_tracker').where("properties IS NOT NULL AND properties != '{}' AND properties != ''") + queue_background_migration_jobs_by_range_at_intervals(relation, + MIGRATION, + INTERVAL, + batch_size: BATCH_SIZE) + end + + def down + # no need + end +end diff --git a/db/schema.rb b/db/schema.rb index 8c019f96f94..8ee1f2ffea6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2020_01_14_204949) do +ActiveRecord::Schema.define(version: 2020_01_17_112554) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" @@ -2037,14 +2037,17 @@ ActiveRecord::Schema.define(version: 2020_01_14_204949) do create_table "import_failures", force: :cascade do |t| t.integer "relation_index" - t.bigint "project_id", null: false + t.bigint "project_id" t.datetime_with_timezone "created_at", null: false t.string "relation_key", limit: 64 t.string "exception_class", limit: 128 t.string "correlation_id_value", limit: 128 t.string "exception_message", limit: 255 + t.integer "retry_count" + t.integer "group_id" t.index ["correlation_id_value"], name: "index_import_failures_on_correlation_id_value" - t.index ["project_id"], name: "index_import_failures_on_project_id" + t.index ["group_id"], name: "index_import_failures_on_group_id_not_null", where: "(group_id IS NOT NULL)" + t.index ["project_id"], name: "index_import_failures_on_project_id_not_null", where: "(project_id IS NOT NULL)" end create_table "index_statuses", id: :serial, force: :cascade do |t| @@ -4645,6 +4648,7 @@ ActiveRecord::Schema.define(version: 2020_01_14_204949) do add_foreign_key "identities", "saml_providers", name: "fk_aade90f0fc", on_delete: :cascade add_foreign_key "import_export_uploads", "namespaces", column: "group_id", name: "fk_83319d9721", on_delete: :cascade add_foreign_key "import_export_uploads", "projects", on_delete: :cascade + add_foreign_key "import_failures", "namespaces", column: "group_id", name: "fk_24b824da43", on_delete: :cascade add_foreign_key "index_statuses", "projects", name: "fk_74b2492545", on_delete: :cascade add_foreign_key "insights", "namespaces", on_delete: :cascade add_foreign_key "insights", "projects", on_delete: :cascade diff --git a/doc/api/deployments.md b/doc/api/deployments.md index 2b5942f6b7a..3890a71f283 100644 --- a/doc/api/deployments.md +++ b/doc/api/deployments.md @@ -15,6 +15,16 @@ GET /projects/:id/deployments | `sort` | string | no | Return deployments sorted in `asc` or `desc` order. Default is `asc` | | `updated_after` | datetime | no | Return deployments updated after the specified date | | `updated_before` | datetime | no | Return deployments updated before the specified date | +| `environment` | string | no | The name of the environment to filter deployments by | +| `status` | string | no | The status to filter deployments by | + +The status attribute can be one of the following values: + +- created +- running +- success +- failed +- canceled ```bash curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects/1/deployments" diff --git a/doc/gitlab-basics/create-project.md b/doc/gitlab-basics/create-project.md index 8edce515ec8..74b3afbcd98 100644 --- a/doc/gitlab-basics/create-project.md +++ b/doc/gitlab-basics/create-project.md @@ -31,7 +31,14 @@ To create a new blank project on the **New project** page: 1. On the **Blank project** tab, provide the following information: - The name of your project in the **Project name** field. You can't use special characters, but you can use spaces, hyphens, underscores or even - emoji. + emoji. When adding the name, the **Project slug** will auto populate. + The slug is what the GitLab instance will use as the URL path to the project. + If you want a different slug, input the project name first, + then change the slug after. + - The path to your project in the **Project slug** field. This is the URL + path for your project that the GitLab instance will use. If the + **Project name** is blank, it will auto populate when you fill in + the **Project slug**. - The **Project description (optional)** field enables you to enter a description for your project's dashboard, which will help others understand what your project is about. Though it's not required, it's a good diff --git a/doc/user/markdown.md b/doc/user/markdown.md index d8cc5a9202d..913a4332b1d 100644 --- a/doc/user/markdown.md +++ b/doc/user/markdown.md @@ -407,6 +407,7 @@ GFM will recognize the following: | merge request | `!123` | `namespace/project!123` | `project!123` | | snippet | `$123` | `namespace/project$123` | `project$123` | | epic **(ULTIMATE)** | `&123` | `group1/subgroup&123` | | +| design **(PREMIUM)** | `#123[file.jpg]` or `#123["file.png"]` | `group1/subgroup#123[file.png]` | `project#123[file.png]` | | label by ID | `~123` | `namespace/project~123` | `project~123` | | one-word label by name | `~bug` | `namespace/project~bug` | `project~bug` | | multi-word label by name | `~"feature request"` | `namespace/project~"feature request"` | `project~"feature request"` | diff --git a/doc/user/project/issues/design_management.md b/doc/user/project/issues/design_management.md index 496021c0a5b..c5358f338a5 100644 --- a/doc/user/project/issues/design_management.md +++ b/doc/user/project/issues/design_management.md @@ -37,6 +37,13 @@ Design Management requires that projects are using [hashed storage](../../../administration/repository_storage_types.html#hashed-storage) (the default storage type since v10.0). +### Feature Flags + +- Reference Parsing + + Designs support short references in Markdown, but this needs to be enabled by setting + the `:design_management_reference_filter_gfm_pipeline` feature flag. + ## Limitations - Files uploaded must have a file extension of either `png`, `jpg`, `jpeg`, `gif`, `bmp`, `tiff` or `ico`. @@ -137,3 +144,32 @@ Different discussions have different badge numbers: From GitLab 12.5 on, new annotations will be outputted to the issue activity, so that everyone involved can participate in the discussion. + +## References + +GitLab Flavored Markdown supports references to designs. The syntax for this is: + + `#123[file.jpg]` - the issue reference, with the filename in square braces + +File names may contain a variety of odd characters, so two escaping mechanisms are supported: + +### Quoting + +File names may be quoted with double quotation marks, eg: + + `#123["file.jpg"]` + +This is useful if, for instance, your filename has square braces in its name. In this scheme, all +double quotation marks in the file name need to be escaped with backslashes, and backslashes need +to be escaped likewise: + + `#123["with with \"quote\" marks and a backslash \\.png"]` + +### Base64 Encoding + +In the case of file names that include HTML elements, you will need to escape these names to avoid +them being processed as HTML literals. To do this, we support base64 encoding, eg. + + The file `<a>.jpg` can be referenced as `#123[base64:PGE+LmpwZwo=]` + +Obviously we would advise against using such filenames. diff --git a/lib/api/deployments.rb b/lib/api/deployments.rb index 62bda64896b..487d4e37a56 100644 --- a/lib/api/deployments.rb +++ b/lib/api/deployments.rb @@ -21,6 +21,14 @@ module API optional :sort, type: String, values: DeploymentsFinder::ALLOWED_SORT_DIRECTIONS, default: DeploymentsFinder::DEFAULT_SORT_DIRECTION, desc: 'Sort by asc (ascending) or desc (descending)' optional :updated_after, type: DateTime, desc: 'Return deployments updated after the specified date' optional :updated_before, type: DateTime, desc: 'Return deployments updated before the specified date' + optional :environment, + type: String, + desc: 'The name of the environment to filter deployments by' + + optional :status, + type: String, + values: Deployment.statuses.keys, + desc: 'The status to filter deployments by' end get ':id/deployments' do diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index ca1f61055b0..7f986bccc99 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -43,15 +43,46 @@ module Banzai # Returns a String replaced with the return of the block. def self.references_in(text, pattern = object_class.reference_pattern) text.gsub(pattern) do |match| - symbol = $~[object_sym] - if object_class.reference_valid?(symbol) - yield match, symbol.to_i, $~[:project], $~[:namespace], $~ + if ident = identifier($~) + yield match, ident, $~[:project], $~[:namespace], $~ else match end end end + def self.identifier(match_data) + symbol = symbol_from_match(match_data) + + parse_symbol(symbol, match_data) if object_class.reference_valid?(symbol) + end + + def identifier(match_data) + self.class.identifier(match_data) + end + + def self.symbol_from_match(match) + key = object_sym + match[key] if match.names.include?(key.to_s) + end + + # Transform a symbol extracted from the text to a meaningful value + # In most cases these will be integers, so we call #to_i by default + # + # This method has the contract that if a string `ref` refers to a + # record `record`, then `parse_symbol(ref) == record_identifier(record)`. + def self.parse_symbol(symbol, match_data) + symbol.to_i + end + + # We assume that most classes are identifying records by ID. + # + # This method has the contract that if a string `ref` refers to a + # record `record`, then `class.parse_symbol(ref) == record_identifier(record)`. + def record_identifier(record) + record.id + end + def object_class self.class.object_class end @@ -265,8 +296,10 @@ module Banzai @references_per[parent_type] ||= begin refs = Hash.new { |hash, key| hash[key] = Set.new } - - regex = Regexp.union(object_class.reference_pattern, object_class.link_reference_pattern) + regex = [ + object_class.reference_pattern, + object_class.link_reference_pattern + ].compact.reduce { |a, b| Regexp.union(a, b) } nodes.each do |node| node.to_html.scan(regex) do @@ -276,8 +309,9 @@ module Banzai full_group_path($~[:group]) end - symbol = $~[object_sym] - refs[path] << symbol if object_class.reference_valid?(symbol) + if ident = identifier($~) + refs[path] << ident + end end end diff --git a/lib/banzai/filter/commit_reference_filter.rb b/lib/banzai/filter/commit_reference_filter.rb index e1d7b36b9a2..3df003a88fa 100644 --- a/lib/banzai/filter/commit_reference_filter.rb +++ b/lib/banzai/filter/commit_reference_filter.rb @@ -37,6 +37,11 @@ module Banzai end end + # The default behaviour is `#to_i` - we just pass the hash through. + def self.parse_symbol(sha_hash, _match) + sha_hash + end + def url_for_object(commit, project) h = Gitlab::Routing.url_helpers @@ -65,10 +70,6 @@ module Banzai private - def record_identifier(record) - record.id - end - def parent_records(parent, ids) parent.commits_by(oids: ids.to_a) end diff --git a/lib/gitlab/application_context.rb b/lib/gitlab/application_context.rb index b9190b519a0..71dbfea70e8 100644 --- a/lib/gitlab/application_context.rb +++ b/lib/gitlab/application_context.rb @@ -5,6 +5,14 @@ module Gitlab class ApplicationContext include Gitlab::Utils::LazyAttributes + Attribute = Struct.new(:name, :type) + + APPLICATION_ATTRIBUTES = [ + Attribute.new(:project, Project), + Attribute.new(:namespace, Namespace), + Attribute.new(:user, User) + ].freeze + def self.with_context(args, &block) application_context = new(**args) Labkit::Context.with_context(application_context.to_lazy_hash, &block) @@ -15,21 +23,36 @@ module Gitlab Labkit::Context.push(application_context.to_lazy_hash) end - def initialize(user: nil, project: nil, namespace: nil) - @user, @project, @namespace = user, project, namespace + def initialize(**args) + unknown_attributes = args.keys - APPLICATION_ATTRIBUTES.map(&:name) + raise ArgumentError, "#{unknown_attributes} are not known keys" if unknown_attributes.any? + + @set_values = args.keys + + assign_attributes(args) end def to_lazy_hash - { user: -> { username }, - project: -> { project_path }, - root_namespace: -> { root_namespace_path } } + {}.tap do |hash| + hash[:user] = -> { username } if set_values.include?(:user) + hash[:project] = -> { project_path } if set_values.include?(:project) + hash[:root_namespace] = -> { root_namespace_path } if include_namespace? + end end private - lazy_attr_reader :user, type: User - lazy_attr_reader :project, type: Project - lazy_attr_reader :namespace, type: Namespace + attr_reader :set_values + + APPLICATION_ATTRIBUTES.each do |attr| + lazy_attr_reader attr.name, type: attr.type + end + + def assign_attributes(values) + values.slice(*APPLICATION_ATTRIBUTES.map(&:name)).each do |name, value| + instance_variable_set("@#{name}", value) + end + end def project_path project&.full_path @@ -46,5 +69,9 @@ module Gitlab project&.full_path_components&.first end end + + def include_namespace? + set_values.include?(:namespace) || set_values.include?(:project) + end end end diff --git a/lib/gitlab/background_migration/migrate_issue_trackers_sensitive_data.rb b/lib/gitlab/background_migration/migrate_issue_trackers_sensitive_data.rb new file mode 100644 index 00000000000..14e14f28439 --- /dev/null +++ b/lib/gitlab/background_migration/migrate_issue_trackers_sensitive_data.rb @@ -0,0 +1,146 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + # This migration takes all issue trackers + # and move data from properties to data field tables (jira_tracker_data and issue_tracker_data) + class MigrateIssueTrackersSensitiveData + delegate :select_all, :execute, :quote_string, to: :connection + + # we need to define this class and set fields encryption + class IssueTrackerData < ApplicationRecord + self.table_name = 'issue_tracker_data' + + def self.encryption_options + { + key: Settings.attr_encrypted_db_key_base_32, + encode: true, + mode: :per_attribute_iv, + algorithm: 'aes-256-gcm' + } + end + + attr_encrypted :project_url, encryption_options + attr_encrypted :issues_url, encryption_options + attr_encrypted :new_issue_url, encryption_options + end + + # we need to define this class and set fields encryption + class JiraTrackerData < ApplicationRecord + self.table_name = 'jira_tracker_data' + + def self.encryption_options + { + key: Settings.attr_encrypted_db_key_base_32, + encode: true, + mode: :per_attribute_iv, + algorithm: 'aes-256-gcm' + } + end + + attr_encrypted :url, encryption_options + attr_encrypted :api_url, encryption_options + attr_encrypted :username, encryption_options + attr_encrypted :password, encryption_options + end + + def perform(start_id, stop_id) + columns = 'id, properties, title, description, type' + batch_condition = "id >= #{start_id} AND id <= #{stop_id} AND category = 'issue_tracker' \ + AND properties IS NOT NULL AND properties != '{}' AND properties != ''" + + data_subselect = "SELECT 1 \ + FROM jira_tracker_data \ + WHERE jira_tracker_data.service_id = services.id \ + UNION SELECT 1 \ + FROM issue_tracker_data \ + WHERE issue_tracker_data.service_id = services.id" + + query = "SELECT #{columns} FROM services WHERE #{batch_condition} AND NOT EXISTS (#{data_subselect})" + + migrated_ids = [] + data_to_insert(query).each do |table, data| + service_ids = data.map { |s| s['service_id'] } + + next if service_ids.empty? + + migrated_ids += service_ids + Gitlab::Database.bulk_insert(table, data) + end + + return if migrated_ids.empty? + + move_title_description(migrated_ids) + end + + private + + def data_to_insert(query) + data = { 'jira_tracker_data' => [], 'issue_tracker_data' => [] } + select_all(query).each do |service| + begin + properties = JSON.parse(service['properties']) + rescue JSON::ParserError + logger.warn( + message: 'Properties data not parsed - invalid json', + service_id: service['id'], + properties: service['properties'] + ) + next + end + + if service['type'] == 'JiraService' + row = data_row(JiraTrackerData, jira_mapping(properties), service) + key = 'jira_tracker_data' + else + row = data_row(IssueTrackerData, issue_tracker_mapping(properties), service) + key = 'issue_tracker_data' + end + + data[key] << row if row + end + + data + end + + def data_row(klass, mapping, service) + base_params = { service_id: service['id'], created_at: Time.current, updated_at: Time.current } + klass.new(mapping).slice(*klass.column_names).compact.merge(base_params) + end + + def move_title_description(service_ids) + query = "UPDATE services SET \ + title = cast(properties as json)->>'title', \ + description = cast(properties as json)->>'description' \ + WHERE id IN (#{service_ids.join(',')}) AND title IS NULL AND description IS NULL" + + execute(query) + end + + def jira_mapping(properties) + { + url: properties['url'], + api_url: properties['api_url'], + username: properties['username'], + password: properties['password'] + } + end + + def issue_tracker_mapping(properties) + { + project_url: properties['project_url'], + issues_url: properties['issues_url'], + new_issue_url: properties['new_issue_url'] + } + end + + def connection + @connection ||= ActiveRecord::Base.connection + end + + def logger + @logger ||= Gitlab::BackgroundMigration::Logger.build + end + end + end +end diff --git a/lib/gitlab/import_export/import_failure_service.rb b/lib/gitlab/import_export/import_failure_service.rb new file mode 100644 index 00000000000..eeaf10870c8 --- /dev/null +++ b/lib/gitlab/import_export/import_failure_service.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module Gitlab + module ImportExport + class ImportFailureService + RETRIABLE_EXCEPTIONS = [GRPC::DeadlineExceeded, ActiveRecord::QueryCanceled].freeze + + attr_reader :importable + + def initialize(importable) + @importable = importable + @association = importable.association(:import_failures) + end + + def with_retry(relation_key, relation_index) + on_retry = -> (exception, retry_count, *_args) do + log_import_failure(relation_key, relation_index, exception, retry_count) + end + + Retriable.with_context(:relation_import, on_retry: on_retry) do + yield + end + end + + def log_import_failure(relation_key, relation_index, exception, retry_count = 0) + extra = { + relation_key: relation_key, + relation_index: relation_index, + retry_count: retry_count + } + extra[importable_column_name] = importable.id + + Gitlab::ErrorTracking.track_exception(exception, extra) + + attributes = { + exception_class: exception.class.to_s, + exception_message: exception.message.truncate(255), + correlation_id_value: Labkit::Correlation::CorrelationId.current_or_new_id + }.merge(extra) + + ImportFailure.create(attributes) + end + + private + + def importable_column_name + @importable_column_name ||= @association.reflection.foreign_key.to_sym + end + end + end +end diff --git a/lib/gitlab/import_export/relation_tree_restorer.rb b/lib/gitlab/import_export/relation_tree_restorer.rb index 03c70f17025..44cf90fb86a 100644 --- a/lib/gitlab/import_export/relation_tree_restorer.rb +++ b/lib/gitlab/import_export/relation_tree_restorer.rb @@ -72,25 +72,18 @@ module Gitlab return if importable_class == Project && group_model?(relation_object) relation_object.assign_attributes(importable_class_sym => @importable) - relation_object.save! + + import_failure_service.with_retry(relation_key, relation_index) do + relation_object.save! + end save_id_mapping(relation_key, data_hash, relation_object) rescue => e - log_import_failure(relation_key, relation_index, e) + import_failure_service.log_import_failure(relation_key, relation_index, e) end - def log_import_failure(relation_key, relation_index, exception) - Gitlab::ErrorTracking.track_exception(exception, - project_id: @importable.id, relation_key: relation_key, relation_index: relation_index) - - ImportFailure.create( - project: @importable, - relation_key: relation_key, - relation_index: relation_index, - exception_class: exception.class.to_s, - exception_message: exception.message.truncate(255), - correlation_id_value: Labkit::Correlation::CorrelationId.current_or_new_id - ) + def import_failure_service + @import_failure_service ||= ImportFailureService.new(@importable) end # Older, serialized CI pipeline exports may only have a diff --git a/spec/finders/deployments_finder_spec.rb b/spec/finders/deployments_finder_spec.rb index 2dcbd5ba8ea..b20c7e5a8a5 100644 --- a/spec/finders/deployments_finder_spec.rb +++ b/spec/finders/deployments_finder_spec.rb @@ -25,6 +25,42 @@ describe DeploymentsFinder do is_expected.to match_array([deployment_1]) end end + + context 'when the environment name is specified' do + let!(:environment1) { create(:environment, project: project) } + let!(:environment2) { create(:environment, project: project) } + let!(:deployment1) do + create(:deployment, project: project, environment: environment1) + end + + let!(:deployment2) do + create(:deployment, project: project, environment: environment2) + end + + let(:params) { { environment: environment1.name } } + + it 'returns deployments for the given environment' do + is_expected.to match_array([deployment1]) + end + end + + context 'when the deployment status is specified' do + let!(:deployment1) { create(:deployment, :success, project: project) } + let!(:deployment2) { create(:deployment, :failed, project: project) } + let(:params) { { status: 'success' } } + + it 'returns deployments for the given environment' do + is_expected.to match_array([deployment1]) + end + end + + context 'when using an invalid deployment status' do + let(:params) { { status: 'kittens' } } + + it 'raises ArgumentError' do + expect { subject }.to raise_error(ArgumentError) + end + end end describe 'ordering' do diff --git a/spec/frontend/error_tracking/components/error_tracking_list_spec.js b/spec/frontend/error_tracking/components/error_tracking_list_spec.js index 10b24cf04d3..310cd676ca1 100644 --- a/spec/frontend/error_tracking/components/error_tracking_list_spec.js +++ b/spec/frontend/error_tracking/components/error_tracking_list_spec.js @@ -143,10 +143,14 @@ describe('ErrorTrackingList', () => { }); it('each error in the list should have an ignore button', () => { - const error = wrapper.findAll('tbody tr'); + findErrorListRows().wrappers.forEach(row => { + expect(row.contains('glicon-stub[name="eye-slash"]')).toBe(true); + }); + }); - error.wrappers.forEach((_, index) => { - expect(error.at(index).exists('glicon-stub[name="eye-slash"]')).toBe(true); + it('each error in the list should have a resolve button', () => { + findErrorListRows().wrappers.forEach(row => { + expect(row.contains('glicon-stub[name="check-circle"]')).toBe(true); }); }); @@ -231,8 +235,7 @@ describe('ErrorTrackingList', () => { }); it('sends the "ignored" status and error ID', () => { - const ignoreButton = wrapper.find({ ref: 'ignoreError' }); - ignoreButton.trigger('click'); + wrapper.find({ ref: 'ignoreError' }).trigger('click'); expect(actions.updateStatus).toHaveBeenCalledWith( expect.anything(), { @@ -245,6 +248,34 @@ describe('ErrorTrackingList', () => { }); }); + describe('When the resolve button on an error is clicked', () => { + beforeEach(() => { + store.state.list.loading = false; + store.state.list.errors = errorsList; + + mountComponent({ + stubs: { + GlTable: false, + GlLink: false, + GlButton: false, + }, + }); + }); + + it('sends "resolved" status and error ID', () => { + wrapper.find({ ref: 'resolveError' }).trigger('click'); + expect(actions.updateStatus).toHaveBeenCalledWith( + expect.anything(), + { + endpoint: '/project/test/-/error_tracking/3.json', + redirectUrl: '/error_tracking', + status: 'resolved', + }, + undefined, + ); + }); + }); + describe('When error tracking is disabled and user is not allowed to enable it', () => { beforeEach(() => { mountComponent({ diff --git a/spec/frontend/lib/utils/text_utility_spec.js b/spec/frontend/lib/utils/text_utility_spec.js index deb6dab772e..803b3629524 100644 --- a/spec/frontend/lib/utils/text_utility_spec.js +++ b/spec/frontend/lib/utils/text_utility_spec.js @@ -27,6 +27,9 @@ describe('text_utility', () => { it('should remove underscores and uppercase the first letter', () => { expect(textUtils.humanize('foo_bar')).toEqual('Foo bar'); }); + it('should remove underscores and dashes and uppercase the first letter', () => { + expect(textUtils.humanize('foo_bar-foo', '[_-]')).toEqual('Foo bar foo'); + }); }); describe('dasherize', () => { @@ -52,14 +55,20 @@ describe('text_utility', () => { expect(textUtils.slugify(' a new project ')).toEqual('a-new-project'); }); it('should only remove non-allowed special characters', () => { - expect(textUtils.slugify('test!_pro-ject~')).toEqual('test-_pro-ject-'); + expect(textUtils.slugify('test!_pro-ject~')).toEqual('test-_pro-ject'); }); it('should squash multiple hypens', () => { - expect(textUtils.slugify('test!!!!_pro-ject~')).toEqual('test-_pro-ject-'); + expect(textUtils.slugify('test!!!!_pro-ject~')).toEqual('test-_pro-ject'); }); it('should return empty string if only non-allowed characters', () => { expect(textUtils.slugify('здрасти')).toEqual(''); }); + it('should squash multiple separators', () => { + expect(textUtils.slugify('Test:-)')).toEqual('test'); + }); + it('should trim any separators from the beginning and end of the slug', () => { + expect(textUtils.slugify('-Test:-)-')).toEqual('test'); + }); }); describe('stripHtml', () => { @@ -109,6 +118,12 @@ describe('text_utility', () => { }); }); + describe('convertToTitleCase', () => { + it('converts sentence case to Sentence Case', () => { + expect(textUtils.convertToTitleCase('hello world')).toBe('Hello World'); + }); + }); + describe('truncateSha', () => { it('shortens SHAs to 8 characters', () => { expect(textUtils.truncateSha('verylongsha')).toBe('verylong'); diff --git a/spec/frontend/monitoring/store/mutations_spec.js b/spec/frontend/monitoring/store/mutations_spec.js index e89c22268d4..cb53ab60bdb 100644 --- a/spec/frontend/monitoring/store/mutations_spec.js +++ b/spec/frontend/monitoring/store/mutations_spec.js @@ -29,8 +29,8 @@ describe('Monitoring mutations', () => { mutations[types.RECEIVE_METRICS_DATA_SUCCESS](stateCopy, payload); const groups = getGroups(); - expect(groups[0].key).toBe('response-metrics-nginx-ingress-vts--0'); - expect(groups[1].key).toBe('system-metrics-kubernetes--1'); + expect(groups[0].key).toBe('response-metrics-nginx-ingress-vts-0'); + expect(groups[1].key).toBe('system-metrics-kubernetes-1'); }); it('normalizes values', () => { mutations[types.RECEIVE_METRICS_DATA_SUCCESS](stateCopy, payload); diff --git a/spec/frontend/releases/list/components/release_block_spec.js b/spec/frontend/releases/list/components/release_block_spec.js index e3d6af13417..20c25a4aac2 100644 --- a/spec/frontend/releases/list/components/release_block_spec.js +++ b/spec/frontend/releases/list/components/release_block_spec.js @@ -169,7 +169,7 @@ describe('Release block', () => { releaseClone.tag_name = 'a dangerous tag name <script>alert("hello")</script>'; return factory(releaseClone).then(() => { - expect(wrapper.attributes().id).toBe('a-dangerous-tag-name-script-alert-hello-script-'); + expect(wrapper.attributes().id).toBe('a-dangerous-tag-name-script-alert-hello-script'); }); }); diff --git a/spec/javascripts/projects/project_import_gitlab_project_spec.js b/spec/javascripts/projects/project_import_gitlab_project_spec.js index 126f73103e0..3c94934699d 100644 --- a/spec/javascripts/projects/project_import_gitlab_project_spec.js +++ b/spec/javascripts/projects/project_import_gitlab_project_spec.js @@ -1,25 +1,59 @@ import projectImportGitlab from '~/projects/project_import_gitlab_project'; describe('Import Gitlab project', () => { - let projectName; - beforeEach(() => { - projectName = 'project'; - window.history.pushState({}, null, `?path=${projectName}`); + const pathName = 'my-project'; + const projectName = 'My Project'; + + const setTestFixtures = url => { + window.history.pushState({}, null, url); setFixtures(` <input class="js-path-name" /> + <input class="js-project-name" /> `); projectImportGitlab(); + }; + + beforeEach(() => { + setTestFixtures(`?name=${projectName}&path=${pathName}`); }); afterEach(() => { window.history.pushState({}, null, ''); }); - describe('path name', () => { + describe('project name', () => { it('should fill in the project name derived from the previously filled project name', () => { - expect(document.querySelector('.js-path-name').value).toEqual(projectName); + expect(document.querySelector('.js-project-name').value).toEqual(projectName); + }); + + describe('empty path name', () => { + it('derives the path name from the previously filled project name', () => { + const alternateProjectName = 'My Alt Project'; + const alternatePathName = 'my-alt-project'; + + setTestFixtures(`?name=${alternateProjectName}`); + + expect(document.querySelector('.js-path-name').value).toEqual(alternatePathName); + }); + }); + }); + + describe('path name', () => { + it('should fill in the path name derived from the previously filled path name', () => { + expect(document.querySelector('.js-path-name').value).toEqual(pathName); + }); + + describe('empty project name', () => { + it('derives the project name from the previously filled path name', () => { + const alternateProjectName = 'My Alt Project'; + const alternatePathName = 'my-alt-project'; + + setTestFixtures(`?path=${alternatePathName}`); + + expect(document.querySelector('.js-project-name').value).toEqual(alternateProjectName); + }); }); }); }); diff --git a/spec/javascripts/projects/project_new_spec.js b/spec/javascripts/projects/project_new_spec.js index 106a3ba94e4..7c6ff90aff6 100644 --- a/spec/javascripts/projects/project_new_spec.js +++ b/spec/javascripts/projects/project_new_spec.js @@ -172,4 +172,34 @@ describe('New Project', () => { expect($projectPath.val()).toEqual('my-dash-delimited-awesome-project'); }); }); + + describe('derivesProjectNameFromSlug', () => { + const dummyProjectPath = 'my-awesome-project'; + const dummyProjectName = 'Original Awesome Project'; + + beforeEach(() => { + projectNew.bindEvents(); + $projectPath.val('').change(); + }); + + it('converts slug to humanized project name', () => { + $projectPath.val(dummyProjectPath); + + projectNew.onProjectPathChange($projectName, $projectPath); + + expect($projectName.val()).toEqual('My Awesome Project'); + }); + + it('does not convert slug to humanized project name if a project name already exists', () => { + $projectName.val(dummyProjectName); + $projectPath.val(dummyProjectPath); + projectNew.onProjectPathChange( + $projectName, + $projectPath, + $projectName.val().trim().length > 0, + ); + + expect($projectName.val()).toEqual(dummyProjectName); + }); + }); }); diff --git a/spec/lib/banzai/filter/abstract_reference_filter_spec.rb b/spec/lib/banzai/filter/abstract_reference_filter_spec.rb index 3e8b0ea113f..798112d0f53 100644 --- a/spec/lib/banzai/filter/abstract_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/abstract_reference_filter_spec.rb @@ -3,30 +3,27 @@ require 'spec_helper' describe Banzai::Filter::AbstractReferenceFilter do - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } + + let(:doc) { Nokogiri::HTML.fragment('') } + let(:filter) { described_class.new(doc, project: project) } describe '#references_per_parent' do - it 'returns a Hash containing references grouped per parent paths' do - doc = Nokogiri::HTML.fragment("#1 #{project.full_path}#2") - filter = described_class.new(doc, project: project) + let(:doc) { Nokogiri::HTML.fragment("#1 #{project.full_path}#2 #2") } - expect(filter).to receive(:object_class).exactly(4).times.and_return(Issue) - expect(filter).to receive(:object_sym).twice.and_return(:issue) + it 'returns a Hash containing references grouped per parent paths' do + expect(described_class).to receive(:object_class).exactly(6).times.and_return(Issue) refs = filter.references_per_parent - expect(refs).to be_an_instance_of(Hash) - expect(refs[project.full_path]).to eq(Set.new(%w[1 2])) + expect(refs).to match(a_hash_including(project.full_path => contain_exactly(1, 2))) end end describe '#parent_per_reference' do it 'returns a Hash containing projects grouped per parent paths' do - doc = Nokogiri::HTML.fragment('') - filter = described_class.new(doc, project: project) - expect(filter).to receive(:references_per_parent) - .and_return({ project.full_path => Set.new(%w[1]) }) + .and_return({ project.full_path => Set.new([1]) }) expect(filter.parent_per_reference) .to eq({ project.full_path => project }) @@ -34,9 +31,6 @@ describe Banzai::Filter::AbstractReferenceFilter do end describe '#find_for_paths' do - let(:doc) { Nokogiri::HTML.fragment('') } - let(:filter) { described_class.new(doc, project: project) } - context 'with RequestStore disabled' do it 'returns a list of Projects for a list of paths' do expect(filter.find_for_paths([project.full_path])) diff --git a/spec/lib/gitlab/application_context_spec.rb b/spec/lib/gitlab/application_context_spec.rb index 1c8fcb8d737..482bf0dc192 100644 --- a/spec/lib/gitlab/application_context_spec.rb +++ b/spec/lib/gitlab/application_context_spec.rb @@ -28,7 +28,7 @@ describe Gitlab::ApplicationContext do describe '.push' do it 'passes the expected context on to labkit' do fake_proc = duck_type(:call) - expected_context = hash_including(user: fake_proc, project: fake_proc, root_namespace: fake_proc) + expected_context = { user: fake_proc } expect(Labkit::Context).to receive(:push).with(expected_context) @@ -78,5 +78,27 @@ describe Gitlab::ApplicationContext do expect(result(context)) .to include(project: project.full_path, root_namespace: project.full_path_components.first) end + + context 'only include values for which an option was specified' do + using RSpec::Parameterized::TableSyntax + + where(:provided_options, :expected_context_keys) do + [:user, :namespace, :project] | [:user, :project, :root_namespace] + [:user, :project] | [:user, :project, :root_namespace] + [:user, :namespace] | [:user, :root_namespace] + [:user] | [:user] + [] | [] + end + + with_them do + it do + # Build a hash that has all `provided_options` as keys, and `nil` as value + provided_values = provided_options.map { |key| [key, nil] }.to_h + context = described_class.new(provided_values) + + expect(context.to_lazy_hash.keys).to contain_exactly(*expected_context_keys) + end + end + end end end diff --git a/spec/lib/gitlab/background_migration/migrate_issue_trackers_sensitive_data_spec.rb b/spec/lib/gitlab/background_migration/migrate_issue_trackers_sensitive_data_spec.rb new file mode 100644 index 00000000000..664e3810fc9 --- /dev/null +++ b/spec/lib/gitlab/background_migration/migrate_issue_trackers_sensitive_data_spec.rb @@ -0,0 +1,320 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Gitlab::BackgroundMigration::MigrateIssueTrackersSensitiveData, :migration, schema: 20190924152703 do + let(:services) { table(:services) } + + # we need to define the classes due to encryption + class IssueTrackerData < ApplicationRecord + self.table_name = 'issue_tracker_data' + + def self.encryption_options + { + key: Settings.attr_encrypted_db_key_base_32, + encode: true, + mode: :per_attribute_iv, + algorithm: 'aes-256-gcm' + } + end + + attr_encrypted :project_url, encryption_options + attr_encrypted :issues_url, encryption_options + attr_encrypted :new_issue_url, encryption_options + end + + class JiraTrackerData < ApplicationRecord + self.table_name = 'jira_tracker_data' + + def self.encryption_options + { + key: Settings.attr_encrypted_db_key_base_32, + encode: true, + mode: :per_attribute_iv, + algorithm: 'aes-256-gcm' + } + end + + attr_encrypted :url, encryption_options + attr_encrypted :api_url, encryption_options + attr_encrypted :username, encryption_options + attr_encrypted :password, encryption_options + end + + let(:url) { 'http://base-url.tracker.com' } + let(:new_issue_url) { 'http://base-url.tracker.com/new_issue' } + let(:issues_url) { 'http://base-url.tracker.com/issues' } + let(:api_url) { 'http://api.tracker.com' } + let(:password) { 'passw1234' } + let(:username) { 'user9' } + let(:title) { 'Issue tracker' } + let(:description) { 'Issue tracker description' } + + let(:jira_properties) do + { + 'api_url' => api_url, + 'jira_issue_transition_id' => '5', + 'password' => password, + 'url' => url, + 'username' => username, + 'title' => title, + 'description' => description, + 'other_field' => 'something' + } + end + + let(:tracker_properties) do + { + 'project_url' => url, + 'new_issue_url' => new_issue_url, + 'issues_url' => issues_url, + 'title' => title, + 'description' => description, + 'other_field' => 'something' + } + end + + let(:tracker_properties_no_url) do + { + 'new_issue_url' => new_issue_url, + 'issues_url' => issues_url, + 'title' => title, + 'description' => description + } + end + + subject { described_class.new.perform(1, 100) } + + shared_examples 'handle properties' do + it 'does not clear the properties' do + expect { subject }.not_to change { service.reload.properties} + end + end + + context 'with jira service' do + let!(:service) do + services.create(id: 10, type: 'JiraService', title: nil, properties: jira_properties.to_json, category: 'issue_tracker') + end + + it_behaves_like 'handle properties' + + it 'migrates data' do + expect { subject }.to change { JiraTrackerData.count }.by(1) + + service.reload + data = JiraTrackerData.find_by(service_id: service.id) + + expect(data.url).to eq(url) + expect(data.api_url).to eq(api_url) + expect(data.username).to eq(username) + expect(data.password).to eq(password) + expect(service.title).to eq(title) + expect(service.description).to eq(description) + end + end + + context 'with bugzilla service' do + let!(:service) do + services.create(id: 11, type: 'BugzillaService', title: nil, properties: tracker_properties.to_json, category: 'issue_tracker') + end + + it_behaves_like 'handle properties' + + it 'migrates data' do + expect { subject }.to change { IssueTrackerData.count }.by(1) + + service.reload + data = IssueTrackerData.find_by(service_id: service.id) + + expect(data.project_url).to eq(url) + expect(data.issues_url).to eq(issues_url) + expect(data.new_issue_url).to eq(new_issue_url) + expect(service.title).to eq(title) + expect(service.description).to eq(description) + end + end + + context 'with youtrack service' do + let!(:service) do + services.create(id: 12, type: 'YoutrackService', title: nil, properties: tracker_properties_no_url.to_json, category: 'issue_tracker') + end + + it_behaves_like 'handle properties' + + it 'migrates data' do + expect { subject }.to change { IssueTrackerData.count }.by(1) + + service.reload + data = IssueTrackerData.find_by(service_id: service.id) + + expect(data.project_url).to be_nil + expect(data.issues_url).to eq(issues_url) + expect(data.new_issue_url).to eq(new_issue_url) + expect(service.title).to eq(title) + expect(service.description).to eq(description) + end + end + + context 'with gitlab service with no properties' do + let!(:service) do + services.create(id: 13, type: 'GitlabIssueTrackerService', title: nil, properties: {}, category: 'issue_tracker') + end + + it_behaves_like 'handle properties' + + it 'does not migrate data' do + expect { subject }.not_to change { IssueTrackerData.count } + end + end + + context 'with redmine service already with data fields' do + let!(:service) do + services.create(id: 14, type: 'RedmineService', title: nil, properties: tracker_properties_no_url.to_json, category: 'issue_tracker').tap do |service| + IssueTrackerData.create!(service_id: service.id, project_url: url, new_issue_url: new_issue_url, issues_url: issues_url) + end + end + + it_behaves_like 'handle properties' + + it 'does not create new data fields record' do + expect { subject }.not_to change { IssueTrackerData.count } + end + end + + context 'with custom issue tracker which has data fields record inconsistent with properties field' do + let!(:service) do + services.create(id: 15, type: 'CustomIssueTrackerService', title: 'Existing title', properties: jira_properties.to_json, category: 'issue_tracker').tap do |service| + IssueTrackerData.create!(service_id: service.id, project_url: 'http://other_url', new_issue_url: 'http://other_url/new_issue', issues_url: 'http://other_url/issues') + end + end + + it_behaves_like 'handle properties' + + it 'does not update the data fields record' do + expect { subject }.not_to change { IssueTrackerData.count } + + service.reload + data = IssueTrackerData.find_by(service_id: service.id) + + expect(data.project_url).to eq('http://other_url') + expect(data.issues_url).to eq('http://other_url/issues') + expect(data.new_issue_url).to eq('http://other_url/new_issue') + expect(service.title).to eq('Existing title') + end + end + + context 'with jira service which has data fields record inconsistent with properties field' do + let!(:service) do + services.create(id: 16, type: 'CustomIssueTrackerService', description: 'Existing description', properties: jira_properties.to_json, category: 'issue_tracker').tap do |service| + JiraTrackerData.create!(service_id: service.id, url: 'http://other_jira_url') + end + end + + it_behaves_like 'handle properties' + + it 'does not update the data fields record' do + expect { subject }.not_to change { JiraTrackerData.count } + + service.reload + data = JiraTrackerData.find_by(service_id: service.id) + + expect(data.url).to eq('http://other_jira_url') + expect(data.password).to be_nil + expect(data.username).to be_nil + expect(data.api_url).to be_nil + expect(service.description).to eq('Existing description') + end + end + + context 'non issue tracker service' do + let!(:service) do + services.create(id: 17, title: nil, description: nil, type: 'OtherService', properties: tracker_properties.to_json) + end + + it_behaves_like 'handle properties' + + it 'does not migrate any data' do + expect { subject }.not_to change { IssueTrackerData.count } + + service.reload + expect(service.title).to be_nil + expect(service.description).to be_nil + end + end + + context 'jira service with empty properties' do + let!(:service) do + services.create(id: 18, type: 'JiraService', properties: '', category: 'issue_tracker') + end + + it_behaves_like 'handle properties' + + it 'does not migrate any data' do + expect { subject }.not_to change { JiraTrackerData.count } + end + end + + context 'jira service with nil properties' do + let!(:service) do + services.create(id: 18, type: 'JiraService', properties: nil, category: 'issue_tracker') + end + + it_behaves_like 'handle properties' + + it 'does not migrate any data' do + expect { subject }.not_to change { JiraTrackerData.count } + end + end + + context 'jira service with invalid properties' do + let!(:service) do + services.create(id: 18, type: 'JiraService', properties: 'invalid data', category: 'issue_tracker') + end + + it_behaves_like 'handle properties' + + it 'does not migrate any data' do + expect { subject }.not_to change { JiraTrackerData.count } + end + end + + context 'with jira service with invalid properties, valid jira service and valid bugzilla service' do + let!(:jira_service_invalid) do + services.create(id: 19, title: 'invalid - title', description: 'invalid - description', type: 'JiraService', properties: 'invalid data', category: 'issue_tracker') + end + let!(:jira_service_valid) do + services.create(id: 20, type: 'JiraService', properties: jira_properties.to_json, category: 'issue_tracker') + end + let!(:bugzilla_service_valid) do + services.create(id: 11, type: 'BugzillaService', title: nil, properties: tracker_properties.to_json, category: 'issue_tracker') + end + + it 'migrates data for the valid service' do + subject + + jira_service_invalid.reload + expect(JiraTrackerData.find_by(service_id: jira_service_invalid.id)).to be_nil + expect(jira_service_invalid.title).to eq('invalid - title') + expect(jira_service_invalid.description).to eq('invalid - description') + expect(jira_service_invalid.properties).to eq('invalid data') + + jira_service_valid.reload + data = JiraTrackerData.find_by(service_id: jira_service_valid.id) + + expect(data.url).to eq(url) + expect(data.api_url).to eq(api_url) + expect(data.username).to eq(username) + expect(data.password).to eq(password) + expect(jira_service_valid.title).to eq(title) + expect(jira_service_valid.description).to eq(description) + + bugzilla_service_valid.reload + data = IssueTrackerData.find_by(service_id: bugzilla_service_valid.id) + + expect(data.project_url).to eq(url) + expect(data.issues_url).to eq(issues_url) + expect(data.new_issue_url).to eq(new_issue_url) + expect(bugzilla_service_valid.title).to eq(title) + expect(bugzilla_service_valid.description).to eq(description) + end + end +end diff --git a/spec/lib/gitlab/import_export/import_failure_service_spec.rb b/spec/lib/gitlab/import_export/import_failure_service_spec.rb new file mode 100644 index 00000000000..0351f88afdb --- /dev/null +++ b/spec/lib/gitlab/import_export/import_failure_service_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::ImportExport::ImportFailureService do + let(:importable) { create(:project, :builds_enabled, :issues_disabled, name: 'project', path: 'project') } + let(:label) { create(:label) } + let(:subject) { described_class.new(importable) } + let(:relation_key) { "labels" } + let(:relation_index) { 0 } + + describe '#log_import_failure' do + let(:standard_error_message) { "StandardError message" } + let(:exception) { StandardError.new(standard_error_message) } + let(:correlation_id) { 'my-correlation-id' } + let(:retry_count) { 2 } + let(:log_import_failure) do + subject.log_import_failure(relation_key, relation_index, exception, retry_count) + end + + before do + # Import is running from the rake task, `correlation_id` is not assigned + allow(Labkit::Correlation::CorrelationId).to receive(:current_or_new_id).and_return(correlation_id) + end + + context 'when importable is a group' do + let(:importable) { create(:group) } + + it_behaves_like 'log import failure', :group_id + end + + context 'when importable is a project' do + it_behaves_like 'log import failure', :project_id + end + + context 'when ImportFailure does not support importable class' do + let(:importable) { create(:merge_request) } + + it 'raise exception' do + expect { subject }.to raise_exception(ActiveRecord::AssociationNotFoundError, "Association named 'import_failures' was not found on MergeRequest; perhaps you misspelled it?") + end + end + end + + describe '#with_retry' do + let(:perform_retry) do + subject.with_retry(relation_key, relation_index) do + label.save! + end + end + + context 'when exceptions are retriable' do + where(:exception) { Gitlab::ImportExport::ImportFailureService::RETRIABLE_EXCEPTIONS } + + with_them do + context 'when retry succeeds' do + before do + expect(label).to receive(:save!).and_raise(exception.new) + expect(label).to receive(:save!).and_return(true) + end + + it 'retries and logs import failure once with correct params' do + expect(subject).to receive(:log_import_failure).with(relation_key, relation_index, instance_of(exception), 1).once + + perform_retry + end + end + + context 'when retry continues to fail with intermittent errors' do + let(:maximum_retry_count) do + Retriable.config.tries + end + + before do + expect(label).to receive(:save!) + .exactly(maximum_retry_count).times + .and_raise(exception.new) + end + + it 'retries the number of times allowed and raise exception', :aggregate_failures do + expect { perform_retry }.to raise_exception(exception) + end + + it 'logs import failure each time and raise exception', :aggregate_failures do + maximum_retry_count.times do |index| + retry_count = index + 1 + + expect(subject).to receive(:log_import_failure).with(relation_key, relation_index, instance_of(exception), retry_count) + end + + expect { perform_retry }.to raise_exception(exception) + end + end + end + end + + context 'when exception is not retriable' do + let(:exception) { StandardError.new } + + it 'raise the exception', :aggregate_failures do + expect(label).to receive(:save!).once.and_raise(exception) + expect(subject).not_to receive(:log_import_failure) + expect { perform_retry }.to raise_exception(exception) + end + end + end +end diff --git a/spec/migrations/20190924152703_migrate_issue_trackers_data_spec.rb b/spec/migrations/20190924152703_migrate_issue_trackers_data_spec.rb new file mode 100644 index 00000000000..0453ac87436 --- /dev/null +++ b/spec/migrations/20190924152703_migrate_issue_trackers_data_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20190924152703_migrate_issue_trackers_data.rb') + +describe MigrateIssueTrackersData, :migration do + let(:services) { table(:services) } + let(:migration_class) { Gitlab::BackgroundMigration::MigrateIssueTrackersSensitiveData } + let(:migration_name) { migration_class.to_s.demodulize } + + let(:properties) do + { + 'url' => 'http://example.com' + } + end + let!(:jira_service) do + services.create(id: 10, type: 'JiraService', properties: properties, category: 'issue_tracker') + end + let!(:jira_service_nil) do + services.create(id: 11, type: 'JiraService', properties: nil, category: 'issue_tracker') + end + let!(:bugzilla_service) do + services.create(id: 12, type: 'BugzillaService', properties: properties, category: 'issue_tracker') + end + let!(:youtrack_service) do + services.create(id: 13, type: 'YoutrackService', properties: properties, category: 'issue_tracker') + end + let!(:youtrack_service_empty) do + services.create(id: 14, type: 'YoutrackService', properties: '', category: 'issue_tracker') + end + let!(:gitlab_service) do + services.create(id: 15, type: 'GitlabIssueTrackerService', properties: properties, category: 'issue_tracker') + end + let!(:gitlab_service_empty) do + services.create(id: 16, type: 'GitlabIssueTrackerService', properties: {}, category: 'issue_tracker') + end + let!(:other_service) do + services.create(id: 17, type: 'OtherService', properties: properties, category: 'other_category') + end + + before do + stub_const("#{described_class}::BATCH_SIZE", 2) + end + + it 'schedules background migrations at correct time' do + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(migration_name).to be_scheduled_delayed_migration(3.minutes, jira_service.id, bugzilla_service.id) + expect(migration_name).to be_scheduled_delayed_migration(6.minutes, youtrack_service.id, gitlab_service.id) + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + end + end + end +end diff --git a/spec/models/import_failure_spec.rb b/spec/models/import_failure_spec.rb new file mode 100644 index 00000000000..d6574791a65 --- /dev/null +++ b/spec/models/import_failure_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ImportFailure do + describe "Associations" do + it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:group) } + end + + describe 'Validations' do + context 'has no group' do + before do + allow(subject).to receive(:group).and_return(nil) + end + + it { is_expected.to validate_presence_of(:project) } + end + + context 'has no project' do + before do + allow(subject).to receive(:project).and_return(nil) + end + + it { is_expected.to validate_presence_of(:group) } + end + end +end diff --git a/spec/requests/api/deployments_spec.rb b/spec/requests/api/deployments_spec.rb index 9add328f6a8..d8fc234cbae 100644 --- a/spec/requests/api/deployments_spec.rb +++ b/spec/requests/api/deployments_spec.rb @@ -40,6 +40,18 @@ describe API::Deployments do end end + context 'with the environment filter specifed' do + it 'returns deployments for the environment' do + get( + api("/projects/#{project.id}/deployments", user), + params: { environment: deployment_1.environment.name } + ) + + expect(json_response.size).to eq(1) + expect(json_response.first['iid']).to eq(deployment_1.iid) + end + end + describe 'ordering' do let(:order_by) { 'iid' } let(:sort) { 'desc' } diff --git a/spec/services/merge_requests/conflicts/list_service_spec.rb b/spec/services/merge_requests/conflicts/list_service_spec.rb index 68a9c0a8b86..13d69307084 100644 --- a/spec/services/merge_requests/conflicts/list_service_spec.rb +++ b/spec/services/merge_requests/conflicts/list_service_spec.rb @@ -74,7 +74,9 @@ describe MergeRequests::Conflicts::ListService do it 'returns a falsey value when the MR has a missing ref after a force push' do merge_request = create_merge_request('conflict-resolvable') service = conflicts_service(merge_request) - allow_any_instance_of(Gitlab::GitalyClient::ConflictsService).to receive(:list_conflict_files).and_raise(GRPC::Unknown) + allow_next_instance_of(Gitlab::GitalyClient::ConflictsService) do |instance| + allow(instance).to receive(:list_conflict_files).and_raise(GRPC::Unknown) + end expect(service.can_be_resolved_in_ui?).to be_falsey end diff --git a/spec/services/merge_requests/create_from_issue_service_spec.rb b/spec/services/merge_requests/create_from_issue_service_spec.rb index 7145cfe7897..3d58ecdd8cd 100644 --- a/spec/services/merge_requests/create_from_issue_service_spec.rb +++ b/spec/services/merge_requests/create_from_issue_service_spec.rb @@ -55,7 +55,9 @@ describe MergeRequests::CreateFromIssueService do end it 'creates the new_issue_branch system note when the branch could be created but the merge_request cannot be created', :sidekiq_might_not_need_inline do - expect_any_instance_of(MergeRequest).to receive(:valid?).at_least(:once).and_return(false) + expect_next_instance_of(MergeRequest) do |instance| + expect(instance).to receive(:valid?).at_least(:once).and_return(false) + end expect(SystemNoteService).to receive(:new_issue_branch).with(issue, project, user, issue.to_branch_name, branch_project: target_project) diff --git a/spec/services/merge_requests/push_options_handler_service_spec.rb b/spec/services/merge_requests/push_options_handler_service_spec.rb index 7f9c47d8670..420c8513c72 100644 --- a/spec/services/merge_requests/push_options_handler_service_spec.rb +++ b/spec/services/merge_requests/push_options_handler_service_spec.rb @@ -714,9 +714,9 @@ describe MergeRequests::PushOptionsHandlerService do let(:exception) { StandardError.new('My standard error') } def run_service_with_exception - allow_any_instance_of( - MergeRequests::BuildService - ).to receive(:execute).and_raise(exception) + allow_next_instance_of(MergeRequests::BuildService) do |instance| + allow(instance).to receive(:execute).and_raise(exception) + end service.execute end @@ -766,9 +766,9 @@ describe MergeRequests::PushOptionsHandlerService do invalid_merge_request = MergeRequest.new invalid_merge_request.errors.add(:base, 'my error') - expect_any_instance_of( - MergeRequests::CreateService - ).to receive(:execute).and_return(invalid_merge_request) + expect_next_instance_of(MergeRequests::CreateService) do |instance| + expect(instance).to receive(:execute).and_return(invalid_merge_request) + end service.execute diff --git a/spec/services/milestones/promote_service_spec.rb b/spec/services/milestones/promote_service_spec.rb index 22c7e9dde30..fa893b86cdb 100644 --- a/spec/services/milestones/promote_service_spec.rb +++ b/spec/services/milestones/promote_service_spec.rb @@ -31,7 +31,9 @@ describe Milestones::PromoteService do it 'does not promote milestone and update issuables if promoted milestone is not valid' do issue = create(:issue, milestone: milestone, project: project) merge_request = create(:merge_request, milestone: milestone, source_project: project) - allow_any_instance_of(Milestone).to receive(:valid?).and_return(false) + allow_next_instance_of(Milestone) do |instance| + allow(instance).to receive(:valid?).and_return(false) + end expect { service.execute(milestone) }.to raise_error(described_class::PromoteMilestoneError) diff --git a/spec/services/milestones/transfer_service_spec.rb b/spec/services/milestones/transfer_service_spec.rb index b3d41eb0763..711969ce504 100644 --- a/spec/services/milestones/transfer_service_spec.rb +++ b/spec/services/milestones/transfer_service_spec.rb @@ -71,7 +71,9 @@ describe Milestones::TransferService do context 'when find_or_create_milestone returns nil' do before do - allow_any_instance_of(Milestones::FindOrCreateService).to receive(:execute).and_return(nil) + allow_next_instance_of(Milestones::FindOrCreateService) do |instance| + allow(instance).to receive(:execute).and_return(nil) + end end it 'removes issues group milestone' do diff --git a/spec/services/namespaces/statistics_refresher_service_spec.rb b/spec/services/namespaces/statistics_refresher_service_spec.rb index 9d42e917efe..1fa0a794edd 100644 --- a/spec/services/namespaces/statistics_refresher_service_spec.rb +++ b/spec/services/namespaces/statistics_refresher_service_spec.rb @@ -17,7 +17,9 @@ describe Namespaces::StatisticsRefresherService, '#execute' do end it 'recalculate the namespace statistics' do - expect_any_instance_of(Namespace::RootStorageStatistics).to receive(:recalculate!).once + expect_next_instance_of(Namespace::RootStorageStatistics) do |instance| + expect(instance).to receive(:recalculate!).once + end service.execute(group) end @@ -45,8 +47,9 @@ describe Namespaces::StatisticsRefresherService, '#execute' do context 'when something goes wrong' do before do - allow_any_instance_of(Namespace::RootStorageStatistics) - .to receive(:recalculate!).and_raise(ActiveRecord::ActiveRecordError) + allow_next_instance_of(Namespace::RootStorageStatistics) do |instance| + allow(instance).to receive(:recalculate!).and_raise(ActiveRecord::ActiveRecordError) + end end it 'raises RefreshError' do diff --git a/spec/services/notes/resolve_service_spec.rb b/spec/services/notes/resolve_service_spec.rb index 3f82e1dbdc0..c98384c226e 100644 --- a/spec/services/notes/resolve_service_spec.rb +++ b/spec/services/notes/resolve_service_spec.rb @@ -17,7 +17,9 @@ describe Notes::ResolveService do end it "sends notifications if all discussions are resolved" do - expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService).to receive(:execute).with(merge_request) + expect_next_instance_of(MergeRequests::ResolvedDiscussionNotificationService) do |instance| + expect(instance).to receive(:execute).with(merge_request) + end described_class.new(merge_request.project, user).execute(note) end diff --git a/spec/services/pages_domains/obtain_lets_encrypt_certificate_service_spec.rb b/spec/services/pages_domains/obtain_lets_encrypt_certificate_service_spec.rb index af79a42b611..9832ba95524 100644 --- a/spec/services/pages_domains/obtain_lets_encrypt_certificate_service_spec.rb +++ b/spec/services/pages_domains/obtain_lets_encrypt_certificate_service_spec.rb @@ -32,9 +32,9 @@ describe PagesDomains::ObtainLetsEncryptCertificateService do def stub_lets_encrypt_order(url, status) order = ::Gitlab::LetsEncrypt::Order.new(acme_order_double(status: status)) - allow_any_instance_of(::Gitlab::LetsEncrypt::Client).to( - receive(:load_order).with(url).and_return(order) - ) + allow_next_instance_of(::Gitlab::LetsEncrypt::Client) do |instance| + allow(instance).to receive(:load_order).with(url).and_return(order) + end order end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 814bf912c8c..bce3f72a287 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -247,7 +247,9 @@ describe Projects::CreateService, '#execute' do context 'repository creation' do it 'synchronously creates the repository' do - expect_any_instance_of(Project).to receive(:create_repository) + expect_next_instance_of(Project) do |instance| + expect(instance).to receive(:create_repository) + end project = create_project(user, opts) expect(project).to be_valid diff --git a/spec/services/projects/import_export/export_service_spec.rb b/spec/services/projects/import_export/export_service_spec.rb index a557e61da78..c7ac07fc524 100644 --- a/spec/services/projects/import_export/export_service_spec.rb +++ b/spec/services/projects/import_export/export_service_spec.rb @@ -94,7 +94,9 @@ describe Projects::ImportExport::ExportService do end it 'notifies the user' do - expect_any_instance_of(NotificationService).to receive(:project_not_exported) + expect_next_instance_of(NotificationService) do |instance| + expect(instance).to receive(:project_not_exported) + end end it 'notifies logger' do @@ -122,7 +124,9 @@ describe Projects::ImportExport::ExportService do end it 'notifies the user' do - expect_any_instance_of(NotificationService).to receive(:project_not_exported) + expect_next_instance_of(NotificationService) do |instance| + expect(instance).to receive(:project_not_exported) + end end it 'notifies logger' do diff --git a/spec/services/projects/lfs_pointers/lfs_import_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_import_service_spec.rb index 7ca20a6d751..016028a96bf 100644 --- a/spec/services/projects/lfs_pointers/lfs_import_service_spec.rb +++ b/spec/services/projects/lfs_pointers/lfs_import_service_spec.rb @@ -16,7 +16,9 @@ describe Projects::LfsPointers::LfsImportService do it 'downloads lfs objects' do service = double - expect_any_instance_of(Projects::LfsPointers::LfsObjectDownloadListService).to receive(:execute).and_return(oid_download_links) + expect_next_instance_of(Projects::LfsPointers::LfsObjectDownloadListService) do |instance| + expect(instance).to receive(:execute).and_return(oid_download_links) + end expect(Projects::LfsPointers::LfsDownloadService).to receive(:new).and_return(service).twice expect(service).to receive(:execute).twice @@ -27,7 +29,9 @@ describe Projects::LfsPointers::LfsImportService do context 'when no downloadable lfs object links' do it 'does not call LfsDownloadService' do - expect_any_instance_of(Projects::LfsPointers::LfsObjectDownloadListService).to receive(:execute).and_return({}) + expect_next_instance_of(Projects::LfsPointers::LfsObjectDownloadListService) do |instance| + expect(instance).to receive(:execute).and_return({}) + end expect(Projects::LfsPointers::LfsDownloadService).not_to receive(:new) result = subject.execute @@ -39,7 +43,9 @@ describe Projects::LfsPointers::LfsImportService do context 'when an exception is raised' do it 'returns error' do error_message = "error message" - expect_any_instance_of(Projects::LfsPointers::LfsObjectDownloadListService).to receive(:execute).and_raise(StandardError, error_message) + expect_next_instance_of(Projects::LfsPointers::LfsObjectDownloadListService) do |instance| + expect(instance).to receive(:execute).and_raise(StandardError, error_message) + end result = subject.execute diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index 001c6670f4b..714256d9b08 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -110,8 +110,9 @@ describe Projects::UpdatePagesService do context 'when timeout happens by DNS error' do before do - allow_any_instance_of(described_class) - .to receive(:extract_zip_archive!).and_raise(SocketError) + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:extract_zip_archive!).and_raise(SocketError) + end end it 'raises an error' do @@ -125,9 +126,10 @@ describe Projects::UpdatePagesService do context 'when failed to extract zip artifacts' do before do - expect_any_instance_of(described_class) - .to receive(:extract_zip_archive!) - .and_raise(Projects::UpdatePagesService::FailedToExtractError) + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:extract_zip_archive!) + .and_raise(Projects::UpdatePagesService::FailedToExtractError) + end end it 'raises an error' do diff --git a/spec/services/system_notes/issuables_service_spec.rb b/spec/services/system_notes/issuables_service_spec.rb index c2f627c681b..56ef0039b63 100644 --- a/spec/services/system_notes/issuables_service_spec.rb +++ b/spec/services/system_notes/issuables_service_spec.rb @@ -265,7 +265,9 @@ describe ::SystemNotes::IssuablesService do context 'when cross-reference disallowed' do before do - expect_any_instance_of(described_class).to receive(:cross_reference_disallowed?).and_return(true) + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:cross_reference_disallowed?).and_return(true) + end end it 'returns nil' do @@ -279,7 +281,9 @@ describe ::SystemNotes::IssuablesService do context 'when cross-reference allowed' do before do - expect_any_instance_of(described_class).to receive(:cross_reference_disallowed?).and_return(false) + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:cross_reference_disallowed?).and_return(false) + end end it_behaves_like 'a system note' do diff --git a/spec/support/helpers/filter_spec_helper.rb b/spec/support/helpers/filter_spec_helper.rb index 95c24d76dcd..45d49696e06 100644 --- a/spec/support/helpers/filter_spec_helper.rb +++ b/spec/support/helpers/filter_spec_helper.rb @@ -28,6 +28,17 @@ module FilterSpecHelper described_class.call(html, context) end + # Get an instance of the Filter class + # + # Use this for testing instance methods, but remember to test the result of + # the full pipeline by calling #call using the other methods in this helper. + def filter_instance + render_context = Banzai::RenderContext.new(project, current_user) + context = { project: project, current_user: current_user, render_context: render_context } + + described_class.new(input_text, context) + end + # Run text through HTML::Pipeline with the current filter and return the # result Hash # diff --git a/spec/support/import_export/common_util.rb b/spec/support/import_export/common_util.rb index 4e149c9fa54..72baec7bfcb 100644 --- a/spec/support/import_export/common_util.rb +++ b/spec/support/import_export/common_util.rb @@ -3,7 +3,9 @@ module ImportExport module CommonUtil def setup_symlink(tmpdir, symlink_name) - allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(tmpdir) + allow_next_instance_of(Gitlab::ImportExport) do |instance| + allow(instance).to receive(:storage_path).and_return(tmpdir) + end File.open("#{tmpdir}/test", 'w') { |file| file.write("test") } FileUtils.ln_s("#{tmpdir}/test", "#{tmpdir}/#{symlink_name}") diff --git a/spec/support/shared_examples/lib/gitlab/import_export/import_failure_service_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/import_export/import_failure_service_shared_examples.rb new file mode 100644 index 00000000000..691564120cc --- /dev/null +++ b/spec/support/shared_examples/lib/gitlab/import_export/import_failure_service_shared_examples.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +shared_examples 'log import failure' do |importable_column| + it 'tracks error' do + extra = { + relation_key: relation_key, + relation_index: relation_index, + retry_count: retry_count + } + extra[importable_column] = importable.id + + expect(Gitlab::ErrorTracking).to receive(:track_exception).with(exception, extra) + + subject.log_import_failure(relation_key, relation_index, exception, retry_count) + end + + it 'saves data to ImportFailure' do + log_import_failure + + import_failure = ImportFailure.last + + aggregate_failures do + expect(import_failure[importable_column]).to eq(importable.id) + expect(import_failure.relation_key).to eq(relation_key) + expect(import_failure.relation_index).to eq(relation_index) + expect(import_failure.exception_class).to eq('StandardError') + expect(import_failure.exception_message).to eq(standard_error_message) + expect(import_failure.correlation_id_value).to eq(correlation_id) + expect(import_failure.retry_count).to eq(retry_count) + end + end +end diff --git a/spec/workers/ci/archive_traces_cron_worker_spec.rb b/spec/workers/ci/archive_traces_cron_worker_spec.rb index fc700c15b10..789e83783bb 100644 --- a/spec/workers/ci/archive_traces_cron_worker_spec.rb +++ b/spec/workers/ci/archive_traces_cron_worker_spec.rb @@ -35,8 +35,9 @@ describe Ci::ArchiveTracesCronWorker do it_behaves_like 'archives trace' it 'executes service' do - expect_any_instance_of(Ci::ArchiveTraceService) - .to receive(:execute).with(build, anything) + expect_next_instance_of(Ci::ArchiveTraceService) do |instance| + expect(instance).to receive(:execute).with(build, anything) + end subject end @@ -64,7 +65,9 @@ describe Ci::ArchiveTracesCronWorker do before do allow(Gitlab::ErrorTracking).to receive(:track_and_raise_for_dev_exception) - allow_any_instance_of(Gitlab::Ci::Trace).to receive(:archive!).and_raise('Unexpected error') + allow_next_instance_of(Gitlab::Ci::Trace) do |instance| + allow(instance).to receive(:archive!).and_raise('Unexpected error') + end end it 'puts a log' do diff --git a/spec/workers/concerns/gitlab/github_import/rescheduling_methods_spec.rb b/spec/workers/concerns/gitlab/github_import/rescheduling_methods_spec.rb index 294eacf09ab..c4f6ddf9aca 100644 --- a/spec/workers/concerns/gitlab/github_import/rescheduling_methods_spec.rb +++ b/spec/workers/concerns/gitlab/github_import/rescheduling_methods_spec.rb @@ -57,9 +57,9 @@ describe Gitlab::GithubImport::ReschedulingMethods do expect(worker) .not_to receive(:notify_waiter) - expect_any_instance_of(Gitlab::GithubImport::Client) - .to receive(:rate_limit_resets_in) - .and_return(14) + expect_next_instance_of(Gitlab::GithubImport::Client) do |instance| + expect(instance).to receive(:rate_limit_resets_in).and_return(14) + end expect(worker.class) .to receive(:perform_in) diff --git a/spec/workers/delete_merged_branches_worker_spec.rb b/spec/workers/delete_merged_branches_worker_spec.rb index 8c983859e36..3eaeb7e0797 100644 --- a/spec/workers/delete_merged_branches_worker_spec.rb +++ b/spec/workers/delete_merged_branches_worker_spec.rb @@ -9,7 +9,9 @@ describe DeleteMergedBranchesWorker do describe "#perform" do it "delegates to Branches::DeleteMergedService" do - expect_any_instance_of(::Branches::DeleteMergedService).to receive(:execute).and_return(true) + expect_next_instance_of(::Branches::DeleteMergedService) do |instance| + expect(instance).to receive(:execute).and_return(true) + end worker.perform(project.id, project.owner.id) end diff --git a/spec/workers/expire_build_artifacts_worker_spec.rb b/spec/workers/expire_build_artifacts_worker_spec.rb index 0a0aea838d2..06561e94fb7 100644 --- a/spec/workers/expire_build_artifacts_worker_spec.rb +++ b/spec/workers/expire_build_artifacts_worker_spec.rb @@ -7,7 +7,9 @@ describe ExpireBuildArtifactsWorker do describe '#perform' do it 'executes a service' do - expect_any_instance_of(Ci::DestroyExpiredJobArtifactsService).to receive(:execute) + expect_next_instance_of(Ci::DestroyExpiredJobArtifactsService) do |instance| + expect(instance).to receive(:execute) + end worker.perform end diff --git a/spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb b/spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb index 6d47d73b92e..3a8fe73622a 100644 --- a/spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb +++ b/spec/workers/gitlab/github_import/stage/import_repository_worker_spec.rb @@ -21,9 +21,9 @@ describe Gitlab::GithubImport::Stage::ImportRepositoryWorker do it 'schedules the importing of the base data' do client = double(:client) - expect_any_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) - .to receive(:execute) - .and_return(true) + expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |instance| + expect(instance).to receive(:execute).and_return(true) + end expect(Gitlab::GithubImport::Stage::ImportBaseDataWorker) .to receive(:perform_async) @@ -37,9 +37,9 @@ describe Gitlab::GithubImport::Stage::ImportRepositoryWorker do it 'does not schedule the importing of the base data' do client = double(:client) - expect_any_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) - .to receive(:execute) - .and_return(false) + expect_next_instance_of(Gitlab::GithubImport::Importer::RepositoryImporter) do |instance| + expect(instance).to receive(:execute).and_return(false) + end expect(Gitlab::GithubImport::Stage::ImportBaseDataWorker) .not_to receive(:perform_async) diff --git a/spec/workers/gitlab_shell_worker_spec.rb b/spec/workers/gitlab_shell_worker_spec.rb index 0758cfc4ee2..5dedf5be9fa 100644 --- a/spec/workers/gitlab_shell_worker_spec.rb +++ b/spec/workers/gitlab_shell_worker_spec.rb @@ -7,7 +7,9 @@ describe GitlabShellWorker do describe '#perform with add_key' do it 'calls add_key on Gitlab::Shell' do - expect_any_instance_of(Gitlab::Shell).to receive(:add_key).with('foo', 'bar') + expect_next_instance_of(Gitlab::Shell) do |instance| + expect(instance).to receive(:add_key).with('foo', 'bar') + end worker.perform(:add_key, 'foo', 'bar') end end diff --git a/spec/workers/gitlab_usage_ping_worker_spec.rb b/spec/workers/gitlab_usage_ping_worker_spec.rb index aff5d112cdd..198daf40493 100644 --- a/spec/workers/gitlab_usage_ping_worker_spec.rb +++ b/spec/workers/gitlab_usage_ping_worker_spec.rb @@ -8,7 +8,9 @@ describe GitlabUsagePingWorker do it 'delegates to SubmitUsagePingService' do allow(subject).to receive(:try_obtain_lease).and_return(true) - expect_any_instance_of(SubmitUsagePingService).to receive(:execute) + expect_next_instance_of(SubmitUsagePingService) do |instance| + expect(instance).to receive(:execute) + end subject.perform end diff --git a/spec/workers/hashed_storage/migrator_worker_spec.rb b/spec/workers/hashed_storage/migrator_worker_spec.rb index 9180da87058..ac76a306f43 100644 --- a/spec/workers/hashed_storage/migrator_worker_spec.rb +++ b/spec/workers/hashed_storage/migrator_worker_spec.rb @@ -10,7 +10,9 @@ describe HashedStorage::MigratorWorker do describe '#perform' do it 'delegates to MigratorService' do - expect_any_instance_of(Gitlab::HashedStorage::Migrator).to receive(:bulk_migrate).with(start: 5, finish: 10) + expect_next_instance_of(Gitlab::HashedStorage::Migrator) do |instance| + expect(instance).to receive(:bulk_migrate).with(start: 5, finish: 10) + end worker.perform(5, 10) end diff --git a/spec/workers/hashed_storage/rollbacker_worker_spec.rb b/spec/workers/hashed_storage/rollbacker_worker_spec.rb index 3ca2601df0f..55fc4fb0fe1 100644 --- a/spec/workers/hashed_storage/rollbacker_worker_spec.rb +++ b/spec/workers/hashed_storage/rollbacker_worker_spec.rb @@ -10,7 +10,9 @@ describe HashedStorage::RollbackerWorker do describe '#perform' do it 'delegates to MigratorService' do - expect_any_instance_of(Gitlab::HashedStorage::Migrator).to receive(:bulk_rollback).with(start: 5, finish: 10) + expect_next_instance_of(Gitlab::HashedStorage::Migrator) do |instance| + expect(instance).to receive(:bulk_rollback).with(start: 5, finish: 10) + end worker.perform(5, 10) end diff --git a/spec/workers/import_issues_csv_worker_spec.rb b/spec/workers/import_issues_csv_worker_spec.rb index 89370c4890d..03944cfb05d 100644 --- a/spec/workers/import_issues_csv_worker_spec.rb +++ b/spec/workers/import_issues_csv_worker_spec.rb @@ -11,7 +11,9 @@ describe ImportIssuesCsvWorker do describe '#perform' do it 'calls #execute on Issues::ImportCsvService and destroys upload' do - expect_any_instance_of(Issues::ImportCsvService).to receive(:execute).and_return({ success: 5, errors: [], valid_file: true }) + expect_next_instance_of(Issues::ImportCsvService) do |instance| + expect(instance).to receive(:execute).and_return({ success: 5, errors: [], valid_file: true }) + end worker.perform(user.id, project.id, upload.id) diff --git a/spec/workers/new_release_worker_spec.rb b/spec/workers/new_release_worker_spec.rb index 9010c36f795..9d8c5bbf919 100644 --- a/spec/workers/new_release_worker_spec.rb +++ b/spec/workers/new_release_worker_spec.rb @@ -6,7 +6,9 @@ describe NewReleaseWorker do let(:release) { create(:release) } it 'sends a new release notification' do - expect_any_instance_of(NotificationService).to receive(:send_new_release_notifications).with(release) + expect_next_instance_of(NotificationService) do |instance| + expect(instance).to receive(:send_new_release_notifications).with(release) + end described_class.new.perform(release.id) end diff --git a/spec/workers/repository_import_worker_spec.rb b/spec/workers/repository_import_worker_spec.rb index b8767af8eee..507098582c9 100644 --- a/spec/workers/repository_import_worker_spec.rb +++ b/spec/workers/repository_import_worker_spec.rb @@ -21,8 +21,9 @@ describe RepositoryImportWorker do allow(subject).to receive(:jid).and_return(jid) - expect_any_instance_of(Projects::ImportService).to receive(:execute) - .and_return({ status: :ok }) + expect_next_instance_of(Projects::ImportService) do |instance| + expect(instance).to receive(:execute).and_return({ status: :ok }) + end # Works around https://github.com/rspec/rspec-mocks/issues/910 expect(Project).to receive(:find).with(started_project.id).and_return(started_project) @@ -36,8 +37,9 @@ describe RepositoryImportWorker do context 'when the import was successful' do it 'imports a project' do - expect_any_instance_of(Projects::ImportService).to receive(:execute) - .and_return({ status: :ok }) + expect_next_instance_of(Projects::ImportService) do |instance| + expect(instance).to receive(:execute).and_return({ status: :ok }) + end # Works around https://github.com/rspec/rspec-mocks/issues/910 expect(Project).to receive(:find).with(project.id).and_return(project) @@ -54,7 +56,9 @@ describe RepositoryImportWorker do error = %q{remote: Not Found fatal: repository 'https://user:pass@test.com/root/repoC.git/' not found } import_state.update(jid: '123') - expect_any_instance_of(Projects::ImportService).to receive(:execute).and_return({ status: :error, message: error }) + expect_next_instance_of(Projects::ImportService) do |instance| + expect(instance).to receive(:execute).and_return({ status: :error, message: error }) + end expect do subject.perform(project.id) @@ -67,7 +71,9 @@ describe RepositoryImportWorker do project.update(import_type: 'gitlab_project') import_state.update(jid: '123') - expect_any_instance_of(Projects::ImportService).to receive(:execute).and_return({ status: :error, message: error }) + expect_next_instance_of(Projects::ImportService) do |instance| + expect(instance).to receive(:execute).and_return({ status: :error, message: error }) + end expect do subject.perform(project.id) @@ -93,8 +99,9 @@ describe RepositoryImportWorker do .to receive(:async?) .and_return(true) - expect_any_instance_of(ProjectImportState) - .not_to receive(:finish) + expect_next_instance_of(ProjectImportState) do |instance| + expect(instance).not_to receive(:finish) + end subject.perform(project.id) end |