Skip to content

Conversation

@RomainLvr
Copy link
Contributor

@RomainLvr RomainLvr commented Nov 28, 2025

Checklist before requesting a review

Please delete options that are not relevant.

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.
  • I have added tests that prove my fix is effective or that my feature works.
  • This change requires a documentation update.

Description

  • It fixes !40371
  • Here is a brief description of what this PR does

Fixes an issue where notification template caching was shared between users of the same type and language, preventing plugins from generating user-specific data (like unique tokens or URLs) for each recipient.

This is a less invasive alternative to #20186 which was closed because the solution was too impactful.

Problem

When multiple users receive the same notification (e.g., two observers on a ticket), the template is cached based only on language and additional options. Plugins using the ITEM_GET_DATA hook (like ApprovalByMail) cannot generate unique data per recipient because getForTemplate() is only called once per language.

As a result, all recipients receive identical tokens/URLs in their emails.

Solution

Following @cedric-anne's suggestion from #20186:

  • Include user identifier (users_id or email) in template cache key
  • Pass $user_infos to getForTemplate() and store it in a new current_user_infos property on NotificationTarget
  • Plugins can now access $target->current_user_infos during ITEM_GET_DATA hook to identify the current recipient

Changes

  • src/NotificationTemplate.php: Add user identifier to cache key, pass user_infos in options
  • src/NotificationTarget.php: Add current_user_infos property, populate it in getForTemplate()
  • .phpstan-baseline.php: Remove obsolete entries (return type is now correctly string)
  • Added unit tests

Related

@RomainLvr RomainLvr self-assigned this Nov 28, 2025
}

$tid = $language;
$tid .= serialize($additionnaloption);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can simplify the solution and do it only in the plugin by passing the user ID in additionaloption?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually considered this approach, but unfortunately it won't work due to how the template caching mechanism works.

The issue is that additionnaloption is already included in the cache key (via serialize($additionnaloption)). So in theory, adding users_id to additionnaloption from the plugin should create unique cache keys per user.

However, the problem is when the plugin hook is called:

  1. NotificationEventAbstract::raise() loops over each user and calls getTemplateByLanguage()
  2. getTemplateByLanguage() computes the cache key using $user_infos['additionnaloption'] (which comes from the core, not the plugin)
  3. Only if the template is not cached, it calls getForTemplate() which triggers the ITEM_GET_DATA hook where plugins can add data
  4. The plugin's hook (plugin_approvalbymail_get_datas) is called after the cache key is computed and inside the cache miss block

So even if the plugin wanted to add users_id to additionnaloption, it's too late - the cache decision has already been made based on the original additionnaloption from the core.

The only way to solve this from the plugin side would be to somehow modify $user_infos['additionnaloption'] before getTemplateByLanguage() is called, but there's no hook available at that point.

That's why the fix needs to be in the core: either by including the user identifier in the cache key (as done here), or by providing a hook before the cache key computation.

@RomainLvr RomainLvr requested a review from Rom1-B December 1, 2025 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants