Problem/Motivation

Menus often contain nested information. Currently, there is no way to retrieve subtree links.

We need a way to include them in the main payload, or maybe provide a sub resource path that specifies a tree root

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#14 3170358-14.patch10.4 KBlarowlan
#14 3170358-14-interdiff.txt463 byteslarowlan
#13 3170358-13.patch10.41 KBlarowlan
#13 3170358-13-interdiff.txt1.63 KBlarowlan
#11 3170358-11.patch10.33 KBlarowlan
#11 3170358-interdiff-11.txt759 byteslarowlan
#8 3170358-8.patch9.86 KBlarowlan
#8 3170358-8-interdiff.txt7.09 KBlarowlan
#5 subtree-3170358-5.patch2.76 KBdeciphered
#4 subtree-3170358-4.patch3.23 KBdeciphered

Comments

mglaman created an issue. See original summary.

deciphered’s picture

deciphered
Primary language English
Location Ballarat, Victoria, Australia
commented
Priority: Normal » Major

Confirming this issue is present and needs resolving as high priority.

See example of issue here: https://dev-druxt.pantheonsite.io/jsonapi/menu_items/main

deciphered’s picture

deciphered
Primary language English
Location Ballarat, Victoria, Australia
commented

The source of this module is mostly the Rest Menu Items module.

One of the specific areas of code that relates to this issue is: https://git.drupalcode.org/project/rest_menu_items/-/blob/3.0.x/src/Plug...

By setting the min depth we can access the subtree items, so it's then just a case of recursively iterating over the data and building the required resources.

I will attempt to get a patch available for testing soon, but if I am unable to I am happy to provide context on what needs to be done.

deciphered’s picture

deciphered
Primary language English
Location Ballarat, Victoria, Australia
commented
Status: Active » Needs review
StatusFileSize
new subtree-3170358-4.patch3.23 KB

The attached patch deals with the subtree. It is under-tested, and likely needs more work, but it's a starting point.

deciphered’s picture

deciphered
Primary language English
Location Ballarat, Victoria, Australia
commented
StatusFileSize
new subtree-3170358-5.patch2.76 KB

Updated with a slightly saner approach, and have tested using the druxt-menu node module.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented 
+++ b/src/Resource/MenuItemsResource.php
@@ -89,27 +128,11 @@ final class MenuItemsResource extends ResourceBase {
+      if ($menu_link->subtree) {
+        $this->getMenuItems($menu_link->subtree, $items);
       }

so this puts them all in a root collection, and then the frontend needs to sort out the tree depth, correct?

deciphered’s picture

deciphered
Primary language English
Location Ballarat, Victoria, Australia
commented

Correct. All menu items are at the root, then it's up to the JSON:API query to retrieve the required data.

To retrieve all root items, you can simply request all items without a parent.
Then, as you walk through the tree you can pull in the next level as required by filtering items that have a specific parent.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
StatusFileSize
new 3170358-8-interdiff.txt7.09 KB
new 3170358-8.patch9.86 KB

Here's some expanded test-coverage for ensuring we see the menu-items we expect and that the parent field is populated.

This test also contains pieces we will be able to easily modify to demonstrate the bugs in #3170340: Apply cacheable metadata to the response object and #3171184: Filter our inaccessible menu links

e.g. the fixture contains an inaccessible item that we can remove for #3171184: Filter our inaccessible menu links test coverage.

It also contains a content entity that we can edit and re-save, to demonstrate the cache issue in #3170340: Apply cacheable metadata to the response object

I'll postpone those on this

Status: Needs review » Needs work

The last submitted patch, 8: 3170358-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Assigned: Unassigned » larowlan
Status: Needs work » Needs review
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
StatusFileSize
new 3170358-interdiff-11.txt759 bytes
new 3170358-11.patch10.33 KB

The test fail here comes from #3171184: Filter our inaccessible menu links - the 'Inaccessible' items are not string, but in fact instances of TranslatableMarkup which causes caching assert issues.

I'll re-categorize that as a bug

Status: Needs review » Needs work

The last submitted patch, 11: 3170358-11.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs work » Needs review
StatusFileSize
new 3170358-13-interdiff.txt1.63 KB
new 3170358-13.patch10.41 KB

Doh, forgot testbot runs in a subdirectory

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
StatusFileSize
new 3170358-14-interdiff.txt463 bytes
new 3170358-14.patch10.4 KB

CS fixes

deciphered’s picture

deciphered
Primary language English
Location Ballarat, Victoria, Australia
commented
Status: Needs review » Reviewed & tested by the community

I have tested this against the `druxt-menu` module and everything appears to be in working order on the JSON:API Menu Items side.

Marking as RTBC and will commit in the next few hours.

Thanks all, appreciate the effort.

  • larowlan committed 148391d on 1.0.x 
    Issue #3170358 by larowlan, Deciphered: Handle subtrees
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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

marcvangend’s picture

marcvangend
he/him
Primary language Dutch
Location Amsterdam
commented

While I'm sure that improvements were made in this issue, I'm missing some documentation. I think I understand dat subtree items are now added to the response, albeit not nested, but on the root level of the response data.

But I also read the following:

To retrieve all root items, you can simply request all items without a parent.
Then, as you walk through the tree you can pull in the next level as required by filtering items that have a specific parent.

Does this mean there I can add parameters to my request to limit the menu items to a maximum depth, and/or to retrieve the children of a specific parent item?

Thank you for your help.

deciphered’s picture

deciphered
Primary language English
Location Ballarat, Victoria, Australia
commented

Hi @marcvangend,

As this issue is closed it would be best to open a new issue with the question so that we can ensure it gets tracked and actioned correctly (e.g., you get an answer, and the answer gets turned into documentation).