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

Nested AR transactions #107

Closed
MSch opened this issue Jan 21, 2014 · 7 comments
Closed

Nested AR transactions #107

MSch opened this issue Jan 21, 2014 · 7 comments
Assignees

Comments

@MSch
Copy link

MSch commented Jan 21, 2014

91a90c9 introduced nested transactions for events.

I'd need a way to turn that off, I just upgraded to the latest aasm to get after_commit and my code does not expect nested transactions.

Would it be ok to introduce a setter like e.g. AASM.perform_events_in_new_transactions that can be turned off?

@ghost ghost assigned alto Jan 22, 2014
@alto
Copy link
Member

alto commented Jan 22, 2014

Well, we got a couple of configurations per class already (for example skip_validation_on_save). There is no general setting like you suggest AASM.perform_events_in_new_transactions though. But I could offer something like

class MyClass
  include AASM
  aasm :skip_transactions => true do
    ...
  end
end

What do you think, would that solve your problem? If you, I can provide it by the weekend latest.

@MSch
Copy link
Author

MSch commented Jan 22, 2014

@alto that would be great!

Regarding naming, I think requires_new_transaction would be better, so that

class MyClass
  include AASM
  aasm requires_new_transaction: false do
    ...
  end
end

would switch https://github.com/aasm/aasm/blob/master/lib/aasm/persistence/active_record_persistence.rb#L141 from :requires_new => true to :requires_new => false

@alto
Copy link
Member

alto commented Jan 22, 2014

Ah, I see. You only got a problem with the nested transactions. Yes, then your naming is better.

@alto
Copy link
Member

alto commented Jan 22, 2014

Take a look at ca7306d and try it out. It's not yet released, but I will once you say it's okay.

@alto
Copy link
Member

alto commented Jan 24, 2014

Closing the issue now. Version 3.1.0 will be released soon. Let me know if there are any problems.

@alto alto closed this as completed Jan 24, 2014
@MSch
Copy link
Author

MSch commented Jan 28, 2014

@alto sorry for taking so long to get back to you. We've now worked around it manually in our code. I think this is exactly what we need though, so once 3.1.0 is out we'll switch to that. Thanks again!!!

@alto
Copy link
Member

alto commented Jan 29, 2014

Version 3.1.0 has been released including the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants