Callbacks for specific notification

[curpeng]

Hi guys,

Thanks for implementing this gem, it's super helpful!
There is one thing, which seems to be really nice to have - an option to specify the name of the notification for a callback. It turned out that quite often, we need to do something only for specific notification. Right now, we have two options:

QuestionsDelivery < ApplicationDelivery
   after_notify :mark_question_as_sent 
  
   after_notify do
     # Option 2: check notification_name and call needed actions
      mark_question_as_sent if notification_name == :send_questions
      create_event if notification_name == :questions_reminder
   end

  private

  def mark_question_as_sent
    # Option 1: another notification doesn't need a question object, e.g. it's a reminder to a user that we can send questions, so we can just check if params[:question] is present. 
     return unless params[:question].present? 
  end
end

As you see, both solutions look hacky, it would be nice to have something like:

  after_notify :mark_question_as_sent, only: %[questions_sent]

[palkan]

Thanks for the suggestion. Sounds good to me.
Would you like to work on it yourself 🙂? My backlog is currently full, so I won't be able to take this until the end of December.

[curpeng]

Sure thing, do you agree with the suggested API or you have other preferences? Are there any guides/requirements for PRs(e.g formatting)?

[palkan]

Yep, except/only looks good to me.

Since we use ActiveSupport::Callbacks, we can take a look at how controllers callbacks are implemented: https://github.com/rails/rails/blob/master/actionpack/lib/abstract_controller/callbacks.rb.

I think, the similar approach would work for us (and, btw, this will also add support for if,unless and other options we do not support currently).

Are there any guides/requirements for PRs(e.g formatting)?

Nothing specific: write new code similarly to the one we already have, follow the code style (it's covered by RuboCop).

[palkan]

Closed by #8