From 2502fd32ceb433781e9e85e353b370fc0b2f8dfa Mon Sep 17 00:00:00 2001 From: Jeremy Stanley Date: Fri, 6 Jan 2023 10:54:39 -0700 Subject: [PATCH] rubocop_canvas: fix add_reference non-transactional logic we don't need to suggest `disable_ddl_transaction!` when adding a non-indexed reference fixes FOO-3298 test plan: specs Change-Id: I33ca47954e5760bf3f14d8b240891713f735eeda Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/308301 Reviewed-by: Jacob Burroughs Tested-by: Service Cloud Jenkins QA-Review: Jeremy Stanley Product-Review: Jeremy Stanley --- .../lib/rubocop_canvas/cops/migration/add_index.rb | 9 +++++---- .../spec/rubocop/cop/migration/add_index_spec.rb | 1 - 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/add_index.rb b/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/add_index.rb index fc176f71755..6a92a77ab9d 100644 --- a/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/add_index.rb +++ b/gems/rubocop-canvas/lib/rubocop_canvas/cops/migration/add_index.rb @@ -104,11 +104,12 @@ module RuboCop end def check_add_reference(node) - check_non_transactional - arg = index_argument(node).first - if arg.nil? || (!false?(arg) && !algorithm_concurrently?(arg)) - add_offense arg || node, message: INDEX_ALGORITHM_CONCURRENTLY_MSG, severity: :warning + if arg.nil? || !false?(arg) + check_non_transactional + if arg.nil? || !algorithm_concurrently?(arg) + add_offense arg || node, message: INDEX_ALGORITHM_CONCURRENTLY_MSG, severity: :warning + end end end diff --git a/gems/rubocop-canvas/spec/rubocop/cop/migration/add_index_spec.rb b/gems/rubocop-canvas/spec/rubocop/cop/migration/add_index_spec.rb index 58597b99017..44e66180073 100644 --- a/gems/rubocop-canvas/spec/rubocop/cop/migration/add_index_spec.rb +++ b/gems/rubocop-canvas/spec/rubocop/cop/migration/add_index_spec.rb @@ -145,7 +145,6 @@ describe RuboCop::Cop::Migration::AddIndex do it "doesn't complain if not indexed" do inspect_source(<<~RUBY) class TestMigration < ActiveRecord::Migration - disable_ddl_transaction! def up add_reference :users, :organizations, index: false end