Validate enum column is an integer in define_enum_for

If your ActiveRecord model stores its `enum` data in a non-integer
column, ActiveRecord will save the data without error. However, when you
access the attribute on the record after saving, AR will look for the
string to what it expects to be a list of numbers and return `nil`
rather than the mapped value.

This change adds a third criterion to the `define_enum_for` matcher,
verifying that the underlying database column has a `sql_type` of
`"integer"`.

Fix #827.

NEWS.md

@@ -46,6 +46,9 @@
 * Update `validate_numericality_of` so that it no longer raises an
   IneffectiveTestError if used against a numeric column.
 
+* Add an additional check to `define_enum_for` to ensure that the column that
+  underlies the enum attribute you're testing is an integer column.
+
 # 3.0.1
 
 ### Bug fixes

lib/shoulda/matchers/active_record/define_enum_for_matcher.rb

@@ -63,8 +63,8 @@ def with(expected_enum_values)
         end
 
         def matches?(subject)
-          @model = subject
-          enum_defined? && enum_values_match?
+          @record = subject
+          enum_defined? && enum_values_match? && column_type_is_integer?
         end
 
         def failure_message
@@ -84,23 +84,25 @@ def description
             desc << " with #{options[:expected_enum_values]}"
           end
 
+          desc << " and store the value in a column with an integer type"
+
           desc
         end
 
         protected
 
-        attr_reader :model, :attribute_name, :options
+        attr_reader :record, :attribute_name, :options
 
         def expectation
-          "#{model.class.name} to #{description}"
+          "#{model.name} to #{description}"
         end
 
         def expected_enum_values
           hashify(options[:expected_enum_values]).with_indifferent_access
         end
 
         def actual_enum_values
-          model.class.send(attribute_name.to_s.pluralize)
+          model.send(attribute_name.to_s.pluralize)
         end
 
         def enum_defined?
@@ -111,6 +113,18 @@ def enum_values_match?
           expected_enum_values.empty? || actual_enum_values == expected_enum_values
         end
 
+        def column_type_is_integer?
+          column.type == :integer
+        end
+
+        def column
+          model.columns_hash[attribute_name.to_s]
+        end
+
+        def model
+          record.class
+        end
+
         def hashify(value)
           if value.nil?
             return {}

spec/unit/shoulda/matchers/active_record/define_enum_for_matcher_spec.rb

@@ -6,10 +6,12 @@
       it 'rejects' do
         record = record_with_array_values
         plural_enum_attribute = enum_attribute.to_s.pluralize
-        message = "Expected #{record.class} to define :#{plural_enum_attribute} as an enum"
-        assertion = -> {
+        message = "Expected #{record.class} to define :#{plural_enum_attribute} as an enum and store the value in a column with an integer type"
+
+        assertion = lambda do
           expect(record).to define_enum_for(plural_enum_attribute)
-        }
+        end
+
         expect(&assertion).to fail_with_message(message)
       end
     end
@@ -19,10 +21,13 @@
         model = define_model :example do
           def self.statuses; end
         end
-        message = "Expected #{model} to define :statuses as an enum"
-        assertion = -> {
+
+        message = "Expected #{model} to define :statuses as an enum and store the value in a column with an integer type"
+
+        assertion = lambda do
           expect(model.new).to define_enum_for(:statuses)
-        }
+        end
+
         expect(&assertion).to fail_with_message(message)
       end
     end
@@ -33,19 +38,38 @@ def self.statuses; end
       end
 
       it "rejects a record where the attribute is not defined as an enum" do
-        message = "Expected #{record_with_array_values.class} to define :#{non_enum_attribute} as an enum"
+        message = "Expected #{record_with_array_values.class} to define :#{non_enum_attribute} as an enum and store the value in a column with an integer type"
+
+        assertion = lambda do
+          expect(record_with_array_values).
+            to define_enum_for(non_enum_attribute)
+        end
 
-        expect do
-          expect(record_with_array_values).to define_enum_for(non_enum_attribute)
-        end.to fail_with_message(message)
+        expect(&assertion).to fail_with_message(message)
       end
 
       it "rejects a record where the attribute is not defined as an enum with should not" do
-        message = "Did not expect #{record_with_array_values.class} to define :#{enum_attribute} as an enum"
+        message = "Did not expect #{record_with_array_values.class} to define :#{enum_attribute} as an enum and store the value in a column with an integer type"
+
+        assertion = lambda do
+          expect(record_with_array_values).
+            not_to define_enum_for(enum_attribute)
+        end
 
-        expect do
-          expect(record_with_array_values).to_not define_enum_for(enum_attribute)
-        end.to fail_with_message(message)
+        expect(&assertion).to fail_with_message(message)
+      end
+
+      context 'if the column storing the attribute is not an integer type' do
+        it 'rejects' do
+          record = record_with_array_values(column_type: :string)
+          message = "Expected #{record.class} to define :statuses as an enum and store the value in a column with an integer type"
+
+          assertion = lambda do
+            expect(record).to define_enum_for(:statuses)
+          end
+
+          expect(&assertion).to fail_with_message(message)
+        end
       end
     end
 
@@ -57,19 +81,26 @@ def self.statuses; end
         end
 
         it "accepts a record where the attribute is not defined as an enum" do
-          message = %{Expected #{record_with_array_values.class} to define :#{non_enum_attribute} as an enum with ["open", "close"]}
+          message = %{Expected #{record_with_array_values.class} to define :#{non_enum_attribute} as an enum with ["open", "close"] and store the value in a column with an integer type}
+
+          assertion = lambda do
+            expect(record_with_array_values).
+              to define_enum_for(non_enum_attribute).with(["open", "close"])
+          end
 
-          expect do
-            expect(record_with_array_values).to define_enum_for(non_enum_attribute).with(["open", "close"])
-          end.to fail_with_message(message)
+          expect(&assertion).to fail_with_message(message)
         end
 
         it "accepts a record where the attribute is defined as an enum but the enum values do not match" do
-          message = %{Expected #{record_with_array_values.class} to define :#{enum_attribute} as an enum with ["open", "close"]}
+          message = %{Expected #{record_with_array_values.class} to define :#{enum_attribute} as an enum with ["open", "close"] and store the value in a column with an integer type}
+
+          assertion = lambda do
+            expect(record_with_array_values).
+              to define_enum_for(enum_attribute).
+              with(["open", "close"])
+          end
 
-          expect do
-            expect(record_with_array_values).to define_enum_for(enum_attribute).with(["open", "close"])
-          end.to fail_with_message(message)
+          expect(&assertion).to fail_with_message(message)
         end
       end
 
@@ -83,20 +114,27 @@ def self.statuses; end
         end
 
         it "accepts a record where the attribute is defined as an enum but the enum values do not match" do
-          message = %{Expected #{record_with_hash_values.class} to define :#{enum_attribute} as an enum with {:active=>5, :archived=>10}}
+          message = %{Expected #{record_with_hash_values.class} to define :#{enum_attribute} as an enum with {:active=>5, :archived=>10} and store the value in a column with an integer type}
 
-          expect do
-            expect(record_with_hash_values).to define_enum_for(enum_attribute).with(active: 5, archived: 10)
-          end.to fail_with_message(message)
+          assertion = lambda do
+            expect(record_with_hash_values).
+              to define_enum_for(enum_attribute).
+              with(active: 5, archived: 10)
+          end
+
+          expect(&assertion).to fail_with_message(message)
         end
 
         it "rejects a record where the attribute is not defined as an enum" do
-          message = %{Expected #{record_with_hash_values.class} to define :record_with_hash_values as an enum with {:active=>5, :archived=>10}}
+          message = %{Expected #{record_with_hash_values.class} to define :record_with_hash_values as an enum with {:active=>5, :archived=>10} and store the value in a column with an integer type}
+
+          assertion = lambda do
+            expect(record_with_hash_values).
+              to define_enum_for(:record_with_hash_values).
+              with(active: 5, archived: 10)
+          end
 
-          expect do
-            expect(record_with_hash_values)
-              .to define_enum_for(:record_with_hash_values).with(active: 5, archived: 10)
-          end.to fail_with_message(message)
+          expect(&assertion).to fail_with_message(message)
         end
       end
     end
@@ -109,18 +147,22 @@ def non_enum_attribute
       :condition
     end
 
-    def record_with_array_values
-      _enum_attribute = enum_attribute
-      define_model :record_with_array_values do
-        enum(_enum_attribute => %w(published unpublished draft))
-      end.new
+    def record_with_array_values(column_type: :integer)
+      model = define_model(
+        :record_with_array_values,
+        enum_attribute => { type: column_type },
+      )
+      model.enum(enum_attribute => ['published', 'unpublished', 'draft'])
+      model.new
     end
 
     def record_with_hash_values
-      _enum_attribute = enum_attribute
-      define_model :record_with_hash_values do
-        enum(_enum_attribute => { active: 0, archived: 1 })
-      end.new
+      model = define_model(
+        :record_with_hash_values,
+        enum_attribute => { type: :integer },
+      )
+      model.enum(enum_attribute => { active: 0, archived: 1 })
+      model.new
     end
   end
 end