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

after_commit callback not firing on initial state #112

Closed
samdec opened this issue Feb 20, 2014 · 3 comments
Closed

after_commit callback not firing on initial state #112

samdec opened this issue Feb 20, 2014 · 3 comments
Assignees
Labels
Milestone

Comments

@samdec
Copy link

samdec commented Feb 20, 2014

I wrote a failing spec that exposes the bug:

In spec/models/validator.rb

state :sleeping, :after_commit => :yawn, :initial => true
...
def yawn
end

In spec/unit/persistence/active_record_persistence_spec.rb

describe "after_commit callback" do
  it "should fire :after_commit on initial create" do
    validator = Validator.new(:name => "name")
    expect(validator).to receive(:yawn).once
    validator.save!
    expect(validator).to be_sleeping
  end
...
$ bundle exec rspec spec/unit/persistence/active_record_persistence_spec.rb

Failures:
  1) transitions with persistence transactions after_commit callback should fire :after_commit on initial create
     Failure/Error: expect(validator).to receive(:yawn).once
       (#<Validator:0x007f8abba9d358>).yawn(any args)
           expected: 1 time with any arguments
           received: 0 times with any arguments
     # ./spec/unit/persistence/active_record_persistence_spec.rb:209:in `block (4 levels) in <top (required)>'

Finished in 0.26046 seconds
26 examples, 1 failure
@alto
Copy link
Member

alto commented Mar 4, 2014

Unfortunately, the concept of :after_commit hooks based on the states is misleading. The :after_commit hooks are fired only after a state transition has happened and should be therefore event-based instead. So, more like this

  aasm do
    state :sleeping, :initial => true
    state :running
    state :failed, :after_enter => :fail

    event :run, :after_commit => :change_name! do
      transitions :to => :running, :from => :sleeping
    end
    event :sleep, :after_commit => :yawn do
      transitions :to => :sleeping, :from => :running
    end
    event :fail do
      transitions :to => :failed, :from => [:sleeping, :running]
    end
  end

although the current implementation is different from that. Since there is no initial event triggered, there is no :after_commit when entering the initial state. I will fix that with the upcoming major release 4.

Another confusion might come from the fact that AAM :after_commit hooks are not the same as ActiveRecord life cycle :after_commit hooks, and they are not implemented to use these.

So, please bear with me, and don't expect the :after_commit hook to be fired when entering the initial state.

I will use this issue as a reminder to change the state and event definitions in version 4.

@alto alto added this to the aasm4 milestone Mar 4, 2014
@alto alto added the aasm4 label Mar 4, 2014
@samdec
Copy link
Author

samdec commented Mar 6, 2014

Our work-around for this was to use a Rails after_commit hook:

after_commit :yawn, :if => Proc.new{ |s| s.state_previously_changed? && s.sleeping? }

Kind of hacky, but it will work until we can hook into the initial event.

@alto
Copy link
Member

alto commented Mar 6, 2014

I'm afraid you got me wrong. I just said I will move the :after_commit hook to events, that's right. But there won't be something like an "initial event" to hook into. The transactions used by AASM are not the same that are used by ActiveRecord to save a record (just the transaction mechanism). It's more like an additional transaction around all the after and before hooks provided by AASM, in order to make sure that the state is not saved unless all hooks are fine (and don't run into an exception). If you really have to hook into an :after_commit, you have to use ActiveRecord's.

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

No branches or pull requests

2 participants