Skip to content

Commit

Permalink
Validate enum column is an integer in define_enum_for
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
geoffharcourt committed Oct 30, 2015
1 parent 0e9c48e commit ee43f19
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 40 deletions.
16 changes: 15 additions & 1 deletion lib/shoulda/matchers/active_record/define_enum_for_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def with(expected_enum_values)

def matches?(subject)
@model = subject
enum_defined? && enum_values_match?
enum_defined? && enum_values_match? && column_type_is_integer?
end

def failure_message
Expand All @@ -84,6 +84,8 @@ def description
desc << " with #{options[:expected_enum_values]}"
end

desc << " and store the value in a column with an integer type"

desc
end

Expand Down Expand Up @@ -111,6 +113,18 @@ def enum_values_match?
expected_enum_values.empty? || actual_enum_values == expected_enum_values
end

def matched_column
@_matched_column ||= begin
@model.class.columns.detect do |column|
column.name == attribute_name.to_s
end
end
end

def column_type_is_integer?
matched_column.type == :integer
end

def hashify(value)
if value.nil?
return {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -19,10 +21,32 @@
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

context 'if the column storing the attribute is not an integer type' do
it 'rejects' do
model = define_model(
:record_with_array_values,
_enum_attribute => { type: :string },
) do |model|
enum enum_attribute => %w(published unpublished draft)
end

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
Expand All @@ -33,19 +57,30 @@ 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"

expect do
expect(record_with_array_values).to define_enum_for(non_enum_attribute)
end.to fail_with_message(message)
expect {
expect(record_with_array_values).
to define_enum_for(non_enum_attribute)
}.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"

expect {
expect(record_with_array_values).
to_not define_enum_for(enum_attribute)
}.to fail_with_message(message)
end

it 'rejects a record where the saved attribute does not come back out after saving' do
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"

expect do
expect(record_with_array_values).to_not define_enum_for(enum_attribute)
end.to fail_with_message(message)
expect {
expect(record_with_array_values).
to_not define_enum_for(enum_attribute)
}.to fail_with_message(message)
end
end

Expand All @@ -57,19 +92,22 @@ 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}

expect do
expect(record_with_array_values).to define_enum_for(non_enum_attribute).with(["open", "close"])
end.to fail_with_message(message)
expect {
expect(record_with_array_values).
to define_enum_for(non_enum_attribute).with(["open", "close"])
}.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}

expect do
expect(record_with_array_values).to define_enum_for(enum_attribute).with(["open", "close"])
end.to fail_with_message(message)
expect {
expect(record_with_array_values).
to define_enum_for(enum_attribute).
with(["open", "close"])
}.to fail_with_message(message)
end
end

Expand All @@ -83,20 +121,23 @@ 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)
expect {
expect(record_with_hash_values).
to define_enum_for(enum_attribute).
with(active: 5, archived: 10)
}.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}

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 {
expect(record_with_hash_values).
to define_enum_for(:record_with_hash_values).
with(active: 5, archived: 10)
}.to fail_with_message(message)
end
end
end
Expand All @@ -110,17 +151,25 @@ def non_enum_attribute
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
model = define_model(
:record_with_array_values,
_enum_attribute => { type: :integer },
) do |model|
enum enum_attribute => %w(published unpublished draft)
end

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 },
) do |model|
model.enum enum_attribute => { active: 0, archived: 1 }
end

model.new
end
end
end

0 comments on commit ee43f19

Please sign in to comment.