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

Assign updated bitemporal times to the receiver after update/destroy #118

Merged
merged 1 commit into from
Feb 1, 2023

Conversation

wata727
Copy link
Contributor

@wata727 wata727 commented Jan 30, 2023

See also #117
Closes #100

We originally planned to assign all attributes to the receiver after update/destroy, but ran into some issues. See #117 for the motivation of this PR.

This PR changes the update/delete operation to only assign valid time/transaction time instead of all attributes. This eliminates the inconsistency described in #117, but the receiver does not always match the actual history when a callback is applied.

Below is an example:

class Employee
  before_create :set_current_time

  def set_current_time
    self.current_time = TIme.current
  end
end

employee = nil

Timecop.freeze("2020/09/01") {
  valid_datetime = "2020/06/01"
  ActiveRecord::Bitemporal.valid_at("2020/06/01") {
    employee = Employee.create(name: "hoge")
  }
}

Timecop.freeze("2020/09/02") {
  ActiveRecord::Bitemporal.valid_at("2020/07/01") {
    pp employee.valid_from.to_time         # => 2020-06-01 09:00:00 +0900
    pp employee.transaction_from.to_time   # => 2020-09-01 09:00:00 +0900

    employee.update!(name: "foo")

    # Updated valid_from/transaction_from
    pp employee.valid_from.to_time         # => 2020-07-01 09:00:00 +0900
    pp employee.transaction_from.to_time   # => 2020-09-02 09:00:00 +0900

    # Refer to current time before update
    pp employee.current_time.to_time       # => 2020-09-01 09:00:00 +0900
    employee.reload
    pp employee.current_time.to_time       # => 2020-09-02 09:00:00 +0900
  }
}

All attribute assignments were required to prevent this issue.

Since we don't assign all attributes, the only reliable receiver attributes are the following. Do not trust that any other value is equal to the latest history.

  • swapped_id
  • swapped_id_previously_was
  • valid_from
  • valid_to
  • transaction_from
  • transaction_to
  • saved_changes

@wata727 wata727 marked this pull request as ready for review January 30, 2023 11:15
@wata727 wata727 requested review from osyo-manga, motsat and Dooor and removed request for k-myst and kodera123 January 30, 2023 11:15
Copy link
Collaborator

@osyo-manga osyo-manga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, Thanks :)

Copy link
Contributor

@Dooor Dooor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Member

@motsat motsat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@wata727 wata727 merged commit da2342c into kufu:master Feb 1, 2023
@wata727 wata727 deleted the assign_updated_bitemporal_times branch February 1, 2023 01:29
@wata727 wata727 mentioned this pull request Feb 1, 2023
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

4 participants