Cannot save translated nodes after upgrading to 8.8 due to invalid path

Setting back to Needs Work, as the patch in 99 failed, and the test-only patch passed but should have failed.

3101344-99.patch doesnt fix it for me.

My project is currently on Drupal 9.5.3.
The patch applies without problem.

The project has 3 languages:
- NL (Dutch) -> is the default language
- FR (French)
- EN (English)

The client/content editor created a node in FR (so not the default language) without a problem and published it.
Now he tries to create an EN translation of that node that is unpublished.
If he tries to save that unpublished EN translation he gets the error
Either the path '/node/123' is invalid or you do not have access to it

The other patches here also did not fix this error.
I also tried this → but that did not fix it either.

In my case, editing a translation via 'Edit translations' does not work, which has this path structure:
/node/123/translations/edit/en
But editing the translation directly via admin/content does work, which has this path structure:
/node/123/edit?language_content_entity=en

Even as admin user, the /translations/edit link does not work while the ?language_content_entity edit link does.

In our case, the patches that work on \Drupal\path\Plugin\Field\FieldWidget\PathWidget::validateFormElement don't work, because the widget is active. Having the widget active should not keep this bug active. I don't agree Alex, this is not a problem present because of a widget, is an underlying issue in the node_grants system.
The patch from #64 fixes it for us.

FWIW, #64 🐛 hook_node_grants implementations lead to a 'URL Alias' validation error when saving translated nodes. Fixed resolves this issue for me on a multilingual Drupal 10.0 site.

Before applying the patch, users with correct node editing and translation permissions attempting to create a new translation receive the following message...

Either the path '/node/[some-nid]' is invalid or you do not have access to it.

...no message is written to the error log.

Patch at #64 fixes for me with 10.1.2. I also have View Unpublished installed.

I tried to reproduce this Drupal 10.1.x using the steps in the Issue Summary (again) and in #98 and was not able to.

I did update the patch, making the change asked for in #88. Also made changes to comments and switched to an MR.

Also have not been able to reproduce. Could someone confirm this is still an issue in D10?

Still an issue on D10 (10.1.6).

Not sure if it was mentioned before but for reproducibility, in my case this bug only manifests itself when creating translation from interface language different from original node language. For example, while this /en/node/276/translations/add/en/cs works (adding translation for originally EN node from EN interface), this does not /cs/node/276/translations/add/en/cs (adding translation for originally EN node from CS interface - and this path where button to add translation uses).

Confirmed steps should be added to the IS.

Alsolast MR has test failures.

Updated steps to reproduce in IS with point mentioning interface language being important when trying to reproduce bug.

The steps to reproduce that are in this issue are not sufficient. I can't reproduce the problem with these steps, I tested this on 11.x

1. Use sth. other than the administrator account. You should have the usual editor permissions to create nodes and create/update translations and translate any entity, of course. You should not have any url alias permissions.
2. Create a new node in the original language (in my case: German)
3. Save the node
4. Edit the node again
5. Click on translation, add translation (English)
6. Site interface of translation form should be now in target (English) language (e.g. path /en/node/123/translations/add/de/en)
7. Try to save

I just had @Mschudders reproduce this, as we are sitting next to each other at the moment. They can reproduce it on their instal without issues, but that is not a core checkout but rather a site with a bunch of contrib modules enabled.
I think this is only a problem with a certain set of contrib modules?

Adding screenshot which will help in debugging/finding the issue:

Even with pathauto installed and a configuration for articles, I can't trigger the error.

I've seen this problem on one of our client sites too. Perhaps @Mschudders and I could compare a list of contrib modules installed so we can cross-check?

Here is how I reproduced this issue in Drupal 10.2

• Install with demo_umami profile
• Give "Editorial workflow: Use Create New Draft transition" and "Article: Create new content" permissions to the editor role.
• Check the node_access table. There will be only one entry with nid 0 which is the default record.
• Create a custom module and implement hook_node_grants(). Install the module.

/**
 * Implements hook_node_grants().
 */
function my_module_node_grants(AccountInterface $account, $operation) {
  $grants = [];

  return $grants;
}

• Login as admin and visit any admin page. There will be a message to rebuild the permissions. Click on Rebuild permissions to rebuild all permissions.
• Login with any of the editor accounts (Eg: uid=3) in incognito mode.
• Add an English article. Give a path in URL alias field and publish it.
• Try to add spanish translation.

Patch #99 will only work if "URL alias" field is not rendered in the edit form (ie: User does not have 'Create URL alias" permission) as it skips the validation completely. It wont work if the field is accessible to the user.
I think patch #64 is the best way to fix this bug and here is why:-

validateFormElement() method in Drupal\path\Plugin\Field\FieldWidget\PathWidget invokes validations whenever there is an entry in the "URL alias" field. This eventually calls access() method in NodeGrantDatabaseStorage. This method checks if any modules have 'hook_node_grants' implementations and, if so, it checks the node_access table for access grants. The db query uses current language code of the node as a condition.

The "node_access" table is updated only when nodes are published. If you check the node access table now, there will be only one entry for node with id '20' (The one created in previous step) and language code will be 'en'. When we try to save the spanish translation, code checks the table with 'es' language code. There won't be any entry and access() method returns AccessResult::neutral()

This result is eventually passed to Drupal\Core\Routing\AccessAwareRouter checkAccess(). This method throws AccessDeniedHttpException if access is not allowed which causes the validation error.

Translation can be added without any errors if the original English content is kept in draft state. In this scenario also, there won't be an entry in node_access table for the new node in both 'en' end 'es' languages. So access() method in NodeGrantDatabaseStorage would stll return neutral access. But access checks also invoke 'content_moderation_entity_access' and it returns AccessResult::allowedIfHasPermission($account, 'view any unpublished content'). So user with 'view any unpublished content' permission would be able to save the translation. Later both translations can be published and updated without any errors.

Patch #64 fixes the error by checking the fallback language if the translation is new.
Committing that patch as an MR. Will also add a test to show the bug.

Akhil Babu → changed the visibility of the branch 11.x to hidden.

The issue is full of very useful comments and brainstorming hooks helping to get a good picture of why it can't be reproduced every time, etc. Great work in here and Thanks @Akhil Babu! +1 for your further steps and information provided in the last comments. Wake up call for all of us.

Just fyi helps to hide the old patches and MRs. There is a new section in the standard template for which MR to review but its new so not surprised it isn't here. Have hidden the patches for clarity

Ran the test-only features

1) Drupal\Tests\path\Functional\PathWithNodeAccessGrantsTest::testAliasTranslation
Behat\Mink\Exception\ResponseTextException: The text "Basic page test has been updated." was not found anywhere in the text of the current page.
/builds/issue/drupal-3101344/vendor/behat/mink/src/WebAssert.php:811
/builds/issue/drupal-3101344/vendor/behat/mink/src/WebAssert.php:262
/builds/issue/drupal-3101344/core/modules/path/tests/src/Functional/PathWithNodeAccessGrantsTest.php:102
/builds/issue/drupal-3101344/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
ERRORS!
Tests: 1, Assertions: 22, Errors: 1.

Left 2 small comments on the MR but overall looks good.

Patch in #64 works for me in 10.2.2 - thanks for the team effort here!

Feedback appears to have been addressed.

One question on the MR.

Feedback addressed. Moving back to needs review.

If going to do that way probably don't need the $langcode variable anymore.

Sure. $language variable was unnecessary, so I've removed it.

Feedback appears to be addressed.

Good to see this one ready. I did my best with issue credit but there are a lot of comments on this issue so apologies if anyone got overlooked.

Committed/pushed to 11.x and cherry-picked to 10.2.x, thanks!

Automatically closed - issue fixed for 2 weeks with no activity.

quietone → closed merge request !6210

quietone → closed merge request !5185