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

version_at not working as expected #354

Closed
nicolasfranck opened this issue Apr 4, 2014 · 4 comments
Closed

version_at not working as expected #354

nicolasfranck opened this issue Apr 4, 2014 · 4 comments
Milestone

Comments

@nicolasfranck
Copy link

The method "version_at" does not seem to work as expected.

Nice to know:

I use postgresql, with timestamps without timezone information,
so everything is stored in UTC (I checked this in the database).
My own timezone is Europe/Brussels. The only were timezone
is used, is when the dates are viewed in the interface.

Problem:

#description is 'a'
    report = Report.new(:description => 'a',:type => 'test')
    report.save

    date_version_1 = report.updated_at

    #description is 'b'
    report.description = "b"
    report.save

    date_version_2 = report.updated_at

    #description is 'c'
    report.description = "c"
    report.save

    date_version_3 = report.updated_at

    #tests
    [
      {
        :date => date_version_1,:description => 'a'
      },
      {
        :date => date_version_2,:description => 'b'
      },
      {
        :date => date_version_3,:description => 'c'
      }
    ].each do |test|

      old_object = report.version_at(test[:date])
      if old_object.nil?
        $stderr.puts "could not retrieve version of date #{test[:date]}"
      elsif old_object.description != test[:description]
        $stderr.puts "got invalid version (expected: #{test[:description]}, found: #{old_object.description})"
      end

    end

output (the date is shown in local time):

could not retrieve version of date 2014-04-04 10:12:43 +0200
got invalid version (expected: b, found: a)
got invalid version (expected: c, found: b)

I logged the sql, and saw that paper_trail requests "versions.created" > 2014-04-04 08:12:43. This UTC is as expected: 2 hours lower than local time.

Any idea on how to solve this?
What am I doing wrong?

@nicolasfranck
Copy link
Author

I think it has the same cause as mentioned in #131

Report.version_at(date_2 + 1.second) returns the report with description "b",
as expected! So there is probably a (variable) delay between storing the version
and the target object.

@batter
Copy link
Collaborator

batter commented Apr 16, 2014

This is a dup of #314, closed in favor of that one.

I have a branch pushed up, order_by_primary_key, which contains my preliminary implementation of my proposed fix. Give that a try if you'd like, should fix your issues. Please see #314 for more info.

Sorry for the delayed response btw, my GitHub notifications were all messed up.

@batter batter closed this as completed Apr 16, 2014
@nicolasfranck
Copy link
Author

Thanks,

I tried this branch, but still have the same issues.
Adding the extra second to the date helps.

But this has more to do with comparing of dates, rather than with the ordering of the keys.
Versions are created after the original (old) record, so there is always a leap of time between "updated_at" of the original (old) record, and "created_at" of the version record.

Why not simply store the timestamp "updated_at" of the old record in the version record, for example in a field called "model_updated_at", and use that timestamp for comparison when requesting "version_at"?

In that case, there is no random time period between "updated_at" of the old record, and created_at of the version record. Rails timestamps should be left untouched, for there are
updated by rails itself.

@batter batter reopened this Apr 17, 2014
@batter batter added this to the 3.1.0 milestone Apr 17, 2014
@batter
Copy link
Collaborator

batter commented Apr 17, 2014

Ok I understand and am aware of the issue you are speaking to. As you can see, I have a comment in the code noting the plan for handling this solution.

The problem is as follows: the versions are currently constructed using a before_update callback to build the version (which allows it to get automatically persisted when the record gets persisted). This is ideal in terms of convenience and makes sense on a lot of levels, but since the persistence of associated objects that are built occurs (a very short amount of time) after the persistence of the original object, the Version's created_at timestamp will actually be a few microseconds behind the updated_at timestamp for the model.

The plan to solve this is to switch the callback that handles building the version objects for updates to use an after_update callback, and then to force assign those created_at timestamps to match the updated_at timestamp from the update that triggered that Version creation. If you want to try to make a PR for that, I'd be happy to examine it.

Right now, I'm focusing on releasing 3.0.2, then shortly thereafter 3.0.3 (which will be focused on finalizing the fix to address #314), then 3.1.0 is the next milestone I have planned, which will contain the patch for this issue, and hopefully a handling of associations (#345 looks promising). Might look into seeing if someone else can sign on as a maintainer to help with some of this..

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

No branches or pull requests

2 participants