Skip to content

Commit

Permalink
Support multiple uniq validations on same attr
Browse files Browse the repository at this point in the history
It turns out that you can have multiple uniqueness validations on the
same attribute. This is useful if you want the attribute to be unique
under different sets of scopes. For instance:

```
class CalibrationConstantVersion < ActiveRecord::Base
  validates_uniqueness_of(
    :calibration_constant,
    scope: [:name]
  )

  validates_uniqueness_of(
    :calibration_constant,
    scope: [:begin_at, :physical_device_id]
  )
end
```

So if you have a test like this:

```
describe CalibrationConstantVersion do
  it do
    should validate_uniqueness_of(:calibration_constant).
      scoped_to(:name).
      case_insensitive
  end

  it do
    should validate_uniqueness_of(:calibration_constant).
      scoped_to(:begin_at, :physical_device_id)
  end
end
```

the first test will fail because the matcher doesn't know that
:calibration_constant is supposed to be unique relative to :begin_at and
:physical_device_id, and the second test will fail because it doesn't
know :calibration_constant is supposed to be unique relative to :name.

Now, in order to properly fix this, we'll need to add another matcher.
However, for the time being we *can* fix the following case:

```
class CalibrationConstantVersion < ActiveRecord::Base
  validates_uniqueness_of(
    :calibration_constant,
    scope: [:name],
    message: 'first message'
  )

  validates_uniqueness_of(
    :calibration_constant,
    scope: [:begin_at, :physical_device_id],
    message: 'second message'
  )
end

describe CalibrationConstantVersion do
  it do
    should validate_uniqueness_of(:calibration_constant).
      scoped_to(:name).
      with_message('first message')
  end

  it do
    should validate_uniqueness_of(:calibration_constant).
      scoped_to(:begin_at, :physical_device_id).
      with_message('second message')
  end
end
```

These tests also fail, but for a different reason. One of the checks we
make is to compare the list of scopes you've provided with the list of
scopes that are actually on the validation. Notice I said "validation"
-- we don't expect there to be multiple, we just look for the first
uniqueness validation that's on the attribute in question. So in this
case, the first test will pass, but the second test will fail, because
it thinks all that :calibration_constant is scoped to is :name.
  • Loading branch information
mcmire committed Jan 28, 2016
1 parent b1a10f5 commit 28bd9a1
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 14 deletions.
10 changes: 10 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@
* *Commits: [45de869]*
* *Fixes: [#880], [#884], [#885]*

* Fix an issue with `validate_uniqueness_of` + `scoped_to` when used against a
model where the attribute has multiple uniqueness validations and each
validation has a different set of scopes. In this case, a test written for the
first validation (and its scopes) would pass, but tests for the other
validations (and their scopes) would not, as the matcher only considered the
first set of scopes as the *actual* set of scopes.

* *References: [#830]*

### Improvements

* Update `validate_uniqueness_of` so that if an existing record fails to be
Expand All @@ -37,6 +46,7 @@
[#884]: https://github.com/thoughtbot/shoulda-matchers/issues/884
[#885]: https://github.com/thoughtbot/shoulda-matchers/issues/885
[78ccfc5]: https://github.com/thoughtbot/shoulda-matchers/commit/78ccfc50b52fa686c109d614df66744b0da65380
[#830]: https://github.com/thoughtbot/shoulda-matchers/issues/830

# 3.1.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ def matches?(given_record)

validate_attribute_present_on_model? &&
validate_scopes_present_on_model? &&
scopes_match? &&
validate_scopes_match? &&
validate_two_records_with_same_non_blank_value_cannot_coexist? &&
validate_case_sensitivity? &&
validate_after_scope_change? &&
Expand Down Expand Up @@ -380,14 +380,14 @@ def description_for_case_sensitive_qualifier
end
end

def validation
model._validators[@attribute].detect do |validator|
def validations
model._validators[@attribute].select do |validator|
validator.is_a?(::ActiveRecord::Validations::UniquenessValidator)
end
end

def scopes_match?
if expected_scopes == actual_scopes
def validate_scopes_match?
if scopes_match?
true
else
@failure_reason = 'Expected the validation'
Expand All @@ -398,7 +398,7 @@ def scopes_match?
@failure_reason << " to be scoped to #{inspected_expected_scopes}"
end

if actual_scopes.empty?
if actual_sets_of_scopes.empty?
@failure_reason << ', but it was not scoped to anything.'
else
@failure_reason << ', but it was scoped to '
Expand All @@ -409,24 +409,42 @@ def scopes_match?
end
end

def expected_scopes
Array.wrap(@options[:scopes])
def scopes_match?
actual_sets_of_scopes.empty? && expected_scopes.empty? ||
actual_sets_of_scopes.any? { |scopes| scopes == expected_scopes }
end

def inspected_expected_scopes
expected_scopes.map(&:inspect).to_sentence
end

def actual_scopes
if validation
Array.wrap(validation.options[:scope])
def inspected_actual_scopes
inspected_actual_sets_of_scopes.to_sentence(
words_connector: " or ",
last_word_connector: ", or"
)
end

def inspected_actual_sets_of_scopes
inspected_sets_of_scopes = actual_sets_of_scopes.map do |scopes|
scopes.map(&:inspect)
end

if inspected_sets_of_scopes.many?
inspected_sets_of_scopes.map { |x| "(#{x.to_sentence})" }
else
[]
inspected_sets_of_scopes.map(&:to_sentence)
end
end

def inspected_actual_scopes
actual_scopes.map(&:inspect).to_sentence
def expected_scopes
Array.wrap(@options[:scopes])
end

def actual_sets_of_scopes
validations.map do |validation|
Array.wrap(validation.options[:scope])
end.reject(&:empty?)
end

def allows_nil?
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,94 @@
end
end

context 'when there is more than one validation on the same attribute with different scopes' do
context 'when a record exists beforehand, where all scopes are set' do
if column_type != :boolean
context 'when each validation has the same (default) message' do
it 'accepts' do
pending 'this needs another qualifier to properly fix'

model = define_model(
'Example',
attribute_name => :string,
scope1: column_type,
scope2: column_type
) do |m|
m.validates_uniqueness_of(attribute_name, scope: [:scope1])
m.validates_uniqueness_of(attribute_name, scope: [:scope2])
end

model.create!(
attribute_name => dummy_value_for(:string),
scope1: dummy_value_for(column_type),
scope2: dummy_value_for(column_type)
)

expect(model.new).to validate_uniqueness.scoped_to(:scope1)
expect(model.new).to validate_uniqueness.scoped_to(:scope2)
end
end
end

context 'when each validation has a different message' do
it 'accepts' do
model = define_model(
'Example',
attribute_name => :string,
scope1: column_type,
scope2: column_type
) do |m|
m.validates_uniqueness_of(
attribute_name,
scope: [:scope1],
message: 'first message'
)
m.validates_uniqueness_of(
attribute_name,
scope: [:scope2],
message: 'second message'
)
end

model.create!(
attribute_name => dummy_value_for(:string),
scope1: dummy_value_for(column_type),
scope2: dummy_value_for(column_type)
)

expect(model.new).
to validate_uniqueness.
scoped_to(:scope1).
with_message('first message')

expect(model.new).
to validate_uniqueness.
scoped_to(:scope2).
with_message('second message')
end
end
end

context 'when no record exists beforehand' do
it 'accepts' do
pending 'this needs another qualifier to properly fix'

model = define_model(
'Example',
attribute_name => :string,
scope1: column_type,
scope2: column_type
) do |m|
m.validates_uniqueness_of(attribute_name, scope: [:scope1])
m.validates_uniqueness_of(attribute_name, scope: [:scope2])
end

expect(model.new).to validate_uniqueness.scoped_to(:scope1)
expect(model.new).to validate_uniqueness.scoped_to(:scope2)
end
end
end

define_method(:build_attribute) do |attribute_options|
attribute_options.deep_merge(
column_type: column_type,
Expand Down

0 comments on commit 28bd9a1

Please sign in to comment.