2

After I create a record, I send an email, which I do in an after_commit callback. I want to save the Message-Id header of the email as an attribute on the record to use later. I implemented this as:

after_commit on: :create do
  if email = Mailer.email(self).deliver
    # `self.message_id = email.message_id` has no effect, so I'm calling update()
    self.update message_id: email.message_id
  end
end

Surprisingly (to me), this causes an infinite email-sending loop (sorry, Mailgun); it appears that the callback is called on update even though on: :create is specified.

Is there something wrong with this approach that I'm not seeing? How else could I attach this value to this record?

My only thought is to try gating the callback on previous_changes, but in any case I'd like to understand why this doesn't work as-is.

Community
  • 1
  • 1
Sam Blake
  • 657
  • 6
  • 13
  • It occurs to me that an easier way than `previous_changes` would be to just only send the email if `message_id` isn't yet set. That would probably work, but it's still confusing to me why this doesn't work as-is. – Sam Blake Feb 23 '14 at 23:23

3 Answers3

6

"Is there something wrong with this approach that I'm not seeing?"

I would also expect update in after_commit callback to trigger only on: update callbacks. However I found this in Rails source:

def committed!(should_run_callbacks = true) #:nodoc:
  _run_commit_callbacks if should_run_callbacks && destroyed? || persisted?
ensure
  force_clear_transaction_record_state
end

Only after running all after_commit callbacks does Rails clear new_record status of the transaction.

"How else could I attach this value to this record?"

Idea 1:

self.update_columns message_id: email.message_id

This won't create transaction and thus won't trigger another after_commit callback. I think it's appropriate to send email and store id in the record outside transaction. After all, if something fails you cannot rollback updating record with message ID and rollback sending email. It's just not atomic by nature.

Idea 2:

In after_commit callback queue an ActiveJob job that sends email and updates record with message id. With real backend like SuckerPunch or DelayedJob the job is queued with record's "Global ID". Job's perform method gets a freshly loaded record with no transaction state stored in its instance variables.

Wojtek Kruszewski
  • 13,940
  • 6
  • 38
  • 38
0

This might work

after_commit :email_send, :if => lambda{ new_record? }

or

after_commit :email_send, :on => :create

or

after_create :email_send


def email_send
    if email = Mailer.email(self).deliver
        # `self.message_id = email.message_id` has no effect, so I'm calling update()
        self.update message_id: email.message_id
    end
end
Ruby Racer
  • 5,690
  • 1
  • 26
  • 43
0

Checking for the prior existence of message_id seems to have solved the problem in my particular case:

after_commit on: :create do
  unless self.message_id
    if email = Mailer.email(self).deliver
      self.update message_id: email.message_id
    end
  end
end

But I imagine there could be a scenario where I couldn't get away with that, so it'd still be good to understand why an on: :create callback is being called on update.

Sam Blake
  • 657
  • 6
  • 13