- 🇬🇧United Kingdom nicrodgers Monmouthshire, UK
Setting back to Needs Work, as the patch in 99 failed, and the test-only patch passed but should have failed.
- 🇧🇪Belgium flyke
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. - 🇧🇪Belgium flyke
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=enEven as admin user, the /translations/edit link does not work while the ?language_content_entity edit link does.
- 🇧🇪Belgium daften
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. - 🇺🇸United States jasloe
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.
- 🇮🇪Ireland frankdesign
Patch at #64 fixes for me with 10.1.2. I also have View Unpublished installed.
- 🇳🇿New Zealand quietone
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.
- Status changed to Needs review
about 1 year ago 12:06pm 30 October 2023 - @quietone opened merge request.
- Status changed to Postponed: needs info
about 1 year ago 2:01pm 31 October 2023 - 🇺🇸United States smustgrave
Also have not been able to reproduce. Could someone confirm this is still an issue in D10?
- Status changed to Needs review
about 1 year ago 2:17pm 7 November 2023 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).- Status changed to Needs work
about 1 year ago 2:21pm 7 November 2023 - 🇺🇸United States smustgrave
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.
- 🇧🇪Belgium borisson_ Mechelen, 🇧🇪
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
- 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.
- Create a new node in the original language (in my case: German)
- Save the node
- Edit the node again
- Click on translation, add translation (English)
- Site interface of translation form should be now in target (English) language (e.g. path /en/node/123/translations/add/de/en)
- Try to save
- 🇧🇪Belgium borisson_ Mechelen, 🇧🇪
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? - 🇧🇪Belgium Mschudders
Adding screenshot which will help in debugging/finding the issue:
- 🇧🇪Belgium borisson_ Mechelen, 🇧🇪
Even with pathauto installed and a configuration for articles, I can't trigger the error.
- 🇳🇱Netherlands tinto Amsterdam
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?
- 🇮🇳India Akhil Babu Chengannur
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.
- 🇮🇳India Akhil Babu Chengannur
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 inDrupal\path\Plugin\Field\FieldWidget\PathWidget
invokes validations whenever there is an entry in the "URL alias" field. This eventually callsaccess()
method inNodeGrantDatabaseStorage
. 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 returnsAccessResult::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. - 🇮🇳India Akhil Babu Chengannur
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. - 🇮🇳India Akhil Babu Chengannur
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. - Merge request !6210Issues/3101344: Check fallback value for new translations. → (Open) created by Akhil Babu
- Status changed to Needs review
10 months ago 5:05pm 17 January 2024 - 🇮🇳India Akhil Babu Chengannur
Akhil Babu → changed the visibility of the branch 11.x to hidden.
- last update
10 months ago Composer error. Unable to continue. - last update
10 months ago CI aborted - last update
10 months ago 29,235 pass, 26 fail - last update
10 months ago CI aborted - 🇫🇷France dqd London | N.Y.C | Paris | Hamburg | Berlin
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.
- 🇺🇸United States smustgrave
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.
- 🇩🇪Germany quotientix
Patch in #64 works for me in 10.2.2 - thanks for the team effort here!
- Status changed to RTBC
10 months ago 2:43pm 30 January 2024 - Status changed to Needs work
10 months ago 6:55pm 30 January 2024 - Status changed to Needs review
10 months ago 7:20am 31 January 2024 - 🇮🇳India Akhil Babu Chengannur
Feedback addressed. Moving back to needs review.
- Status changed to Needs work
10 months ago 2:32pm 31 January 2024 - 🇺🇸United States smustgrave
If going to do that way probably don't need the $langcode variable anymore.
- Status changed to Needs review
10 months ago 5:32am 1 February 2024 - 🇮🇳India Akhil Babu Chengannur
Sure. $language variable was unnecessary, so I've removed it.
- Status changed to RTBC
10 months ago 2:55pm 1 February 2024 - 🇬🇧United Kingdom catch
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!
- Status changed to Fixed
9 months ago 5:22pm 6 February 2024 Automatically closed - issue fixed for 2 weeks with no activity.
- First commit to issue fork.