Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Idea: debouncing notifications #83

Closed
Schniz opened this issue Jan 11, 2021 · 2 comments
Closed

Idea: debouncing notifications #83

Schniz opened this issue Jan 11, 2021 · 2 comments
Labels
question Further information is requested

Comments

@Schniz
Copy link

Schniz commented Jan 11, 2021

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
Copy link
Owner

excid3 commented Jan 13, 2021

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.

@excid3 excid3 added the question Further information is requested label Jan 13, 2021
@SirRawlins
Copy link
Contributor

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?

@excid3 excid3 mentioned this issue Feb 5, 2021
Repository owner locked and limited conversation to collaborators Oct 31, 2023
@Kentasmic Kentasmic converted this issue into discussion #342 Oct 31, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants