Skip to content

Add timestamptz to the dummy value case statement#1544

Merged
mcmire merged 1 commit intothoughtbot:mainfrom
callahat:add-timestamptz-to-dummy_value_for
Mar 30, 2023
Merged

Add timestamptz to the dummy value case statement#1544
mcmire merged 1 commit intothoughtbot:mainfrom
callahat:add-timestamptz-to-dummy_value_for

Conversation

@callahat
Copy link
Copy Markdown
Contributor

@callahat callahat commented Mar 9, 2023

Ran into an exception when upgrading to Rails 7.0.4.2 and a column of type timestamptz.

     Failure/Error:
       is_expected.to validate_uniqueness_of(:amount).
         scoped_to(:column1, :column2, :column3, :date_time_column).
         with_message('must be unique')
     
     NoMethodError:
       undefined method `getutc' for nil:NilClass

After tracing, 'dummy value' was observed as being sent into this function, when then resulted in time being nil:
https://github.com/rails/rails/blob/v7.0.4.2/activerecord/lib/active_record/connection_adapters/postgresql/oid/timestamp_with_time_zone.rb#L122

@mcmire
Copy link
Copy Markdown
Collaborator

mcmire commented Mar 10, 2023 •
edited
Loading

Hi @callahat, thanks for the PR. Would you mind adding tests for this so that the bug doesn't reappear later accidentally? You can see an example of existing tests here:

shoulda-matchers/spec/unit/shoulda/matchers/active_record/validate_uniqueness_of_matcher_spec.rb

Line 832 in 195153c

context 'when one of the scoped attributes is a time column (using Time)' do

Thanks!

@callahat callahat force-pushed the add-timestamptz-to-dummy_value_for branch 2 times, most recently from 07160f7 to 5ed93c3 Compare March 10, 2023 18:49
@callahat callahat force-pushed the add-timestamptz-to-dummy_value_for branch from 5ed93c3 to dc877f1 Compare March 10, 2023 18:53
@callahat
Copy link
Copy Markdown
Contributor Author

callahat commented Mar 30, 2023 •
edited
Loading

@mcmire Tests added for timestamptz as well as timestamp (the later already existed, but didn't have a similar test to datetime, and all three are in the same when clause)

@mcmire mcmire merged commit e40c707 into thoughtbot:main Mar 30, 2023
@mcmire
Copy link
Copy Markdown
Collaborator

mcmire commented Mar 30, 2023

@callahat Great, thanks so much!

@vsppedro vsppedro mentioned this pull request Dec 22, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@callahat @mcmire