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

don't use .to_i to avoid different usec being considered equal #94

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pesta
Copy link

@pesta pesta commented Mar 10, 2023

✍️ Description

(required) Please include any relevant details about this Pull Request, with a focus on:

  • What does it fix.
    Different times being considered equal.
ApplicationRecord.connection.execute("CREATE TABLE models (id UUID, my_date TIMESTAMP)")

class Model < ApplicationRecord
  validates :my_date, date: { equal_to: ->(model) { model.my_date.end_of_day } }
end

model = Model.new
model.my_date = Time.current.end_of_day.change(usec: 0)
model.valid? # true
model.my_date == model.my_date.end_of_day # false
  • Why did you do things this way.
    The previous implementation used .to_i to compare dates, this lead to a bug where a model would pass validation without actually being valid.

  • How does it fix the issue at hand.
    Just changed the implementation to compare dates without .to_i

Based on the test suite, it looks fine.

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.

1 participant