Problem/Motivation

The menu items lack the langcode, which is required in order to tell if the given item has a translation in the requested language or not.

Proposed resolution

Add the langcode where possible.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

blazey created an issue. See original summary.

blazey’s picture

Assigned: blazey » Unassigned
Status: Active » Needs review
FileSize
1.14 KB
Deciphered’s picture

Status: Needs review » Needs work

Patch needs a re-roll, and tests are currently failing, likely due to the dependency on #2997790.

Suresh Prabhu Parkala’s picture

Status: Needs work » Needs review
FileSize
1.28 KB

Re-rolled patch. Please review.

Status: Needs review » Needs work

The last submitted patch, 4: 3192576-4.patch, failed testing. View results

Pooja Ganjage’s picture

FileSize
1.42 KB

Hi,

Creating a patch for solving issue that got raised in #4 comment patch.

Please review the patch.

Thanks.

Pooja Ganjage’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 6: 3192576-6.patch, failed testing. View results

Pooja Ganjage’s picture

FileSize
2.38 KB
larowlan’s picture

Category: Task » Feature request
cola’s picture

Anyone who has a new patch for it?

Error: Call to protected method Drupal\menu_link_content\Plugin\Menu\MenuLinkContent::getEntity() from scope Drupal\jsonapi_menu_items\Resource\MenuItemsResource in Drupal\jsonapi_menu_items\Resource\MenuItemsResource->getMenuItems() (Zeile 205 in /modules/contrib/jsonapi_menu_items/src/Resource/MenuItemsResource.php)

Deciphered’s picture

Issue tags: +Needs reroll

A re-rolled patch of PR would be greatly appreciated if anyone has the capacity, otherwise I will aim to re-roll and test this in the coming weeks.

cola’s picture

Would be good if we can remove the dependency on Set MenuLinkContent getEntity to public visibility

Deciphered’s picture

Version: 1.1.0 » 1.2.x-dev

Deciphered’s picture

I've created a feature branch and merge request with the latest patch against 1.2.x. The patch can be found @ https://git.drupalcode.org/project/jsonapi_menu_items/-/merge_requests/7...

vermario’s picture

FileSize
794 bytes

Here's a revised patch against the latest version of the module

u_tiwari’s picture

FileSize
2.03 KB

Here is a better version which actually seems to work correctly and is compatible with latest version.

jb044’s picture

FileSize
2.03 KB

Rerolled against latest dev

andrewbelcher’s picture

Title: Add langcode » Add langcode & fix content caching
Category: Feature request » Bug report
Priority: Normal » Major
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
2.44 KB

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:

  1. 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.
  2. 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!

larowlan’s picture

Issue tags: +Needs tests

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.

andrewbelcher’s picture

Status: Needs review » Needs work
FileSize
2.44 KB

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 for MenuLinkContent on translatable menus
  • Cacheability is correct in all scenarios
  • No errors for non-translatable menu links (the bug I've just fixed)
Jérôme Dehorter’s picture

Hello,

With this addition of "langcode" would it be possible to add the possibility of filtering in the API with ?filter[langcode] for example.

larowlan’s picture

I think that's worth adding for sure, as multi-lingual sites would want that right?

Jérôme Dehorter’s picture

Hello larowlan,

That's right, we have several multilingual projects and this would be to display language-specific menu items.

mglaman’s picture

I'm assuming patch from #22 is the latest work and https://git.drupalcode.org/project/jsonapi_menu_items/-/merge_requests/7 is outdated?

andrewbelcher’s picture

Yes, i think so. We're running that patch in prod and it is working nicely.

mglaman’s picture

#3276561: Add support for the Menu Item Extras module 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.

mglaman changed the visibility of the branch 1.2.x to hidden.

mglaman’s picture

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.