Idea: debouncing notifications

[Schniz]

Hey! first of all - thanks for this awesome lib. Really liked that it does things "the rails way" and lets you focus on the notifications themselves.

I wonder if that's something that can be handled in the library itself — but one might want to debounce notifications or at least send some in batches. Imagine a busy thread in FB or Twitter — you don't want to get a message for every message there.

A nice approach would be to have a notification that is stored to DB and a cron job that runs every 5m/10m/whatever to get all the necessary notifications and group them into one big email instead of many small ones.

I wonder if that's something you have considered to bake into the library itself. It's not hard to implement that in userland (or at least I think so, haven't tried yet) but I wonder how it would look if it was baked in

I guess it's just a discussion, because there is no real suggestion here.

Thanks again for a wonderfully crafted library.

[excid3]

Certainly doable, but nothing in particular built-in right now.

To build throttling, some functionality to build this is already there. For example, you could use the if and unless hooks to determine if there are too many notifications to a user and skip sending the email if there was.

The actual sending of batched notifications via email (or other delivery methods) isn't. That will also probably depend per delivery method anyways.

[SirRawlins]

As above, thanks for the great library, Chris.

Batching is something I'd love to see.

If we have a notification to deliver to several thousand recipients, then some mail services such a Postmark/Sendgrid/Mailgun allow you to batch send 1,000 messages at a time, converting 000s of API requests into a single call.

Being able to defineDeliveryMethod classes that were instantiated and enqueued with a collection of recipients, instead of just a single recipient would be nice.

Something like this pseudocode is what I imagine:

class DeliveryMethods::Postmark < Noticed::DeliveryMethods::Base

    # Define batch size on the delivery method class.
    deliver_in_batches_of 1_000

    def deliver
        post "https://api.postmarkapp.com/email/send", {
            # Has access to 1_000 recipients, instead of single recipient.
            to: recipients.map(&:email),
            subject: notification.subject
        }
    end

end

class GroupNotification < Noticed::Base

    deliveryby :database
    deliver_by :twilio
    deliver_by :postmark

end

User.all.count
# => 5000

GroupNotification.deliver_later(User.all)
# => Enqueues DeliveryMethods::Database.deliver 5,000 times.
# => Enqueues DeliveryMethods::Twilio.deliver 5,000 times.
# => Enqueues DeliveryMethods::Postmark.deliver 5 times.

This would require some refactoring of the delivery, deliver_later and run_delivery methods in Noticed::Base to batch and loop recipients if the DeliveryMethod supported it.

Not sure if the gains in efficiency and scalability would make it worthwhile though?

[conarro]

A nice approach would be to have a notification that is stored to DB and a cron job that runs every 5m/10m/whatever to get all the necessary notifications and group them into one big email instead of many small ones.

I wonder if that's something you have considered to bake into the library itself. It's not hard to implement that in userland (or at least I think so, haven't tried yet) but I wonder how it would look if it was baked in

Hey @Schniz -- I just rigged something up for this using Noticed to "debounce" task assignment notifications. After a few approaches, here's what I settled on...

Goals

• I want to be able to lean on the underlying Notification objects for easy presentation
• I want a Notification object to be able to declare that it should be batched, and how the batch should be delivered
• I don't want batches to be stored as a Notification directly, as that would pollute user.notifications in the DB, which I want to be clean so I can display it in the web UI
• Bonus points:
  • I want to skip batch delivery if the batch's notifications are already read
  • I want to mark a batch's notifications as read if the batch itself is read (I ended up handling this via Postmark webhook, so when a batched email is clicked I just mark the batch and its notifications as read)

Approach (working in production ✨):

• a custom delivery method, DeliveryMethods::Batch, that a Notification object can invoke using deliver_by :batch
  • the options passed to this delivery method determine how notifications will be grouped, and also how the batch will ultimately be delivered
  • see full code below
• new models for storing batches of notifications: Notifications::Batch and Notifications::BatchItem (a join model for Batch<>Notification)
  • Batch has recipient, key and status (enum) used for lookups/grouping, as well as completed_at and read_at
• a cron job, Notifications::DeliverBatchesJob, that finds pending batches and delivers them
  • full code below
• a BatchNotification Noticed model that the cron job uses to deliver the batch
  • in my case, I don't have deliver_by :database in this, as I didn't want the batch itself to show up in user.notifications -- this Noticed notification is there to encapsulate the logic of what channels to send the batch over (email only for me, at this point)

Example

Here's an example of how a Notification object can declare that it should be batched:

class TaskAssignedNotification < Noticed::Base
	deliver_by :batch, class:        "DeliveryMethods::Batch",
                     batch_key:      :generate_batch_key,

					# how long to wait since the last item was added to the batch
                     inactive_wait:  10.minutes,

					# after this time, we'll just send the notification regardless of inactivity
                     max_wait:       30.minutes,  

					# options to persist with the batch, that will ultimately be passed through to BatchNotification
					# this lets the Notification declaring that it should be batched have control over how its batch is delivered
                     deliver_by: { email: { mailer: "TasksMailer", method: :batch } }

	param :task

	def generate_batch_key
		"tasks" # this has access to objects so you can do more interesting things as well like group by "task_#{task.id}"
    end
end

Code samples

DeliveryMethods::Batch

class DeliveryMethods::Batch < Noticed::DeliveryMethods::Base
  def deliver
    # create (or find) the batch record for the given key and recipient
    batch_key = options[:batch_key].is_a?(Symbol) ? notification.send(options[:batch_key]) : options[:batch_key]

    Notifications::Batch.transaction do
      @batch = Notifications::Batch.create_or_find_by!(recipient: recipient, key: batch_key, status: 'pending') do |batch|
        batch.inactive_wait = options.fetch(:inactive_wait, 10.minutes)
        batch.max_wait = options.fetch(:max_wait, 1.hour)
        batch.options = options # store the passed options for reference later
      end

      # add the notification record to the batch
      Notifications::BatchItem.create_or_find_by!(batch: @batch, notification: record) do |item|
        item.notification = record
      end
      @batch.update!(items_modified_at: Time.current)
    end
	# don't actually delivery anything, that'll be handled by Notifications::DeliverBatchesJob
    @batch
  end

  private

  def self.validate!(options)
    super
    raise ::Noticed::ValidationError, "batch_key missing" unless options[:batch_key].present?
    raise ::Noticed::ValidationError, "deliver_by missing" unless options[:deliver_by] && options[:deliver_by].any?

    if options[:deliver_by][:email]
      raise ::Noticed::ValidationError, "deliver_by missing :email mailer" unless options[:deliver_by][:email][:mailer]
      raise ::Noticed::ValidationError, ":deliver_by missing :email method" unless options[:deliver_by][:email][:method]
    end
  end
end

BatchNotification Noticed object

class Notifications::BatchNotification < Noticed::Base
  deliver_by :email, mailer: :mailer_from_options, method: :mailer_method_from_options

  param :batch

  after_deliver do
    batch.update!(status: :delivered, completed_at: Time.current)
  end

  # The creator of the batch passes through this data via options
  # E.g. A Notifications::Batch.new(options: { deliver_by: { email: { mailer: "TasksMailer", method: :batch }}} })
  # will invoke `TasksMailer.batch`
  #
  def mailer_method_from_options
    delivery_options_for(:email).fetch(:method)
  end

  def mailer_from_options
    delivery_options_for(:email).fetch(:mailer).constantize
  end

  private

  def delivery_options_for(method)
    batch_options[:deliver_by][method]
  end

  def batch_options
    batch.options.with_indifferent_access
  end

  def batch
    params[:batch]
  end
end

Notifications::Batch model

class Notifications::Batch < ApplicationRecord
  belongs_to :recipient, polymorphic: true

  validates :key, presence: true
  validate :validate_deliver_by_options, on: :create

  has_many :items, class_name: 'Notifications::BatchItem', dependent: :destroy
  has_many :notifications, through: :items

  enum status: {
    pending:   0,
    delivered: 1,
    cancelled: 2
  }

  # convenience method so I can call `batch.deliver_later` in the cron job
  def deliver_later
    Notifications::BatchNotification.with(batch: self).deliver_later(self.recipient)
  end

  def wait_is_over?
    inactive_wait_exceeded? || max_wait_exceeded?
  end

  # if *any* notification in the batch is read, consider the batch read
  def unread?
    notifications.read.count == 0
  end

  private

  def inactive_wait_exceeded?
    return false unless items_modified_at.present?

    Time.current - items_modified_at > inactive_wait
  end

  def max_wait_exceeded?
    Time.current - created_at > max_wait
  end

  def validate_deliver_by_options
    errors.add(:options, "must include `deliver_by` options") unless options.has_key?("deliver_by")
  end
end

Cron job logic

def perform
	Notifications::Batch.pending.find_each do |batch|
        if batch.wait_is_over?
          if batch.unread?
            batch.deliver_later
          else
            # at least one notification has been read so let's cancel this batch
            batch.update!(status: :cancelled, completed_at: Time.current)
          end
        end
      end
end