Fix create event bug

[runeb]

Grove is using the ActiveRecord hook after_create for sending create events in River. This means that external systems are notified before the object is available in the database, because the transaction has not completed by the time after_create triggers:

http://apidock.com/rails/ActiveRecord/Callbacks/after_create

Is called after Base.save on new objects that haven’t been saved yet (no record exists). Note that this callback is still wrapped in the transaction around save. For example, if you invoke an external indexer at this point it won’t see the changes in the database.

The fix changes the Observer hook to after_commit, which fixes this issue. However, after_commit triggers after every commit to the database, so in order to distinguish create events, we need to reach into ActiveRecord internals to query the transaction type. Normally one would specify a condition to the hook in an ActiveRecord model like this

class Post < ActiveRecord::Base
  after_commit :some_method, on: :create
end

This is not possible in an Observer because hooks are specified as declared methods:

class RiverNotifications < ActiveRecord::Observer
  observe Post

  def after_commit(object)
  end
end

So we introduce a call to the private method transaction_include_any_action? which accepts an array of actions and returns true is the transaction involves any of them:

  def after_commit(object)
    if object.is_a?(Post) && object.send(:transaction_include_any_action?, [:create])
      prepare_for_publish(object, :create)
    end
  end

The pull request wraps this hackery in a private method that checks for availability of transaction_include_any_action? and raises an error if it disappears in a future upgrade.

In order for transactional tests to pass with this way of specifying hooks, we need to explicitly run the commit callbacks in the test

    it "runs commit callback even within the transactional tests" do
      post = Post.create!(:canonical_path => 'this.that')
      post.run_callbacks(:commit)
    end

[simen]

+1

[thomax]

+1

Excellent stuff. You were making this patch apply to update and delete, too, right?

[runeb]

Having big problems making the tests pass with :update and :destroy events. Has to do with transactional fixtures. Waiting for more time to look at this.

[runeb]

Expanding this patch for update and delete is trivial in the implementation, but difficult in tests. Which is why its not implemented yet. If anyone wants to have a go, please do.

And to drop the private method send, the hooks can be moved into the model by using the ActiveRecord DSL as explained above.