- Status changed to Needs review
over 1 year ago 1:00pm 5 February 2023 The last submitted patch, 82: 2915792-82.patch, failed testing. View results β
- Status changed to Needs work
about 1 year ago 5:17pm 22 March 2023 - π¬π§United Kingdom marekgti
Modified #82 patch to allow unrouted links that starts with "/"
- π³π±Netherlands Lendude Amsterdam
Closed π Custom menu item entity reference is not accessible to anonymous user Closed: duplicate as a duplicate of this. Discussed with @catch in #bugsmash and the possible problems with entity references should probably be handled here too.
- π¨π¦Canada Austin986
Patch #85 not working on Drupal 10.0.9.
It says patch can not be applied.
Any idea? - π¨π¦Canada Austin986
Patch #85's compatible version for Drupal 10.1.x.
- π¨π¦Canada Austin986
Patch #88 still not working if paragraph is a part of extra field of meu item.
Because it tries to check permission to accessentity.menu_link_content.canonical
, and it seems there is no way to set custom permission forentity.menu_link_content.canonical
, without any additional module installed.This patch simply ignores all access permission for "view" case, to make sure all menu items are visible.
PLEASE DO NOT USE THIS PATCH, if you need menu item specific permission set up.
- last update
about 1 year ago 29,378 pass - last update
about 1 year ago Custom Commands Failed - π§πͺBelgium tim-diels Belgium π§πͺ
@Lendude regarding "Discussed with @catch in #bugsmash and the possible problems with entity references should probably be handled here too." ... Could you give some more info on what needs to be done to get this issue moving again?
- π©πͺGermany Anybody Porta Westfalica
@Lendude & @Voleger, we just came to this issue from here: #2925283: New fields visibility and access β
TL;DR: Due to this, entity reference (revision) contents are not visible for guests within the Menu (example: Mega Menu implementation using Paragraphs). Workaround: Use paragraphs_type_permissions submodule, which seems to override this using
hook_ENTITY_TYPE_permission()
Still we'd like to help fixing this in core, so we can remove the workaround.
You have both invested a lot of time here and I think you're aware of what has to be done to finish this. What ca we do?
We can reroll the MR against 11.x and 10.3.x but then we should try to finish it and hot have it around for years again... any feedback?The MR still seems good, a clear improvement and has a lot of positive feedback.
- π©πͺGermany Anybody Porta Westfalica
Here's the current state of the MR339 which still works fine against 10.2.x, 10.3.x and eventually 11.x (haven't tried). Great work @voleger!
- πΊπ¦Ukraine voleger Ukraine, Rivne
rest and json_api integration tests are failing
- π³π±Netherlands bbrala Netherlands
Test are failing because the new link to user/1 is missing in the menu tests. So that should be a very easy fix.
See this link for all tests that are failing.
- π©πͺGermany Grevil
Not as simple as it seems. The expected cache tags are invalid, since we are swapping the "https://nl.wikipedia.org/wiki/Llama" link with a cacheable internal link to user:1 out. We can implement a dirty workaround and manually prepend the new cache to "getExpectedCacheTags()", but then the test will just throw an error later on.
To be honest, the current test changes generally seem really rushed. Simply replacing a remote link string with an internal link inside already existing tests / test bases without changing anything else just doesn't feel right. It also looks like we removed testing for external links entirely?
I'd say we revert the current test changes and create dedicated tests for internal menu links (json and rest) additionally to the already existing external link tests. Only problem being, that the test structure seems quite complex, so this will probably need quite a bit of work...
Really unsure about this. Does anybody else have an idea? Maybe there is a super simple way to test this, and we only need to add a quick test without changing any existing tests / test bases?
- π©πͺGermany Grevil
I reverted the test changes and created a new test class inside the jsonapi space. Unsure, whether to create a REST test class as well. But I don't think we should adjust the rest test base, how we did it in "MenuLinkContentResourceTestBase.php" originally, which I reverted here: https://git.drupalcode.org/project/drupal/-/merge_requests/339/diffs?com....
Unfortunately, now the old "MenuLinkContentTest" fails. Here, it expects a 403 status code, but it actually receives an unexpected 200 status code. This might show, that the current MR needs some refactoring? I am not sure. These tests are highly complex with many dependencies.