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.

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

Bug fix: uniqueness validation fails with scoped attributes of data type time #1190

Conversation

mikeduynguyen
Copy link
Contributor

PR to fix issue #1114

The bug

The validation fails but it should pass when the scope has data type time.

Example: model Schedule with the following attributes and their data types:
day_of_week: integer
start_time: time

class Schedule
    validates :day_of_week, uniqueness: { scope: :start_time }
end

Spec:

it { should validate_uniqueness_of(:day_of_week).scoped_to(:start_time) }

Failures:

  1) Schedule should require case sensitive unique value for day_of_week scoped to start_time
     Failure/Error: it { should validate_uniqueness_of(:day_of_week).scoped_to(:start_time) }
     
       Did not expect errors to include "has already been taken" when day_of_week is set to 0,
       got errors:
       * "has already been taken" (attribute: day_of_week, value: 0) (with different value of start_time)

This bug is caused by the method that calculates the next value to test for the uniqueness of the scope:
lib/shoulda/matchers/active_record/validate_uniqueness_of_matcher.rb

module Shoulda
  module Matchers
    module ActiveRecord
    ...
    def next_scalar_value_for(scope, previous_value)
        ...
        elsif previous_value.respond_to?(:to_datetime)
            previous_value.to_datetime.next
    ...

When previous_value is of type time:

previous_value -> 2018-07-02 11:20:14
previous_value.to_datetime.next -> 2018-07-03 11:20:14

The date is incremented by 1, however the time remains the same. As a result when the next value is set on the new record the uniqueness validation fails since the 2 records have the same time value.

The fix

This PR fixes the bug by adding in(60) to increment the timestamp by 60 second before incrementing the date with next:
previous_value.to_datetime.in(60).next

See issue thoughtbot#1114: validate_uniqueness_of with scoped_to fails when the scope is of data type time

Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>
@mcmire mcmire force-pushed the issue_1114_validate_uniqueness_of_with_scope_data_type_time branch from 7f9fb4e to 778b5f4 Compare July 12, 2020 01:45
@mcmire mcmire merged commit 778b5f4 into thoughtbot:master Jul 12, 2020
@mcmire
Copy link
Collaborator

mcmire commented Jul 12, 2020

Sorry for the long delay on merging this. I tweaked your branch slightly to get all CI to fully pass. This is in master now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants