Updates to an entity's URL alias do not reflect on the corresponding local tasks.

Created on 29 November 2017, over 6 years ago
Updated 13 June 2024, 13 days ago

Problem/Motivation

The local tasks (specifically the 'View' link) associated with an entity do not update immediately when its URL alias is modified. To reflect the changes, the Drupal cache must be cleared. Without clearing the cache, the 'View' link points to the old alias which results in "page not found" error.

Debugging showed that cache tags of an entity are not incuded in local tasks.

Steps to reproduce

  • Install with standard profile
  • create an aricle, Give URL alias and save.
  • Edit the created article, Change the URL alias and save.
  • No click on 'View' link from the local tasks.
  • 'View' link leads to 404 as it is still using old alias.
  • Clear Drupal cache and try again.
  • 'View' link points to the new alias.

Taxonomy term also show similer behaviour

Proposed resolution

The cache tags associated with an entity should be linked to it's local task block.

Remaining tasks

None

User interface changes

NA

API changes

NA

Data model changes

NA

Release notes snippet

NA

🐛 Bug report
Status

Fixed

Version

10.3

Component
Base 

Last updated about 2 hours ago

Created by

🇧🇪Belgium geertvd

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

Merge Requests

Comments & Activities

Not all content is available!

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

  • 🇧🇪Belgium detroz

    I have the same problem with node and Sub-pathauto (Sub-path URL Aliases) : when the node's path is changed, the subpaths are always with the previous path and not the new one. For exemple, the local tasks links are wrong now. So, it's not very easily to edit or delete the node in that condition. Only a clear cache resolve that.

    I updated the patch #4 to be compatible with Drupal 9.5.x (I didn't yet test it on Drupal 10).

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update 11 months ago
    27,120 pass, 288 fail
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MySQL 5.7
    last update 11 months ago
    28,296 pass, 441 fail
  • 🇮🇳India Akhil Babu Chengannur
  • 🇮🇳India Akhil Babu Chengannur

    I tested this issue in Drupal 11 and it is still reproducible. Unlike #2, this issue was reproducible for nodes as well.

    • Install with standard profile
    • create an aricle, Give URL alias and save.
    • Edit the created article, Change the URL alias and save.
    • No click on 'View' link from the local tasks.
    • 'View' link leads to 404 as it is still using old alias.
    • Clear Drupal cache and try again.
    • 'View' link points to the new alias.
  • 🇮🇳India Akhil Babu Chengannur

    However, this issue was not reproducible for any of the entities in the demo_umami profile. While debugging the cache tags in the standard profile, it was found that the cache tags of the entity were not added to the local task block where the cache tag for the entity was correctly added to the local task in the umami profile.

    As @geertvd pointed out in #2, cache tags are added to the local tasks block through access checks. Currently, EntityAccessControlHandler::access() does not add any cache tags associated with the entity to the access results. Therefore, the local task block won’t receive any cache tags related to the entity.
    In demo_umami profile, since content_moderation module is enabled, local task access checks eventually invoke content_moderation_entity_access and it adds the entity as a cachable dependency to the access result.

    function content_moderation_entity_access(EntityInterface $entity, $operation, AccountInterface $account) {
      /** @var \Drupal\content_moderation\ModerationInformationInterface $moderation_info */
      $moderation_info = Drupal::service('content_moderation.moderation_information');
    
      $access_result = NULL;
      if ($operation === 'view') {
        $access_result = (($entity instanceof EntityPublishedInterface) && !$entity->isPublished())
          ? AccessResult::allowedIfHasPermission($account, 'view any unpublished content')
          : AccessResult::neutral();
    
        $access_result->addCacheableDependency($entity);
      }
    //
    
  • Pipeline finished with Failed
    5 months ago
    #82259
  • Pipeline finished with Running
    5 months ago
    #82305
  • Pipeline finished with Failed
    5 months ago
    #82306
  • 🇮🇳India Akhil Babu Chengannur

    Patch #19 attempted to add a cachable dependency from the EntityAccessControlHandler. But it resulted in numerous test failures.

    Link to the failed pipeline: https://git.drupalcode.org/issue/drupal-2927141/-/pipelines/82261

    Exploring another option, we could integrate cache tags directly from the local tasks block, iff the current route is related to an entity.

  • Pipeline finished with Running
    5 months ago
    #82323
  • 🇮🇳India Akhil Babu Chengannur
  • 🇮🇳India Akhil Babu Chengannur
  • 🇮🇳India Akhil Babu Chengannur

    The new change caused only one test failure .

    Drupal\Tests\config_translation\Functional\ConfigTranslationUiModulesTest::testNodeFieldTranslation
    Behat\Mink\Exception\ExpectationException: The string "FR label" was not found anywhere in the HTML response of the current page.

    These are the lines that failed. The test makes a field translatable and tries to add 'French' translation to it's label and help text.

        $this->drupalGet("/entity_test/structure/article/fields/node.article.$field_name/translate");
        $this->clickLink('Add');
    
        $form_values = [
          'translation[config_names][field.field.node.article.translatable_field][description]' => 'FR Help text.',
          'translation[config_names][field.field.node.article.translatable_field][label]' => 'FR label',
        ];
        $this->submitForm($form_values, 'Save translation');
        $this->assertSession()->pageTextContains('Successfully saved French translation.');
    

    Then it checks if the translation is saved or not.

        // Check that the translations are saved.
        $this->clickLink('Add');
        $this->assertSession()->responseContains('FR label');
    

    The code suses $this->clickLink('Add'); to check the added translation which is wrong. If a translation is already added, then the link text will be 'Edit' instead of 'Add'.

    Without the fix in MR 6315, the link text would remain as 'Add,' which is a bug that was somehow overlooked. Correcting the test.

  • Status changed to Needs review 5 months ago
  • 🇮🇳India Akhil Babu Chengannur

    This needs tests. But temporarly moving to needs review for core maintainers to check the proposed solution and provide further guidance.

  • 🇮🇳India Akhil Babu Chengannur

    Akhil Babu changed the visibility of the branch 2927141-taxonomy-local-tasks to hidden.

  • 🇮🇳India Akhil Babu Chengannur

    Akhil Babu changed the visibility of the branch 2927141-taxonomy-local-tasks to active.

  • Pipeline finished with Canceled
    5 months ago
    Total: 271s
    #84245
  • Pipeline finished with Failed
    5 months ago
    Total: 483s
    #84250
  • Status changed to Needs work 5 months ago
  • 🇺🇸United States smustgrave

    Appears to have a test failure.

    Have not retested.

  • Pipeline finished with Failed
    5 months ago
    Total: 259s
    #84610
  • Pipeline finished with Failed
    5 months ago
    Total: 484s
    #84614
  • Pipeline finished with Success
    5 months ago
    Total: 572s
    #84620
  • 🇮🇳India Akhil Babu Chengannur

    When a node/taxonomy term is created with an alias like this

    $this->drupalCreateNode([
            'type' => 'article',
            'path' => [
              'alias' => '/original-node-alias',
            ],
    ])
    

    The ‘view’ link in its local task will have the link ‘/subdirectory/original-node-alias’ when the test is executed. This lead to the test failure.

       Failed asserting that two strings are identical.
        --- Expected
        +++ Actual
        @@ @@
        -'/original-node-alias'
        +'/subdirectory/original-node-alias'

    I believe this issue arises because SIMPLETEST_BASE_URL is set as http://localhost/subdirectory in pipeline.yml
    So, to confirm that the link has been updated, I replaced assertSame() with assertStringContainsString() in the test. Pipeline is up now.

  • Status changed to Needs review 5 months ago
  • 🇮🇳India Akhil Babu Chengannur
  • Status changed to Needs work 5 months ago
  • 🇺🇸United States smustgrave

    Left a comment but why are we changing a translation button, and does changing it mean we are losing coverage for when the link is "Add"

  • Status changed to Needs review 5 months ago
  • 🇮🇳India Akhil Babu Chengannur
  • Status changed to RTBC 5 months ago
  • 🇺🇸United States smustgrave

    Crediting @catch for taking a look at the change I had a question on.

    Removing tests tag as they appear to be added.

    Clearing patches for clarity as fix is in MR 6315

    Tested out following the steps appears fixed to me.

    Thanks!

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Fixing component and updating issue credits

  • Status changed to Needs work 4 months ago
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Left some comments on the MR

  • First commit to issue fork.
  • Pipeline finished with Failed
    4 months ago
    Total: 646s
    #107154
  • Status changed to Needs review 4 months ago
  • Pipeline finished with Success
    4 months ago
    Total: 489s
    #108203
  • Status changed to Needs work 4 months ago
  • 🇺🇸United States smustgrave

    For the open thread in the MR.

  • Pipeline finished with Success
    4 months ago
    Total: 583s
    #110822
  • Status changed to Needs review 4 months ago
  • Status changed to RTBC 4 months ago
  • 🇺🇸United States smustgrave

    Looks good to me, thanks!

  • Pipeline finished with Success
    4 months ago
    Total: 651s
    #111972
  • Status changed to Needs work 4 months ago
  • The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

  • Status changed to Needs review 22 days ago
  • Status changed to Needs work 22 days ago
  • The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

  • 🇮🇳India Prashant.c Dharamshala

    Prashant.c made their first commit to this issue’s fork.

  • Status changed to Needs review 22 days ago
  • 🇮🇳India Prashant.c Dharamshala
  • Pipeline finished with Failed
    22 days ago
    Total: 648s
    #190452
  • Status changed to Needs work 20 days ago
  • Test failures issue needs to fix

  • Pipeline finished with Success
    20 days ago
    Total: 707s
    #192352
  • Status changed to Needs review 20 days ago
  • Status changed to RTBC 13 days ago
  • 🇺🇸United States smustgrave

    Rebase seems fine

  • First commit to issue fork.
  • Pipeline finished with Failed
    13 days ago
    Total: 166s
    #197563
  • Pipeline finished with Success
    13 days ago
    #197571
  • Test failures resolved after implemented suggestion & pipeline passed successfully.

  • Status changed to Fixed 13 days ago
  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    I think this is a major bug as you end up with a broken link immediately after saving an entity. Therefore backporting to 10.3.x to be included in 10.3.0 when it is released.

    Committed and pushed 8448f2d14f to 11.x and e664ed0182 to 11.0.x and 4792a5ccf4 to 10.4.x and 66531e2783 to 10.3.x. Thanks!

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍
    • alexpott committed 66531e27 on 10.3.x
      Issue #2927141 by Akhil Babu, pooja_sharma, geertvd, smustgrave, JeroenT...
    • alexpott committed 4792a5cc on 10.4.x
      Issue #2927141 by Akhil Babu, pooja_sharma, geertvd, smustgrave, JeroenT...
    • alexpott committed e664ed01 on 11.0.x
      Issue #2927141 by Akhil Babu, pooja_sharma, geertvd, smustgrave, JeroenT...
    • alexpott committed 8448f2d1 on 11.x
      Issue #2927141 by Akhil Babu, pooja_sharma, geertvd, smustgrave, JeroenT...
Production build 0.69.0 2024