Allow separate theming of each individual outgoing email instead of caching the prepared newsletter

No new feature will be added to 6.x-1.x.

Is there an update to this patches integration?
So has this feature been applied to 6.x-2.0-alpha or is it in the 6.x-2.x dev release or does it require patching post install still?

rfay

I would like to discuss this approach.
I'm currently pretty unlucky about the SN7 mail creation code. This needs some love.

While some people even ask for BCC support (which is the absolute opposite and we won't support ever), your request makes also sense.

Currently mails are created under user anonymous and only alter is allowed per user - so things like tokens get applied. For most cases this is enough.
Tokens allow pretty much personalization - even whole parts of the mail, like embedding a user specific view..

Do you have information about the performance? How much gain is the current caching?
(we need data from a blank drupal AND a drupal with many modules enabled)

Regarding UI i feel it's a huge problem if users need to decide in which caching mode a specific newsletter would be sent. We should avoid such technical settings and possibly make it a global switch or a switch per newsletter (list)... Still unsure what makes sense... Modules might need to adjust that.

While the analysis might be fine in D6, we first would need the D7 solution before consider to backport it.

Any further thoughts or developments on this issue? I've got a site that sends different content based on role (via Premium Content module). I might make a modified version of this patch to just work with content cached by role so that I have fewer performance issues.

If the maintainers don't want to support this functionality directly, perhaps you'd be willing to have a clear API for it so that a custom module could deal with site-specific permission and performance issues without having to break or patch simplenews?

Meanwhile, i start to have an idea about a possible creation/caching api that by default a per-recipient rendering while still allow to cache based on custom rules.

This will also allow to have the full range from same-for-all to fully-individual mail with maximum performance.
There will be some samples and basic patterns, but you could hook and implement custom caching.

Trying to provide some more details soon.

related to
#256795: Allow simplenews cron to send nodes that anonymous users cannot access

Thanks-- this sounds promising.

@miro_dietiker

Tokens allow pretty much personalization - even whole parts of the mail, like embedding a user specific view.

This is what I've been trying to accomplish, embedding a user specific view, but the only way I could see to do it was by impersonating the receiving user and using views_embed_view to pass the user ID as an argument... are you saying there's another way to accomplish this without needing to impersonate the receiving user?

If you implement a token that replaces the token string with a view with arguments, then that's possible.
However your custom code cannot take the user from $user but much more needs to consider the message context...

There's simplenews_realname that provides some similar functionality with tokens... i think a good development copy base.

I ended up taking @rfay's approach.

For myself, because the whole of the newsletter is dominated by the user-specific View I don't think I'd see much difference in resource consumption between a token replacement method and this. Sending out ~100 emails in ~1.5 seconds, even though each newsletter impersonates the receiving user object and generates a unique Views query, I'd be surprised if the cached method is comparatively much faster than this, the major bottleneck is probably altering each email and passing it through mime mail.

So long as it's optional and has appropriate warnings, I don't see how it's a particularly dangerous feature to add?

Tagging (Hope that's correct).

Pushing this against D7 and assigning it to current bugfixing/feature development cycle.
I expect no backports of this feature for now...

We should also consider this requirement for long term development:
#976650: Multiple BCC recipients in one e-mail

NOTE: We should also consider multilingual newsletters here.

Related to
#1244910: Replacement patterns don't work on HTML format mails

Writing results of an internal discussion with Berdir

The level of Caching depends the usecase. This possibly even can vary per newsletter item. Still we don't want to bother regular users with special settings.
Suggestion is as follows:

Global Option
- (Dropdown): Caching Strategy
- (Checkbox): Allow caching strategy override per newsletter

(If per newsletter caching override enabled)
Per Newsletter Options
- (Dropdown): Caching Strategy
- (Checkbox): Allow caching strategy override per newsletter item

(If per-newsletter item caching override enabled)
Per Newsletter item options:
- (Dropdown): Caching Strategy

Ok, I've been cooking something up in the last few days.

This approach actually goes quite a bit further than just having pluggable caching. It's more something along the lines of #1188488: rework sending code - build mail content api that also makes the caching pluggable. So I'll be able to close a bunch of issues as duplicates of this one later on...

It defines three 3 new concepts, each is an interface and therefore pluggable, although not everything is actually configurable yet.

- SimplenewsSpool: This interface receives an array of mail spool rows, as returned by simplenews_get_spool(). It is then responsible for returning so called sources, see below. This will later on allow to add different implementations which can e.g. use BCC to process multiple mail spool rows/recipients in a single mail.

- SimplenewsSource: This interface basically returns everything that Simplenews needs to know to generate a newsletter mail. The interface is very generic and doesn't care about mail spools, nodes or even newsletter categories. There is however a slightly extended interface SimplenewsSourceNode which cares a bit more. Specifically, you need to pass a node and a subscriber to it. To write a custom source that works together with SimplenewsSpool, you need to implement this one. But it should be possible to send a newsletter through simplenews by implementing the basic interface manually. But it's kinda pointless at this point as Simplenews doesn't really do anything for you then, no spooling, no UI, no nothing. But maybe this can be extended later or something. Sources can either be sent by adding rows to the mail_spool and then calling simplenews_mail_spool() or by invoking simplenews_send_source() directly, which is what the "send test" now does.

- SimplenewsSourceCache: And finally, this is the pluggable caching. It's quite simple, it is constructed with the source as the argument and then has a get() and set() method, which are called by the source. There are currently three groups of caches: data (kinda raw stuff, like attachments. Yeah, attachments are now implemented again), build (the built and themed node data, basically what is cached now) and final (everything cached, including tokens). Cache implementations just need to decide what they want to cache. There is an abstract implementation for a static cache, so child classes just need to implement a single method to decide if something should be cached or not. But it would also be possible to cache over multiple requests, using cache_*() functions. not sure if it's worth doing that, though.

Every interface has an implementation, only the cache interface has two (cache nothing vs. cache data/build).

The patch also cleans up a bunch of other things. In total, there are +1k loc and -400. The new includes/simplenews.source.inc file is 800loc but 50% are api docs.

I'll list the todo's and relevant changes other than those interfaces in the next comment, need to run now...

Attached patch should apply and incorporates some changes from #437456: Allow to use a field for the text part of a multipart message to set the plaintext version explicitly instead of having it generated by mimemail. The linked issue will provide a different template and

Reviewing a bit...
I don't like 999999 still to show up for "unlimited" instead we should define a NULL=unlimited ... and possibly in forms a key "unlimited" if needed...
Thought you wanted to remove simplenews_recipients_alter completely as it's unused now. Separate issue?
Later again, once 99999 and once 999999 (one nine more)... We should get rid of it, really. I did so in D6, seems we missed that port.

About the global user object... A cache implementation should have the capability to switch to the real recipient user.. Such a user might have specific field visibility based on his permission records, traditionally roles. Note that they'll nee to have different cache entries, so the cache implementation needs to know how to build the key...
About the language switch: i'm pretty sure we should also switch GLOBAL['language'] due to some modules relying on this.

In function setNode you could easily set $nid = $tnid; ... no further foreach needed if no translation available. Note that we stay with the language at the users language, not the node one. $langcode is even unused here.

In getLanguage() do we have a guarantee that $this->getSubscriber()->language; ... is always fine?

Note that attachments are also only supported by mime.. Here we need a test too.
Are you sure that we should add ALL attachments on a node? There are at least three situations that can result for a file
- attach it, content disposition attach (e.g. for an image, still show as file attachment in file list)
- attach it, content disposition inline (e.g. for an image, show directly in body)
Using HTML body also allows us to refer to files as a link, especially with HTML, link or image sources. But that's possibly nothing to do with real file fields.
An option is also to define formatter to choose the disposition per field. Or add the setting per file in the widget...
Dunno what's best to do with files...

We should also test the fail of sending a mail to a single address.

- 9999... removed and replaced with a constant (which has the value -1). The check now does a $limit > 0 check instead of is_numeric() (which would allow negative values, hex and all kinds of funny things).

- Added a backport of http://drupal.org/node/287292 which allows safe, nested user switching. Added the user switch to getBody()/getPlainBody(), is it necessary elsewhere, e.g. the footer?

- Removed the recipients alter code. There are lots of possible clean-ups we need to stop somewhere.. :)

- It probably makes sense to do a language switch, yes. However, not to the same value as $node->language, imho, but to getLanguage() I think. Not done yet.

- Yeah, $langcode was used in the old static cache, but this is now done based on the node object. And the cache key now uses getLanguage(). About that, not sure. there always is a subscriber, but what happens if you have language set to 'de' and the there is no translation for that node and you use the en node. then it's cached with de as language, but that imho makes sense if there are t() strings ( which require that we set the global language as well). So I think this is actually the correct behavior.

- About attachments, yes, a custom formatter makes sense.

Yes, more tests would be great.

Updated the patch. interdiff attached as well to see the changes.

#20: source3.patch queued for re-testing.

Commited the above patch!

Opened #1374926: Test and improve active language handling when building the newsletter and #1374930: Add tests for new source concept as follow-up tasks, although there might be more.