- ๐ฌ๐ง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 1:18pm 20 July 2024 - ๐ฌ๐ง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.