diff options
31 files changed, 217 insertions, 177 deletions
diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index dc09d0499ff..e5f6baa1623 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -36,7 +36,7 @@ /ee/app/finders/ @gitlab-org/maintainers/database # Feature specific owners -/ee/lib/gitlab/code_owners/ @reprazent +/ee/lib/gitlab/code_owners/ @reprazent @kerrizor /ee/lib/ee/gitlab/auth/ldap/ @dblessing @mkozono /lib/gitlab/auth/ldap/ @dblessing @mkozono /lib/gitlab/ci/templates/ @nolith @zj diff --git a/app/assets/javascripts/behaviors/markdown/render_mermaid.js b/app/assets/javascripts/behaviors/markdown/render_mermaid.js index e9f563d3781..057cdb6cc4c 100644 --- a/app/assets/javascripts/behaviors/markdown/render_mermaid.js +++ b/app/assets/javascripts/behaviors/markdown/render_mermaid.js @@ -28,7 +28,6 @@ function importMermaidModule() { if ( window.gon?.user_color_scheme === 'dark' && - window.gon?.features?.webideDarkTheme && // if on the Web IDE page document.querySelector('.ide') ) { diff --git a/app/assets/javascripts/ide/components/ide.vue b/app/assets/javascripts/ide/components/ide.vue index 40d36063391..998d3f2d8ea 100644 --- a/app/assets/javascripts/ide/components/ide.vue +++ b/app/assets/javascripts/ide/components/ide.vue @@ -57,7 +57,7 @@ export default { 'editorTheme', ]), themeName() { - return this.glFeatures.webideDarkTheme && window.gon?.user_color_scheme; + return window.gon?.user_color_scheme; }, }, mounted() { diff --git a/app/controllers/ide_controller.rb b/app/controllers/ide_controller.rb index bffbdf01f8f..8a838db04f9 100644 --- a/app/controllers/ide_controller.rb +++ b/app/controllers/ide_controller.rb @@ -6,10 +6,6 @@ class IdeController < ApplicationController include ClientsidePreviewCSP include StaticObjectExternalStorageCSP - before_action do - push_frontend_feature_flag(:webide_dark_theme) - end - def index Gitlab::UsageDataCounters::WebIdeCounter.increment_views_count end diff --git a/app/models/project_ci_cd_setting.rb b/app/models/project_ci_cd_setting.rb index 39e177e8bd8..c295837002a 100644 --- a/app/models/project_ci_cd_setting.rb +++ b/app/models/project_ci_cd_setting.rb @@ -37,8 +37,6 @@ class ProjectCiCdSetting < ApplicationRecord private def set_default_git_depth - return unless Feature.enabled?(:ci_set_project_default_git_depth, default_enabled: true) - self.default_git_depth ||= DEFAULT_GIT_DEPTH end end diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index fb59797a8df..2d8a29cda1e 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -85,8 +85,6 @@ module Ci # to make sure that this is properly handled by runner. Result.new(nil, false) rescue => ex - raise ex unless Feature.enabled?(:ci_doom_build, default_enabled: true) - scheduler_failure!(build) track_exception_for_build(ex, build) diff --git a/app/workers/gitlab/jira_import/import_issue_worker.rb b/app/workers/gitlab/jira_import/import_issue_worker.rb index b3031256445..78de5cf1307 100644 --- a/app/workers/gitlab/jira_import/import_issue_worker.rb +++ b/app/workers/gitlab/jira_import/import_issue_worker.rb @@ -29,9 +29,11 @@ module Gitlab def create_issue(issue_attributes, project_id) label_ids = issue_attributes.delete('label_ids') + assignee_ids = issue_attributes.delete('assignee_ids') issue_id = insert_and_return_id(issue_attributes, Issue) label_issue(project_id, issue_id, label_ids) + assign_issue(project_id, issue_id, assignee_ids) issue_id end @@ -49,6 +51,14 @@ module Gitlab Gitlab::Database.bulk_insert(LabelLink.table_name, label_link_attrs) end + def assign_issue(project_id, issue_id, assignee_ids) + return if assignee_ids.blank? + + assignee_attrs = assignee_ids.map { |user_id| { issue_id: issue_id, user_id: user_id } } + + Gitlab::Database.bulk_insert(IssueAssignee.table_name, assignee_attrs) + end + def build_label_attrs(issue_id, label_id) time = Time.now { diff --git a/db/migrate/20200215222507_drop_forked_project_links_fk.rb b/db/migrate/20200215222507_drop_forked_project_links_fk.rb index 226e3fbcf85..7fe38a6ccdb 100644 --- a/db/migrate/20200215222507_drop_forked_project_links_fk.rb +++ b/db/migrate/20200215222507_drop_forked_project_links_fk.rb @@ -8,20 +8,16 @@ class DropForkedProjectLinksFk < ActiveRecord::Migration[6.0] disable_ddl_transaction! def up - # rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction with_lock_retries do remove_foreign_key_if_exists :forked_project_links, column: :forked_to_project_id end - # rubocop: enable Migration/WithLockRetriesWithoutDdlTransaction end def down unless foreign_key_exists?(:forked_project_links, :projects, column: :forked_to_project_id) - # rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction with_lock_retries do add_foreign_key :forked_project_links, :projects, column: :forked_to_project_id, on_delete: :cascade, validate: false end - # rubocop: enable Migration/WithLockRetriesWithoutDdlTransaction end fk_name = concurrent_foreign_key_name(:forked_project_links, :forked_to_project_id, prefix: 'fk_rails_') diff --git a/db/migrate/20200304024025_add_sprint_id_index_to_issues.rb b/db/migrate/20200304024025_add_sprint_id_index_to_issues.rb index 1782841f82e..aaece3445db 100644 --- a/db/migrate/20200304024025_add_sprint_id_index_to_issues.rb +++ b/db/migrate/20200304024025_add_sprint_id_index_to_issues.rb @@ -13,7 +13,7 @@ class AddSprintIdIndexToIssues < ActiveRecord::Migration[6.0] end def down - with_lock_retries do # rubocop:disable Migration/WithLockRetriesWithoutDdlTransaction + with_lock_retries do remove_foreign_key :issues, column: :sprint_id end remove_concurrent_index :issues, :sprint_id diff --git a/db/migrate/20200304024042_add_sprint_id_index_to_merge_requests.rb b/db/migrate/20200304024042_add_sprint_id_index_to_merge_requests.rb index 44fbab4b96a..219d10ff47c 100644 --- a/db/migrate/20200304024042_add_sprint_id_index_to_merge_requests.rb +++ b/db/migrate/20200304024042_add_sprint_id_index_to_merge_requests.rb @@ -13,7 +13,7 @@ class AddSprintIdIndexToMergeRequests < ActiveRecord::Migration[6.0] end def down - with_lock_retries do # rubocop:disable Migration/WithLockRetriesWithoutDdlTransaction + with_lock_retries do remove_foreign_key :merge_requests, column: :sprint_id end remove_concurrent_index :merge_requests, :sprint_id diff --git a/db/migrate/20200407171133_add_protected_tag_create_access_levels_user_id_foreign_key.rb b/db/migrate/20200407171133_add_protected_tag_create_access_levels_user_id_foreign_key.rb index 3d41707face..79532b8179c 100644 --- a/db/migrate/20200407171133_add_protected_tag_create_access_levels_user_id_foreign_key.rb +++ b/db/migrate/20200407171133_add_protected_tag_create_access_levels_user_id_foreign_key.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction class AddProtectedTagCreateAccessLevelsUserIdForeignKey < ActiveRecord::Migration[6.0] include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20200408154331_add_protected_branch_merge_access_levels_user_id_foreign_key.rb b/db/migrate/20200408154331_add_protected_branch_merge_access_levels_user_id_foreign_key.rb index 943aea902fa..0c5118c162b 100644 --- a/db/migrate/20200408154331_add_protected_branch_merge_access_levels_user_id_foreign_key.rb +++ b/db/migrate/20200408154331_add_protected_branch_merge_access_levels_user_id_foreign_key.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction class AddProtectedBranchMergeAccessLevelsUserIdForeignKey < ActiveRecord::Migration[6.0] include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20200408154411_add_path_locks_user_id_foreign_key.rb b/db/migrate/20200408154411_add_path_locks_user_id_foreign_key.rb index 65e857ff88f..aae79c175bc 100644 --- a/db/migrate/20200408154411_add_path_locks_user_id_foreign_key.rb +++ b/db/migrate/20200408154411_add_path_locks_user_id_foreign_key.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction class AddPathLocksUserIdForeignKey < ActiveRecord::Migration[6.0] include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20200408154455_add_protected_branch_push_access_levels_user_id_foreign_key.rb b/db/migrate/20200408154455_add_protected_branch_push_access_levels_user_id_foreign_key.rb index 01676156fb5..1cfe240df05 100644 --- a/db/migrate/20200408154455_add_protected_branch_push_access_levels_user_id_foreign_key.rb +++ b/db/migrate/20200408154455_add_protected_branch_push_access_levels_user_id_foreign_key.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction class AddProtectedBranchPushAccessLevelsUserIdForeignKey < ActiveRecord::Migration[6.0] include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20200408154604_add_u2f_registrations_user_id_foreign_key.rb b/db/migrate/20200408154604_add_u2f_registrations_user_id_foreign_key.rb index 74d826adc88..cdb81d73c99 100644 --- a/db/migrate/20200408154604_add_u2f_registrations_user_id_foreign_key.rb +++ b/db/migrate/20200408154604_add_u2f_registrations_user_id_foreign_key.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction class AddU2fRegistrationsUserIdForeignKey < ActiveRecord::Migration[6.0] include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20200420172752_add_sprints_foreign_key_to_projects.rb b/db/migrate/20200420172752_add_sprints_foreign_key_to_projects.rb index 8d1af8f98c7..46b5c9c2ac2 100644 --- a/db/migrate/20200420172752_add_sprints_foreign_key_to_projects.rb +++ b/db/migrate/20200420172752_add_sprints_foreign_key_to_projects.rb @@ -12,7 +12,7 @@ class AddSprintsForeignKeyToProjects < ActiveRecord::Migration[6.0] end def down - with_lock_retries do # rubocop:disable Migration/WithLockRetriesWithoutDdlTransaction + with_lock_retries do remove_foreign_key :sprints, column: :project_id end end diff --git a/db/migrate/20200420172927_add_sprints_foreign_key_to_groups.rb b/db/migrate/20200420172927_add_sprints_foreign_key_to_groups.rb index 81b9805b874..cd90fe50cde 100644 --- a/db/migrate/20200420172927_add_sprints_foreign_key_to_groups.rb +++ b/db/migrate/20200420172927_add_sprints_foreign_key_to_groups.rb @@ -12,7 +12,7 @@ class AddSprintsForeignKeyToGroups < ActiveRecord::Migration[6.0] end def down - with_lock_retries do # rubocop:disable Migration/WithLockRetriesWithoutDdlTransaction + with_lock_retries do remove_foreign_key :sprints, column: :group_id end end diff --git a/db/migrate/20200423081441_add_foreign_key_from_users_to_metrics_users_starred_dashboars.rb b/db/migrate/20200423081441_add_foreign_key_from_users_to_metrics_users_starred_dashboars.rb index c30e753be62..c3a7635193c 100644 --- a/db/migrate/20200423081441_add_foreign_key_from_users_to_metrics_users_starred_dashboars.rb +++ b/db/migrate/20200423081441_add_foreign_key_from_users_to_metrics_users_starred_dashboars.rb @@ -11,7 +11,7 @@ class AddForeignKeyFromUsersToMetricsUsersStarredDashboars < ActiveRecord::Migra end def down - with_lock_retries do # rubocop:disable Migration/WithLockRetriesWithoutDdlTransaction + with_lock_retries do remove_foreign_key_if_exists :metrics_users_starred_dashboards, column: :user_id end end diff --git a/db/migrate/20200423081519_add_foreign_key_from_projects_to_metrics_users_starred_dashboars.rb b/db/migrate/20200423081519_add_foreign_key_from_projects_to_metrics_users_starred_dashboars.rb index 4a4239d7b8c..88565c24eca 100644 --- a/db/migrate/20200423081519_add_foreign_key_from_projects_to_metrics_users_starred_dashboars.rb +++ b/db/migrate/20200423081519_add_foreign_key_from_projects_to_metrics_users_starred_dashboars.rb @@ -11,7 +11,7 @@ class AddForeignKeyFromProjectsToMetricsUsersStarredDashboars < ActiveRecord::Mi end def down - with_lock_retries do # rubocop:disable Migration/WithLockRetriesWithoutDdlTransaction + with_lock_retries do remove_foreign_key_if_exists :metrics_users_starred_dashboards, column: :project_id end end diff --git a/db/post_migrate/20200424043515_drop_namespaces_plan_id.rb b/db/post_migrate/20200424043515_drop_namespaces_plan_id.rb index 53ce13cc699..16a56b16e5a 100644 --- a/db/post_migrate/20200424043515_drop_namespaces_plan_id.rb +++ b/db/post_migrate/20200424043515_drop_namespaces_plan_id.rb @@ -8,14 +8,14 @@ class DropNamespacesPlanId < ActiveRecord::Migration[6.0] disable_ddl_transaction! def up - with_lock_retries do # rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction + with_lock_retries do remove_column :namespaces, :plan_id end end def down unless column_exists?(:namespaces, :plan_id) - with_lock_retries do # rubocop: disable Migration/WithLockRetriesWithoutDdlTransaction + with_lock_retries do add_column :namespaces, :plan_id, :integer end end diff --git a/doc/development/code_review.md b/doc/development/code_review.md index d544fdef3f8..8ea7e35dfb1 100644 --- a/doc/development/code_review.md +++ b/doc/development/code_review.md @@ -319,7 +319,7 @@ Before taking the decision to merge: - Set the milestone. - Consider warnings and errors from danger bot, code quality, and other reports. Unless a strong case can be made for the violation, these should be resolved - before merge. A comment must to be posted if the MR is merged with any failed job. + before merging. A comment must to be posted if the MR is merged with any failed job. When ready to merge: diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index 07652707382..e8a32ad3be0 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -327,6 +327,34 @@ def down end ``` +**Usage with `disable_ddl_transaction!`** + +Generally the `with_lock_retries` helper should work with `disabled_ddl_transaction!`. A custom RuboCop rule ensures that only allowed methods can be placed within the lock retries block. + +```ruby +disable_ddl_transaction! + +def up + with_lock_retries do + add_column :users, :name, :text + end + + add_text_limit :users, :name, 255 # Includes constraint validation (full table scan) +end +``` + +The RuboCop rule generally allows standard Rails migration methods, listed below. This example will cause a rubocop offense: + +```ruby +disabled_ddl_transaction! + +def up + with_lock_retries do + add_concurrent_index :users, :name + end +end +``` + ### When to use the helper method The `with_lock_retries` helper method can be used when you normally use @@ -350,8 +378,6 @@ Example changes: - `change_column_default` - `create_table` / `drop_table` -**Note:** `with_lock_retries` method **cannot** be used with `disable_ddl_transaction!`. - **Note:** `with_lock_retries` method **cannot** be used within the `change` method, you must manually define the `up` and `down` methods to make the migration reversible. ### How the helper method works diff --git a/lib/gitlab/ci/config/entry/trigger.rb b/lib/gitlab/ci/config/entry/trigger.rb index 7202784842a..c6ba53adfd7 100644 --- a/lib/gitlab/ci/config/entry/trigger.rb +++ b/lib/gitlab/ci/config/entry/trigger.rb @@ -25,8 +25,7 @@ module Gitlab strategy :CrossProjectTrigger, if: -> (config) { !config.key?(:include) } strategy :SameProjectTrigger, if: -> (config) do - ::Feature.enabled?(:ci_parent_child_pipeline, default_enabled: true) && - config.key?(:include) + config.key?(:include) end class CrossProjectTrigger < ::Gitlab::Config::Entry::Node @@ -72,11 +71,7 @@ module Gitlab class UnknownStrategy < ::Gitlab::Config::Entry::Node def errors - if ::Feature.enabled?(:ci_parent_child_pipeline, default_enabled: true) - ['config must specify either project or include'] - else - ['config must specify project'] - end + ['config must specify either project or include'] end end end diff --git a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb new file mode 100644 index 00000000000..309ddcc9746 --- /dev/null +++ b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + class WithLockRetriesDisallowedMethod < RuboCop::Cop::Cop + include MigrationHelpers + + ALLOWED_MIGRATION_METHODS = %i[ + create_table + drop_table + add_foreign_key + remove_foreign_key + add_column + remove_column + execute + change_column_default + remove_foreign_key_if_exists + remove_foreign_key_without_error + table_exists? + index_exists_by_name? + foreign_key_exists? + index_exists? + column_exists? + ].sort.freeze + + MSG = "The method is not allowed to be called within the `with_lock_retries` block, the only allowed methods are: #{ALLOWED_MIGRATION_METHODS.join(', ')}" + + def_node_matcher :send_node?, <<~PATTERN + send + PATTERN + + def on_block(node) + block_body = node.body + + return unless in_migration?(node) + return unless block_body + return unless node.method_name == :with_lock_retries + + if send_node?(block_body) + check_node(block_body) + else + block_body.children.each { |n| check_node(n) } + end + end + + def check_node(node) + return unless send_node?(node) + + name = node.children[1] + add_offense(node, location: :expression) unless ALLOWED_MIGRATION_METHODS.include?(name) + end + end + end + end +end diff --git a/rubocop/cop/migration/with_lock_retries_without_ddl_transaction.rb b/rubocop/cop/migration/with_lock_retries_without_ddl_transaction.rb deleted file mode 100644 index ebd91dd5a6e..00000000000 --- a/rubocop/cop/migration/with_lock_retries_without_ddl_transaction.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -require_relative '../../migration_helpers' - -module RuboCop - module Cop - module Migration - # Cop that prevents usage of `with_lock_retries` with `disable_ddl_transaction!` - class WithLockRetriesWithoutDdlTransaction < RuboCop::Cop::Cop - include MigrationHelpers - - MSG = '`with_lock_retries` cannot be used with disabled DDL transactions (`disable_ddl_transaction!`). ' \ - 'Please remove the `disable_ddl_transaction!` call from your migration.'.freeze - - def_node_matcher :disable_ddl_transaction?, <<~PATTERN - (send _ :disable_ddl_transaction!) - PATTERN - - def_node_matcher :with_lock_retries?, <<~PATTERN - (send _ :with_lock_retries) - PATTERN - - def on_send(node) - return unless in_migration?(node) - return unless with_lock_retries?(node) - - node.each_ancestor(:begin) do |begin_node| - disable_ddl_transaction_node = begin_node.children.find { |n| disable_ddl_transaction?(n) } - - add_offense(node, location: :expression) if disable_ddl_transaction_node - end - end - end - end - end -end diff --git a/scripts/regenerate-schema b/scripts/regenerate-schema index b63a75cdc83..cedd612f766 100755 --- a/scripts/regenerate-schema +++ b/scripts/regenerate-schema @@ -2,7 +2,7 @@ # frozen_string_literal: true -require 'net/http' +require 'open3' require 'uri' class SchemaRegenerator @@ -56,35 +56,15 @@ class SchemaRegenerator # Get clean schema from remote servers # # This script might run in CI, using a shallow clone, so to checkout - # the file, download it from the server. + # the file, fetch the target branch from the server. def remote_checkout_clean_schema return false unless project_url + return false unless target_project_url - uri = URI.join("#{project_url}/", 'raw/', "#{merge_base}/", FILENAME) + run %Q[git remote add target_project #{target_project_url}.git] + run %Q[git fetch target_project #{target_branch}:#{target_branch}] - download_schema(uri) - end - - ## - # Download the schema from the given +uri+. - def download_schema(uri) - puts "Downloading #{uri}..." - - Net::HTTP.start(uri.host, uri.port, use_ssl: true) do |http| - request = Net::HTTP::Get.new(uri.request_uri) - http.read_timeout = 500 - http.request(request) do |response| - raise("Failed to download file: #{response.code} #{response.message}") if response.code.to_i != 200 - - File.open(FILENAME, 'w') do |io| - response.read_body do |chunk| - io.write(chunk) - end - end - end - end - - true + local_checkout_clean_schema end ## @@ -150,15 +130,17 @@ class SchemaRegenerator # When the command failed an exception is raised. def run(cmd) puts "\e[32m$ #{cmd}\e[37m" - ret = system(cmd) - puts "\e[0m" - raise("Command failed") unless ret + stdout_str, stderr_str, status = Open3.capture3(cmd) + puts "#{stdout_str}#{stderr_str}\e[0m" + raise("Command failed: #{stderr_str}") unless status.success? + + stdout_str end ## # Return the base commit between source and target branch. def merge_base - @merge_base ||= `git merge-base #{target_branch} #{source_ref}`.chomp + @merge_base ||= run("git merge-base #{target_branch} #{source_ref}").chomp end ## @@ -179,12 +161,18 @@ class SchemaRegenerator end ## - # Return the project URL from CI environment variable. + # Return the source project URL from CI environment variable. def project_url ENV['CI_PROJECT_URL'] end ## + # Return the target project URL from CI environment variable. + def target_project_url + ENV['CI_MERGE_REQUEST_PROJECT_URL'] + end + + ## # Return whether the script is running from CI def ci? ENV['CI'] diff --git a/spec/lib/gitlab/ci/config/entry/trigger_spec.rb b/spec/lib/gitlab/ci/config/entry/trigger_spec.rb index 752c3f59a95..dfd9807583c 100644 --- a/spec/lib/gitlab/ci/config/entry/trigger_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/trigger_spec.rb @@ -114,19 +114,6 @@ describe Gitlab::Ci::Config::Entry::Trigger do .to match /config contains unknown keys: branch/ end end - - context 'when feature flag is off' do - before do - stub_feature_flags(ci_parent_child_pipeline: false) - end - - let(:config) { { include: 'path/to/config.yml' } } - - it 'is returns an error if include is used' do - expect(subject.errors.first) - .to match /config must specify project/ - end - end end context 'when config contains unknown keys' do diff --git a/spec/models/project_ci_cd_setting_spec.rb b/spec/models/project_ci_cd_setting_spec.rb index 312cbbb0948..86115a61aa7 100644 --- a/spec/models/project_ci_cd_setting_spec.rb +++ b/spec/models/project_ci_cd_setting_spec.rb @@ -54,17 +54,5 @@ describe ProjectCiCdSetting do expect(project.reload.ci_cd_settings.default_git_depth).to eq(0) end - - context 'when feature flag :ci_set_project_default_git_depth is disabled' do - let(:project) { create(:project) } - - before do - stub_feature_flags(ci_set_project_default_git_depth: { enabled: false } ) - end - - it 'does not set default value for new records' do - expect(project.ci_cd_settings.default_git_depth).to eq(nil) - end - end end end diff --git a/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb b/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb new file mode 100644 index 00000000000..48570c1c8d8 --- /dev/null +++ b/spec/rubocop/cop/migration/with_lock_retries_disallowed_method_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/with_lock_retries_disallowed_method' + +describe RuboCop::Cop::Migration::WithLockRetriesDisallowedMethod do + include CopHelper + + subject(:cop) { described_class.new } + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + it 'registers an offense when `with_lock_retries` block has disallowed method' do + inspect_source('def change; with_lock_retries { disallowed_method }; end') + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end + + it 'registers an offense when `with_lock_retries` block has disallowed methods' do + source = <<~HEREDOC + def change + with_lock_retries do + disallowed_method + + create_table do |t| + t.text :text + end + + other_disallowed_method + + add_column :users, :name + end + end + HEREDOC + + inspect_source(source) + + aggregate_failures do + expect(cop.offenses.size).to eq(2) + expect(cop.offenses.map(&:line)).to eq([3, 9]) + end + end + + it 'registers no offense when `with_lock_retries` has only allowed method' do + inspect_source('def up; with_lock_retries { add_foreign_key :foo, :bar }; end') + + expect(cop.offenses.size).to eq(0) + end + end + + context 'outside of migration' do + it 'registers no offense' do + inspect_source('def change; with_lock_retries { disallowed_method }; end') + + expect(cop.offenses.size).to eq(0) + end + end +end diff --git a/spec/rubocop/cop/migration/with_lock_retries_without_ddl_transaction_spec.rb b/spec/rubocop/cop/migration/with_lock_retries_without_ddl_transaction_spec.rb deleted file mode 100644 index b42a4a14c67..00000000000 --- a/spec/rubocop/cop/migration/with_lock_retries_without_ddl_transaction_spec.rb +++ /dev/null @@ -1,46 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -require 'rubocop' -require 'rubocop/rspec/support' - -require_relative '../../../../rubocop/cop/migration/with_lock_retries_without_ddl_transaction' - -describe RuboCop::Cop::Migration::WithLockRetriesWithoutDdlTransaction do - include CopHelper - - let(:valid_source) { 'class MigrationClass < ActiveRecord::Migration[6.0]; def up; with_lock_retries {}; end; end' } - let(:invalid_source) { 'class MigrationClass < ActiveRecord::Migration[6.0]; disable_ddl_transaction!; def up; with_lock_retries {}; end; end' } - - subject(:cop) { described_class.new } - - context 'in migration' do - before do - allow(cop).to receive(:in_migration?).and_return(true) - end - - it 'registers an offense when `with_lock_retries` is used with `disable_ddl_transaction!` method' do - inspect_source(invalid_source) - - aggregate_failures do - expect(cop.offenses.size).to eq(1) - expect(cop.offenses.map(&:line)).to eq([1]) - end - end - - it 'registers no offense when `with_lock_retries` is used inside an `up` method' do - inspect_source(valid_source) - - expect(cop.offenses.size).to eq(0) - end - end - - context 'outside of migration' do - it 'registers no offense' do - inspect_source(invalid_source) - - expect(cop.offenses.size).to eq(0) - end - end -end diff --git a/spec/workers/gitlab/jira_import/import_issue_worker_spec.rb b/spec/workers/gitlab/jira_import/import_issue_worker_spec.rb index 36af65a3a06..2de609761e2 100644 --- a/spec/workers/gitlab/jira_import/import_issue_worker_spec.rb +++ b/spec/workers/gitlab/jira_import/import_issue_worker_spec.rb @@ -19,9 +19,12 @@ describe Gitlab::JiraImport::ImportIssueWorker do subject { described_class.new } describe '#perform', :clean_gitlab_redis_cache do + let(:assignee_ids) { [user.id] } let(:issue_attrs) do build(:issue, project_id: project.id, title: 'jira issue') - .as_json.merge('label_ids' => [jira_issue_label_1.id, jira_issue_label_2.id]).compact + .as_json.merge( + 'label_ids' => [jira_issue_label_1.id, jira_issue_label_2.id], 'assignee_ids' => assignee_ids + ).compact end context 'when any exception raised while inserting to DB' do @@ -67,6 +70,23 @@ describe Gitlab::JiraImport::ImportIssueWorker do expect(issue.title).to eq('jira issue') expect(issue.project).to eq(project) expect(issue.labels).to match_array([label, jira_issue_label_1, jira_issue_label_2]) + expect(issue.assignees).to eq([user]) + end + + context 'when assignee_ids is nil' do + let(:assignee_ids) { nil } + + it 'creates an issue without assignee' do + expect(Issue.last.assignees).to be_empty + end + end + + context 'when assignee_ids is an empty array' do + let(:assignee_ids) { [] } + + it 'creates an issue without assignee' do + expect(Issue.last.assignees).to be_empty + end end end end |