Skip to content

clear rolled back versions from associated model#440

Merged
batter merged 4 commits intopaper-trail-gem:masterfrom
moserrya:master
Nov 11, 2014
Merged

clear rolled back versions from associated model#440
batter merged 4 commits intopaper-trail-gem:masterfrom
moserrya:master

Conversation

@moserrya
Copy link
Copy Markdown
Contributor

@moserrya moserrya commented Nov 6, 2014

I propose this change to fix an issue where, under the right circumstances, version records are created for changes that never took place. Let's say I have two models, Item and Location, and attempt to save changes in a transaction:

item.transaction do
  item.update_attributes!(sku: 'AYH123HJLF')
  Location.first.update_attributes!(code: '999-999')
end

The Item model here has_paper_trail. A version is created for the item's update_attributes! call is created and then rolled back, as expected. However, if we rescue from the error that happens when we run Location.first.update_attributes! and then call item.save, the rolled back version created when we tried to update the sku in a transaction is also saved - even though the sku on the item is not changed.

An alternative approach - could try setting autosave: false in has_paper_trail

@batter
Copy link
Copy Markdown
Collaborator

batter commented Nov 10, 2014

I think this makes sense.. I'm guessing the concern here is a failure on the first update, then a second update going through and a version being persisted for a state change that never happened? Any chance you can add some tests?

@moserrya
Copy link
Copy Markdown
Contributor Author

This is exactly the concern. I will add tests.

@batter batter added this to the 3.1.0 milestone Nov 10, 2014
@batter batter merged commit 44e4332 into paper-trail-gem:master Nov 11, 2014
@batter
Copy link
Copy Markdown
Collaborator

batter commented Nov 11, 2014

Cheers

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