validate_uniqueness_of with scoped_to fails when the scope is of data type time

[mikeduynguyen]

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.

A possible fix would be to also increment the previous_value by 60 seconds before calling next to guarantee both the time and the date are different:
previous_value.to_datetime.in(60).next

I'm more than happy to open a PR for this fix! Let me know what you guys think about this.

[mcmire]

Ah... yes that makes sense! We would definitely appreciate a PR. 👍

[mikeduynguyen]

@mcmire I apologize for the looong the delay on this 😅. Finally got to open the PR to fix this bug here #1190 .

It's my first time contributing to an open source project so please let me know if you have any suggestions for the PR.

[mcmire]

Hey folks. In an effort to lighten our load as maintainers and be able to serve you better in the future, the shoulda-matchers team is working on cleaning out the cobwebs in this repo by pruning the backlog. As there are few of us, there are a lot of items that will simply never earn our attention in a reasonable time frame, and rather than giving you an empty promise, we think it makes more sense to focus on more recent issues. That means, unfortunately, that we must close this issue.

Don't take this the wrong way: our aim is not to diminish the effort people have made or dismiss problems that have been raised. If you feel that we should reopen this issue, then please let us know so that we can reprioritize it. Thanks!

[mcmire]

Reopening since this is dependent on a PR.

[KapilSachdev]

@mcmire seems like this issue is also fixed in the PR mentioned, and will be good to close.

[mcmire]

@KapilSachdev Oops, good eye! Will close.