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

Add timestamptz to the dummy value case statement #1544

Merged
merged 1 commit into from
Mar 30, 2023

Conversation

callahat
Copy link
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
Collaborator

mcmire commented Mar 10, 2023

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:

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
Copy link
Contributor Author

callahat commented Mar 30, 2023

@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
Collaborator

mcmire commented Mar 30, 2023

@callahat Great, thanks so much!

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.

2 participants