Problem/Motivation

With the addToSpool functionality now in the Recipient Handler (#1931006: Problem with recipient handlers) it seems to make sense that we allow recipient handlers define behaviour to send emails immediately. This will allow us to use a recipient handler for the test email that can be sent. (#1931042: Convert sending a test email to use the Recipient Handler system)

Proposed Solution

Add a sendNow method to the RecipientHandler interface that will attempt to send the email immediately. Recipient handlers should be able to refuse this behaviour as it may cause errors for large numbers of recipients - an alternative would be to use this method to redirect to a batch process that sent the email immediately.

Comments

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Hm. We need something along those lines but if there are two methods, how do we know which one we need to call?

Maybe just have a single send method and let the handler decide if he wants to send immediately or not?

This will probably require to move more logic into the handlers and allow them to control the displayed message, if we need to check the spool and so on. So maybe this is a won't fix and we should instead simply rename the new method in #1931006: Problem with recipient handlers?

rlmumford’s picture

rlmumford
Primary language English
Location Manchester
CreditAttribution: rlmumford commented

Hmmm, I thought that would be decided by whatever form you used to trigger the sendNow/addToSpool process.

I'm a little uncomfortable about wrapping it all up in one method mainly because the recipient handler is then deciding two things rather than one thing ("Who it gets sent to" and "how it gets sent" rather than just "who it gets sent to") Maybe we should inject the RecipientHandler into a method on the Spool Class and then allow you to choose which "spooling" class you use, one of which doesn't spool at all.

The problem with that is the the 'Uber-Performant' solution we have for spooling is going to require the two to be coupled quite strongly. Maybe the 'addToSpool' method in one of our (hypothetical) Spooling classes could insist on a SimplenewsRecipientHandlerQueryInterface object?

We could have a SimplenewsSendNow spool class that required that the RecipientHandler it got passed was an instance of the SimplenewsRecipientHandlerInstantInterface (that would return an array of emails or something).

I feel like if we tie how it gets spooled into one addToSpool method someone's going to come along who wants to use one of our Recipient Handler plugins but with a spooling mechanism the handler can't support.

Just thinking aloud here.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

That the method would then decide on two things is a valid point. But still, who if not the handler would then decide how? It just doesn't make sense to spool a test e-mail.

And saying that we'd always route everything through the spool but then immediately send it is a bit strange too, especially since we currently usually don't control what we are sending (e.g. which newsletter/recipient), we just send it out.

Not sure yet, maybe push the other issues first and see how this develops.

andrewbelcher’s picture

andrewbelcher CreditAttribution: andrewbelcher commented

Not sure yet, maybe push the other issues first and see how this develops.

I think this would be my inclination. I like the idea of it, but until we have some concrete use cases (and I'm not convinced about the test email - see the other issue) I think we're at risk of building something needlessly complicated or annoyingly inflexible...