Add setting to disable per-user rendering

Thank you @znerol! Works perfectly!

In my view #17 is not the complete solution.

The comment says

Site builders need to ensure that newsletter content is rendered identical for all users before enabling this cache backend.

It's rather easy for the site builder to make a mistake and break this rule, which could be a security bug. So the remaining part of the fix is to disable the account switch in MailEntity::setContext().

Here I propose a modified version of the issue solution
Add setting to disable per-user rendering(issue #3081703)

Changes that are proposed to be made:

1) Custom Mail Cache (MailCacheJust.php):
-Implementation of a custom mail cache class (MailCacheJust) that ensures caching of only necessary data and build information.
-This helps in maintaining consistency across newsletter deliveries, irrespective of whether subscribers have associated user accounts or not.

2)Custom Mail Entity (CustomMailEntity.php):

-Extention of the MailEntity class to disable account switching during entity rendering.
-This prevents issues related to recursive rendering errors, especially critical when rendering entities with references.

3)Custom Service Provider (CustomNewsletterServiceProvider.php):

-Creation of a service provider to replace Simplenews services with your custom implementations (MailCacheJust and CustomMailEntity).
-This ensures that Drupal uses your custom classes throughout the Simplenews module, improving performance and consistency.

4)Service Definitions (simplenews.services.yml):

-Definition of the services (MailCacheJust, CustomMailEntity, and CustomNewsletterServiceProvider) correctly in the service definitions.
-This registration ensures that Drupal’s dependency injection container correctly manages and uses your custom services.

Thanks. Yes, that has the right idea, but it's missing one thing. The purpose of this issue is to add a setting. We could call it mail.per_user_rendering.

• Add code for the setting to simplenews.settings.yml, simplenews.schema.yml, MailSettingsForm.php. For backward-compatibility the default should be TRUE.
• Create an update hook in simplenews_install() that sets the value for existing sites.
• Instead of disabling the switch, we need to switch to the anonymous user - see comments #15, #16.
• MailCacheBuild and MailEntity can read this setting (so we don't need CustomMailEntity and MailCacheJust).
• Add a test.

At a content level I imagine an "always use cache for all users" setting so that a site builder can say

Structure >> Content Types >> ContentName >> Edit >> Publishing options >> Use as Simplenews newsletter AND IGNORE per-user rendering

I understand one issue here is that there is a need to prevent publishing content that only certain recipents should see, but surely this is already sufficiently under the control of site builders who can specify different Display View Modes where fields can be omitted from display.
Or, if this is not sufficient then the Content's Publishing options could specify one Display View Mode for sensitive information and another for anonymous viewing ... gets complicated to implement I imagine.

Another poossibilty is a switch that could be set per field in the Content's Display View Mode in much the same way as a site builder can specify "Image Loading: eager" for a Media field so that you can say "Structure >> Content Types >> ContentName >> Manage Display >> Email:HTML >> FieldName >> cogwheel >> "IGNORE / MUST DO per-user rendering"

@iainH This setting is only for rendering emails. It doesn't affect normal rendering of the entity as a web page.

Yes, I understand. I think this would meet our needs.

@adamps I' working on this patch based on the requirements laid out in #22. I got the first 2 working. But I could use a bit more directions for 3 (disable the account switch: where? any examples?) and 5 (add a test: to test what? any examples?). Thanks!

I'm done implementing the new per_user_rendering setting in MailEntity, Mailer, etc. I able to send a test with per_user_rendering enabled and disabled without errors.

About the required test.

Since the new setting (Per user rendering) appears immediately below 'Use cron to send newsletters', I searched for occurrences of 'use_cron' and found 3 of them in tests/src/Functional:

1. testSendNowNoCron()
2. testSendMultipleNoCron()
3. testSendPublishNoCron()

Would adapting testSendNowNoCron() into e.g. testSendPerUserRenderingDisabled() meet the requirements ?

/**
 * Test newsletter sending with per user rendering disabled.
 */
public function testSendPerUserRenderingDisabled() {
  // Disable per user rendering - emails will be rendered as anonymous user.
  $config = $this->config('simplenews.settings');
  $config->set('mail.per_user_rendering', FALSE);
  $config->save();

  // Create newsletter with user-specific token
  $this->drupalGet('node/add/simplenews_issue');
  $edit = [
    'title[0][value]' => 'Test Newsletter',
    'body[0][value]' => 'Hello [user:account-name], this is your newsletter.',
    'simplenews_issue[target_id]' => 'default',
  ];
  $this->submitForm($edit, 'Save');
  $this->assertEquals(1, preg_match('|node/(\d+)$|', $this->getUrl(), $matches), 'Node created');
  $node = Node::load($matches[1]);

  // Send newsletter
  $this->clickLink(t('Newsletter'));
  $this->submitForm([], 'Send now');

  // Verify mails were sent (no breakage)
  $mails = $this->getMails();
  $this->assertCount(5, $mails, 'All mails were sent with per user rendering disabled.');

  // Verify user-specific tokens are not rendered (anonymous context) ?
}

The following commits have been pushed to Issue fork simplenews-3081703

• Add 'Per user rendering' option in settings and hook_update for backward compatibility.
• Implement switch to anonymous user when 'Per user rendering' is enabled.
• Update MailEntity instantiation.
• Add testSendPerUserRenderingDisabled().

Please advise on next steps. Thanks

Having a hard time with the GitLab fork workflow : / And then, to make matters worst, and not knowing better, I used the issue’s fork without realizing that it was stale (created July 2024).

Until I'm able to untangled this mess, I'll upload a patch. Standby…

Here is the patch…

Setting to Needs review…

Just a quick update & some questions.

I was able to rebase the issue’s stale fork. The good news is MR !88 is mergeable.

But in Gitlab, on the MR’s page, I see:

• Test summary: 144 failed, 242 total tests
• 78 out of 144 failed tests have failed more than once in the last 14 days

Are all these fails directly related to the MR ? Or is there a backlog of fails that are carried over from other issues ?

If someone can mentor me through this, I'll find more time to fix what needs fixing.

Thanks!