lib/strong_migrations/checker.rb in strong_migrations-0.4.2 vs lib/strong_migrations/checker.rb in strong_migrations-0.5.0

@@ -57,13 +57,23 @@
           options ||= {}
 
           if columns.is_a?(Array) && columns.size > 3 && !options[:unique]
             raise_error :add_index_columns, header: "Best practice"
           end
-          if postgresql? && options[:algorithm] != :concurrently && !@new_tables.include?(table.to_s)
+          if postgresql? && options[:algorithm] != :concurrently && !new_table?(table)
             raise_error :add_index, command: command_str("add_index", [table, columns, options.merge(algorithm: :concurrently)])
           end
+        when :remove_index
+          table, options = args
+          unless options.is_a?(Hash)
+            options = {column: options}
+          end
+          options ||= {}
+
+          if postgresql? && options[:algorithm] != :concurrently && !new_table?(table)
+            raise_error :remove_index, command: command_str("remove_index", [table, options.merge(algorithm: :concurrently)])
+          end
         when :add_column
           table, column, type, options = args
           options ||= {}
           default = options[:default]
 
@@ -71,17 +81,11 @@
 
             if options[:null] == false
               options = options.except(:null)
               append = "
 
-Then add the NOT NULL constraint.
-
-class %{migration_name}NotNull < ActiveRecord::Migration%{migration_suffix}
-  def change
-    #{command_str("change_column_null", [table, column, false])}
-  end
-end"
+Then add the NOT NULL constraint."
             end
 
             raise_error :add_column_default,
               add_command: command_str("add_column", [table, column, type, options.except(:default)]),
               change_command: command_str("change_column_default", [table, column, default]),
@@ -131,13 +135,22 @@
           end
         when :execute
           raise_error :execute, header: "Possibly dangerous operation"
         when :change_column_null
           table, column, null, default = args
-          if !null && !default.nil?
-            raise_error :change_column_null,
-              code: backfill_code(table, column, default)
+          if !null
+            if postgresql?
+              # match https://github.com/nullobject/rein
+              constraint_name = "#{table}_#{column}_null"
+
+              raise_error :change_column_null_postgresql,
+                add_constraint_code: constraint_str("ALTER TABLE %s ADD CONSTRAINT %s CHECK (%s IS NOT NULL) NOT VALID", [table, constraint_name, column]),
+                validate_constraint_code: constraint_str("ALTER TABLE %s VALIDATE CONSTRAINT %s", [table, constraint_name])
+            elsif !default.nil?
+              raise_error :change_column_null,
+                code: backfill_code(table, column, default)
+            end
           end
         when :add_foreign_key
           from_table, to_table, options = args
           options ||= {}
           validate = options.fetch(:validate, true)
@@ -157,12 +170,12 @@
               column = options[:column] || "#{to_table.to_s.singularize}_id"
               hashed_identifier = Digest::SHA256.hexdigest("#{from_table}_#{column}_fk").first(10)
               fk_name = options[:name] || "fk_rails_#{hashed_identifier}"
 
               raise_error :add_foreign_key,
-                add_foreign_key_code: foreign_key_str("ALTER TABLE %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s) NOT VALID", [from_table, fk_name, column, to_table, primary_key]),
-                validate_foreign_key_code: foreign_key_str("ALTER TABLE %s VALIDATE CONSTRAINT %s", [from_table, fk_name])
+                add_foreign_key_code: constraint_str("ALTER TABLE %s ADD CONSTRAINT %s FOREIGN KEY (%s) REFERENCES %s (%s) NOT VALID", [from_table, fk_name, column, to_table, primary_key]),
+                validate_foreign_key_code: constraint_str("ALTER TABLE %s VALIDATE CONSTRAINT %s", [from_table, fk_name])
             end
           end
         end
 
         StrongMigrations.checks.each do |check|
@@ -212,10 +225,12 @@
         end
       end
     end
 
     def raise_error(message_key, header: nil, **vars)
+      return unless StrongMigrations.check_enabled?(message_key, version: version)
+
       message = StrongMigrations.error_messages[message_key] || "Missing message"
 
       vars[:migration_name] = self.class.name
       vars[:migration_suffix] = "[#{ActiveRecord::VERSION::MAJOR}.#{ActiveRecord::VERSION::MINOR}]"
       vars[:base_model] = "ApplicationRecord"
@@ -227,11 +242,11 @@
 
       # escape % not followed by {
       @migration.stop!(message.gsub(/%(?!{)/, "%%") % vars, header: header || "Dangerous operation detected")
     end
 
-    def foreign_key_str(statement, identifiers)
+    def constraint_str(statement, identifiers)
       # not all identifiers are tables, but this method of quoting should be fine
       code = statement % identifiers.map { |v| connection.quote_table_name(v) }
       "safety_assured do\n      execute '#{code}' \n    end"
     end
 
@@ -259,8 +274,12 @@
     end
 
     def backfill_code(table, column, default)
       model = table.to_s.classify
       "#{model}.unscoped.in_batches do |relation| \n      relation.update_all #{column}: #{default.inspect}\n      sleep(0.1)\n    end"
+    end
+
+    def new_table?(table)
+      @new_tables.include?(table.to_s)
     end
   end
 end