Add setting to disable per-user rendering

Created on 17 September 2019, about 5 years ago
Updated 1 August 2024, 4 months ago

When sending newsletter mails, if the subscriber is a user, this module switches to the user account for rendering the issue entity. This allows the output to vary, for example some users might see extra fields because they have a related permission.

The disadvantage of this is that it reduces performance, because the entity cannot be cached. In some cases, Drupal knows that the rendered entity could vary per user, so won't cache it, but the site admin knows that in reality there is no noticeable variation. In this case it would be useful to have a setting to disable the user switching.

The situation is made much worse because of a core bug ๐Ÿ› Can only intentionally re-render an entity with references 20 times Needs work . The bug will be hit if the issue entity contains an entity reference, and there are more than 20 authenticated subscribers. The proposed setting would avoid hitting the bug.

Original summary

Hello There

We create a newsletter content. On this we have a reference field (referenced to the a Simple Page).
We render this page as a teaser.

Wenn we send the newsletter as default by cron, some of the 400 recipient get the newsletter rendered correct. Some of them only get the content of the newsletter node and not of the rendered simple page. We get the Following error:
Recursive rendering detected when rendering entity node: 49, using the field_newsletter_content field on the simplenews_issue bundle. Aborting rendering.

It is not a problem of the mail client. We also have sent the 400 newsletter to one singe account, who get the mail rendert correct.
Also on this account, when we send all 400 news we get some rendered incorrectly.

Any Ideas, why the newsletter is not correct on every recipient?

Thanks for ur help.

โœจ Feature request
Status

Needs work

Version

4.0

Component

Code

Created by

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine vlad.dancer Kyiv

    Thank you @znerol! Works perfectly!

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom adamps

    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().

  • First commit to issue fork.
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia Aditiup

    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.

  • Status changed to Needs work 4 months ago
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom adamps

    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.
  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom iainH

    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"

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom adamps

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

  • ๐Ÿ‡ฌ๐Ÿ‡งUnited Kingdom iainH

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

Production build 0.71.5 2024