Current hook_pathauto_pattern_alter() implementation has unwanted side effects

Created on 9 February 2024, 5 months ago

On a project of mine the path alias for a whole series of content types is generated based on the title field (e.g. my-wonderful-content/[node:title]). We, however, allow users to override the title used in path alias generation by providing a separate field (field_path_alias_title & the (more or less) following hook_pathauto_pattern_alter() implementation:

/**
 * Implements hook_pathauto_pattern_alter().
 */
function mymodule_pathauto_pattern_alter(PathautoPatternInterface $pattern, array $context) {
  // Get the entity the pattern applies to.
  $entity = NULL;
  if (!empty($context['data'])) {
    foreach ($context['data'] as $value) {
      if ($value instanceof EntityInterface) {
        $entity = $value;
        break;
      }
    }
  }

  if ($entity instanceof FieldableEntityInterface
    && $entity->hasField('field_path_alias_title')
    && !$entity->get('field_path_alias_title')->isEmpty()
  ) {
    $normal_title_token = "[{$entity->getEntityTypeId()}:title]";
    $path_alias_title_token = "[{$entity->getEntityTypeId()}:field_path_alias_title]";
    $pattern->setPattern(str_replace($normal_title_token, $path_alias_title_token, $pattern->getPattern()));
  }
}

This works well when users are creating/updating content, 1 by 1, through the interface.

But... we have another piece of functionality that creates/updates a series of path aliases in 1 go and this is where it goes wrong. The $pattern passed into hook_pathauto_pattern_alter() is an entity object, and thus passed by reference. So in the example above, once the replacement is done for a node, its done for all the subsequent nodes because the entity object was altered before.

Is there a reason we are passing the full pattern entity object to hook_pathauto_pattern_alter() and not just the pattern string? I'm pretty sure making users alter entity objects (that won't be saved) is wrong. Looking by the code, I don't think there is a good reason to do this.

I'd like to propose to only just pass the pattern string & pass the pattern entity object via the $context argument. That way people still have access to the pattern entity object, but altering the pattern happens "isolated" per entity.

πŸ› Bug report
Status

Active

Version

1.0

Component

Code

Created by

πŸ‡§πŸ‡ͺBelgium rp7

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

Comments & Activities

Production build 0.69.0 2024