- ๐ซ๐ฎFinland vermario
Here's a revised patch against the latest version of the module
- ๐ฎ๐ณIndia u_tiwari
Here is a better version which actually seems to work correctly and is compatible with latest version.
- last update
9 months ago 1 pass, 1 fail - Status changed to Needs review
8 months ago 11:40am 7 November 2023 - last update
8 months ago Composer require failure - ๐ฌ๐งUnited Kingdom andrewbelcher
This feature actually paves way to fix a bug, which is that there are a couple broken bits of cache when using menu item content:
- The JSON:API normalisation cache uses the language of the
ResourceObject
as part of it's cache key, resulting in normalisation getting cached incorrectly for language specific requests including translated menu item content. - The cacheability doesn't vary enough for the language content. Adding a cacheable dependency on the content (which will also resolve cache tags etc) fixes that.
As such, have updated the patch and updated the category/priority of this issue!
- The JSON:API normalisation cache uses the language of the
- last update
8 months ago 1 pass, 1 fail - ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
Looks good to me, but would be great to add tests for this because I don't want it to break again in the future, and as someone with the luxury of only working on single language sites, i18n can sometimes be an afterthought.
- Status changed to Needs work
7 months ago 12:00pm 11 December 2023 - ๐ฌ๐งUnited Kingdom andrewbelcher
Switched to needs work pending tests.
Have also added a new patch that fixes some indentation and an error when a menu item is not translatable.
In terms of items we need to test this with:
- Menu is/is not translatable
- Item is/is not a
MenuLinkContent
Test should validate:
langcode
only included on output forMenuLinkContent
on translatable menus- Cacheability is correct in all scenarios
- No errors for non-translatable menu links (the bug I've just fixed)
- ๐ซ๐ทFrance Jรฉrรดme Dehorter Lille
Hello,
With this addition of "langcode" would it be possible to add the possibility of filtering in the API with ?filter[langcode] for example.
- ๐ฆ๐บAustralia larowlan ๐ฆ๐บ๐.au GMT+10
I think that's worth adding for sure, as multi-lingual sites would want that right?
- ๐ซ๐ทFrance Jรฉrรดme Dehorter Lille
Hello larowlan,
That's right, we have several multilingual projects and this would be to display language-specific menu items.
- ๐บ๐ธUnited States mglaman WI, USA
I'm assuming patch from #22 is the latest work and https://git.drupalcode.org/project/jsonapi_menu_items/-/merge_requests/7 is outdated?
- ๐ฌ๐งUnited Kingdom andrewbelcher
Yes, i think so. We're running that patch in prod and it is working nicely.
- ๐บ๐ธUnited States mglaman WI, USA
โจ Add support for the Menu Item Extras module Needs review has logic for fetching the entity for menu_link_content, which is what this MR and patch are also adding. It's also using `getTranslationFromContext`. The only difference is that this adds
langcode
So I think the fix in will fix this, causing this issue to just add the langcode property.
- ๐บ๐ธUnited States mglaman WI, USA
mglaman โ changed the visibility of the branch 1.2.x to hidden.
- ๐บ๐ธUnited States mglaman WI, USA
I've opened https://git.drupalcode.org/project/jsonapi_menu_items/-/merge_requests/16 which is the essence of #22. The base field for the langcode field is allowed and the link's language is passed to the resource object.