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

Adding before/after hooks for emails #65

Closed
jwoertink opened this issue Mar 7, 2022 · 4 comments · Fixed by #72
Closed

Adding before/after hooks for emails #65

jwoertink opened this issue Mar 7, 2022 · 4 comments · Fixed by #72

Comments

@jwoertink
Copy link
Member

I just had a case come up where we need to send emails when a user is active on our site, but there may be technical issues where their system is flopping. Currently we would be sending an email on each call which just ends up being spam. Instead, we need to throttle sending the emails so we only send once on some parameter.

For this, it would be cool if there was a before/after hook on the deliver. This would allow us to do something like before we send, make sure we're allowed to send the email. Then after sending, we would update some table with the throttling info.

@jwoertink
Copy link
Member Author

Doing this would actually solve the case for #61

class NotifierEmail < BaseEmail
  def initialize(@recipient : Carbon::Emailable)
  end

  before_send do
    # this would be up to you
    unless @recipient.wants_to_get_marketing_emails?
      # not sure about this... 
      skip_sending
    end
  end

  after_send do |delivery_response|
    # if this runs, the delivery_response would
    # be the return value of the deliver_now adapter method
  end

  to @recipient
  subject "Info for you"
  templates text, html
end

I also wonder if this would fit better in the Deliver Later strategy 🤔 It might be something you could do now possibly?

class CustomHookDeliverStrategy < Carbon::DeliverLaterStrategy

  def run(email : Carbon::Email, &block)
    before_send(email)
    response = block.call
    after_send(email, response)
  end

  def before_send(email)
    # custom logic
  end

  def after_send(email, response)
    # custom logic
  end
end

BaseEmail.configure do |settings|
  settings.deliver_later_strategy = CustomHookDeliverStrategy.new
end

The downsides to doing that path would be that you'd have to ALWAYS call deliver_later to get the callbacks. Calling deliver would skip those (maybe built-in escape hatch?). The next thing is where the logic for this all goes... On the one hand you could do it like this and have access to the email being sent, or you could move it in to all of the emails and call something like email.before_send instead of before_send(email). But if you do put the logic here, you can't guarantee that the recipient of the email is always the same type of object. For example, you may have a User and Admin as two separate models. Trying to short circuit the email here could get a little hairy.

@jwoertink
Copy link
Member Author

It also seems that this could coincide with #43 Where the idea is that you could subscribe to before email sending events, or after sending email events to do whatever it is you need (maybe save to Breeze?)

@robacarp
Copy link

At the strategy layer, in order to solve #61 you'd need to be able to tag an email with a type (marketing, password, transactional) and then short circuit it in there. I don't think that's a great way to solve it. I can see that there would be some benefits to having a before/after hook at the strategy layer but I don't think it's a good fit for filtering out unwanted emails. It feels like a place to hang framework level concerns instead.

Adding a before hook to the Email base class seems like it'd be really powerful. Eg:

# include into any email which can be opted-out of
module EmailOptOutHandler
  macro included
    before_send do
      unless @recepient.opt_in_email_types.includes? @type
        skip_email
      end
    end
  end
end

@jwoertink
Copy link
Member Author

I like that. It makes sense. I think I just need some sort of mechanism that tells the Email that when we call deliver, don't actually do it. So I guess the skip_email method would be the thing that sets the flag.

I'm thinking something like this:

abstract class Carbon::Email
  property? deliverable : Bool = true

  def skip_sending_email
    self.deliverable = false
  end
end

So the before/after hooks would all be empty similar to how they work in Avram. It would be up to you to decide what logic happens. The difference is Avram has add_error() and valid? which will short circuit the saving. In this case, we don't have that.

carbon/src/carbon/email.cr

Lines 124 to 126 in 6cda3dd

def deliver
settings.adapter.deliver_now(self)
end

This then becomes more like

def deliver
  before_send

  if deliverable?
    response = settings.adapter.deliver_now(self)
    after_send(response)
  end
end

Does that sound good?

jwoertink added a commit that referenced this issue Aug 15, 2022
* allows the emails to have before/after callbacks. Fixes #65

* adds in functionality to stop an email from being delivered. Fixes #61
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants