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 may incorrectly return nothing #131

Closed
henrik opened this issue Feb 15, 2012 · 8 comments
Closed

version_at may incorrectly return nothing #131

henrik opened this issue Feb 15, 2012 · 8 comments
Assignees
Milestone

Comments

@henrik
Copy link
Contributor

henrik commented Feb 15, 2012

Had an issue where version_at returned nothing but should have.

Turns out version_at uses versions.created_at, which may be slightly later than the update that caused that version (item.updated_at).

Don't know if this is particular to Postgres. Suppose it happens more if you have higher-resolution timestamps.

Example:

> item.versions.last.created_at_before_type_cast
=> "2012-02-13 14:53:39.583205"
> item.updated_at_before_type_cast
=> "2012-02-13 14:53:39.580887"

In our case, we stowed away the updated_at of one record in another (e.g. item.campaign_updated_at). When we later tried to use that to find the right version of the campaign, we got nothing, because it queried for versions with created_at > item.campaign_updated_at. It is supposed to find the next version (representing the state the campaign was in at the given time), but instead found the version created at almost the same time as this update, representing the previous state. Sometimes there is no previous state, and we get nil.

Took me a while to wrap my head around exactly why we got the nil, but it should be enough to notice that the timestamps don't match up and this can cause off-by-one-version issues with version_at.

Would probably be best to set versions.created_at to the exact updated_at of the item, or to set another column like versions.item_updated_at to this value, and query for that with version_at.

@airblade
Copy link
Collaborator

Thanks for flagging this. It sounds like it was quite tricky to track down.

I agree with your suggested solutions, particularly the first one: I think that's most consistent. I'll try to get a fix out in the next day or two.

@airblade
Copy link
Collaborator

Sorry about the delay in getting back to you. I hope it hasn't inconvenienced you.

When PaperTrail is preparing a new version to record an update to your model, it builds it and lets ActiveRecord take care of saving it when the main model is saved. I'm not sure how best to set the version's created_at because when we build it the parent hasn't yet been updated.

I think there would be a similar difficulty with your second idea.

Hmm. Have you had any further thoughts since you ran into this?

@henrik
Copy link
Contributor Author

henrik commented Mar 12, 2012

No problem. We've worked around it so far by adding a subsecond offset in the code that relied on this.

I see the update callback is before_update. Would it be very tricky to move it to after_update instead, and let it manage its own creation?

@airblade
Copy link
Collaborator

I have a vague memory that after_update caused some problems but frustratingly I can't remember any details.

Off the top of my head I think after_update would be tricky because when the callback builds the version instance, the changes hash on the parent model would have been cleared by the save.

@henrik
Copy link
Contributor Author

henrik commented Mar 12, 2012

One solution for that would be to store the changes in an instance variable before_update and actually save the version from that ivar after_update.

@batter
Copy link
Collaborator

batter commented Jul 30, 2014

This issue should be fixed via 5ee4582 (PR => #375)

@henrik
Copy link
Contributor Author

henrik commented Jul 31, 2014

@batter Oh, good job 👍

@batter
Copy link
Collaborator

batter commented Aug 1, 2014

Cheers, sorry this took so long to tackle!

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

3 participants