Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No more throws a error when use validate_uniqueness_of with scope of boolean column #694

Closed
wants to merge 1 commit into from
+58 −3
Closed

No more throws a error when use validate_uniqueness_of with scope of boolean column #694

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ def matches?(subject)
@original_subject = subject
@subject = subject.class.new
@expected_message ||= :taken
@all_records = @subject.class.all

set_scoped_attributes &&
validate_everything_except_duplicate_nils_or_blanks? &&
Expand Down Expand Up @@ -397,12 +398,11 @@ def model_class?(model_name)
end

def validate_after_scope_change?
if @options[:scopes].blank?
if @options[:scopes].blank? || all_scopes_are_booleans?
true
else
all_records = @subject.class.all
@options[:scopes].all? do |scope|
previous_value = all_records.map(&scope).compact.max
previous_value = @all_records.map(&scope).compact.max

next_value =
if previous_value.blank?
Expand Down Expand Up @@ -447,6 +447,8 @@ def dummy_scalar_value_for(column)
DateTime.now
when :uuid
SecureRandom.uuid
when :boolean
true
else
'dummy value'
end
Expand Down Expand Up @@ -474,11 +476,23 @@ def next_scalar_value_for(scope, previous_value)
previous_value.next
elsif previous_value.respond_to?(:to_datetime)
previous_value.to_datetime.next
elsif boolean_value?(previous_value)
!previous_value
else
previous_value.to_s.next
end
end

def all_scopes_are_booleans?
@options[:scopes].all? do |scope|
@all_records.map(&scope).all? { |s| boolean_value?(s) }
end
end

def boolean_value?(value)
value.in?([true, false])
end

def defined_as_enum?(scope)
@subject.class.respond_to?(:defined_enums) &&
@subject.defined_enums[scope.to_s]
Expand Down
41 changes: 41 additions & 0 deletions spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,43 @@
column_type: :string
end

context 'when one of the scoped attributes is a boolean column' do
include_context 'it supports scoped attributes of a certain type',
column_type: :boolean
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the tests for these are going to be a little different, because there's a limited number of values for a boolean column.

If you have one existing record where the scope attribute is either true or false, then you can simply flip that scope attribute. If you have two existing records where one of the record's scope attribute is true and the other is false... then you can't flip the scope attribute.

Say you have a validation on an attribute against a boolean scope:

class Person < ActiveRecord::Base
  validates :name, scope: :snowflake
end

Here's what a test would look like if there's only one existing record:

describe Person do
  it "is valid with a unique name when snowflake is true" do
    create(:person, name: "Steve", snowflake: true)

    person = build(:person, name: "Steve", snowflake: true)
    person.valid?
    expect(person).errors[:name].not_to be_empty

    person = build(:person, name: "Joe", snowflake: true)
    person.valid?
    expect(person).errors[:name].to be_empty

    # here we can test against the snowflake attribute -
    # i.e. if name stays same but snowflake is different
    person = build(:person, name: "Steve", snowflake: false)
    person.valid?
    expect(person).errors[:name].to be_empty
  end
end

But now consider if there are two existing records. Here's what those tests might look like:

describe Person do
  it "is valid with a unique name when snowflake is true" do
    create(:person, name: "Steve", snowflake: true)
    create(:person, name: "Steve", snowflake: false)

    person = build(:person, name: "Steve", snowflake: true)
    person.valid?
    expect(person).errors[:name].not_to be_empty

    person = build(:person, name: "Joe", snowflake: true)
    person.valid?
    expect(person).errors[:name].to be_empty

    # we can't really test what happens if name stays same and snowflake is different
    # b/c all values for snowflake are filled - so let's just test what happens if name is
    # different when snowflake is false
    person = build(:person, name: "Steve", snowflake: false)
    person.valid?
    expect(person).errors[:name].not_to be_empty

    person = build(:person, name: "Joe", snowflake: false)
    person.valid?
    expect(person).errors[:name].not_to be_empty
  end
end

So now the question is... how do we modify the matcher to run these tests. I don't have a clear solution for this, but it seems like as soon as one of the scopes is a boolean attribute, it kind of messes with the existing logic...


context 'when there is more than one scoped attribute and all are boolean columns' do
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [91/80]

it 'accepts when all the scoped attribute are true' do
record = build_record_validating_uniqueness(
scopes: [
build_attribute(name: :scope1),
build_attribute(name: :scope2),
]
)
expect(record).to validate_uniqueness.scoped_to(:scope1, :scope2)
end

it 'accepts when all the scoped attribute are false' do
record = build_record_validating_uniqueness(
scopes: [
build_attribute(name: :scope1, value: false),
build_attribute(name: :scope2, value: false),
]
)
expect(record).to validate_uniqueness.scoped_to(:scope1, :scope2)
end

it 'accepts when one scoped attribute is true and other is false' do
record = build_record_validating_uniqueness(
scopes: [
build_attribute(name: :scope1),
build_attribute(name: :scope2, value: false),
]
)
expect(record).to validate_uniqueness.scoped_to(:scope1, :scope2)
end
end
end

context 'when one of the scoped attributes is an integer column' do
include_context 'it supports scoped attributes of a certain type',
column_type: :integer
Expand Down Expand Up @@ -752,6 +789,8 @@ def dummy_scalar_value_for(attribute_type)
Time.now
when :uuid
SecureRandom.uuid
when :boolean
true
else
raise ArgumentError, "Unknown type '#{attribute_type}'"
end
Expand All @@ -764,6 +803,8 @@ def next_version_of(value, value_type)
SecureRandom.uuid
elsif value.is_a?(Time)
value + 1
elsif value.in?([true, false])
!value
elsif value.respond_to?(:next)
value.next
end
Expand Down