Add an option for menu depth

Created on 11 February 2019, about 6 years ago
Updated 17 June 2022, almost 3 years ago

Problem/Motivation

Limit menus depth diplayed/loaded in sitemap.

Proposed resolution

Add an option in the sitemap settings in order to manage menus depth just like terms depth is manage.

✨ Feature request
Status

Needs review

Version

2.0

Component

Code

Created by

πŸ‡«πŸ‡·France ikouoh

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.0.7 + Environment: PHP 8.1 & MariaDB 10.3.22
    last update almost 2 years ago
    Patch Failed to Apply
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MariaDB 10.3.22
    last update almost 2 years ago
    Patch Failed to Apply
  • First commit to issue fork.
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MariaDB 10.3.22
    last update over 1 year ago
    22 pass
  • πŸ‡¬πŸ‡§United Kingdom welly

    Created a new patch/fork because the patch above doesn't seem to apply on the latest version.

  • πŸ‡¬πŸ‡§United Kingdom welly

    Interdiff for patch #10 and patch #12

  • Status changed to RTBC about 1 year ago
  • πŸ‡ΊπŸ‡ΈUnited States firewaller

    #12 works for me

  • Status changed to Needs work about 1 year ago
  • πŸ‡¨πŸ‡¦Canada mparker17 UTC-4

    This looks great, but I don't want to merge new functionality without tests.

  • First commit to issue fork.
  • πŸ‡¬πŸ‡§United Kingdom nexusnovaz

    nexusnovaz β†’ changed the visibility of the branch 3032084-add-an-option to hidden.

  • πŸ‡¬πŸ‡§United Kingdom nexusnovaz

    I was trying to write some tests unfortunately I can't seem to get PhpStorm to work with it. I've rolled #12 into the MR !39. I'm going to keep trying to see if i can get mine to work, but if anyone gets it first, feel free.

  • πŸ‡―πŸ‡΅Japan tom konda Kanagawa, Japan

    tom konda β†’ made their first commit to this issue’s fork.

  • Pipeline finished with Failed
    5 days ago
    Total: 346s
    #461624
  • Pipeline finished with Failed
    5 days ago
    Total: 390s
    #461642
  • Pipeline finished with Canceled
    5 days ago
    Total: 333s
    #461657
  • Pipeline finished with Failed
    5 days ago
    Total: 314s
    #461663
  • πŸ‡―πŸ‡΅Japan tom konda Kanagawa, Japan

    I rebased feature branch and added a test for menu depth option.
    But, need to fix SitemapUpdateSettingsTest::testUpdateHookN.

  • πŸ‡¨πŸ‡¦Canada mparker17 UTC-4

    Putting on my maintainer hat, thank you, @nexusnovaz and @tom konda!

    SitemapMenuTest::testMenuDepth() looks good to me: thank you very much!

    Putting on my developer hat, I think that SitemapUpdateSettingsTest::testUpdateHookN() is failing because the new menu_depth configuration option needs to be added to the configuration schema in config/schema/sitemap.schema.yml β€” at time-of-writing, there is a schema for Menu plugins between lines 67 and 74 of that file.

    I wanted to provide an example of what that configuration might look like, but I could not find an example, because this merge request uses the string unlimited to indicate unlimited menu depth...

    1. I searched Drupal core and some contrib modules ( Admin Toolbar β†’ , Better Exposed Filters β†’ , Client-side Hierarchical Select β†’ ) for the search term depth in all *.schema.yml files, to see there were any other examples of "depth" configuration that could either be a string or an integer, but I could not find any examples.
    2. It seems like most "menu depth" configuration uses type: integer, where the integer 9 indicates the maximum possible depth.
    3. This module's sitemap_plugin_settings.vocabulary schema defines both term_depth and rss_depth as type: integer, but use NULL to indicate unlimited depth (i.e.: by saying both type: integer and nullable: true in the config schema).
    4. Other examples of "string or integer" configuration that I could think of (e.g.: system.performance:cache.page.max_age, automated_cron.settings:interval, dblog.settings:row_limit, system.file:temporary_maximum_age, etc.) use a special integer (0, -1) to indicate the string "Unlimited", "Never", "All", etc. That being said, I might have missed something!

    In summary, it might be simpler to write a config schema if you changed the merge request to use an integer like -1, or the value NULL to indicate an unlimited depth, instead of what the merge request does now (i.e.: use the string unlimited to indicate an unlimited depth).

    For what it's worth, I've found the Configuration Inspector module β†’ to be helpful when writing and debugging configuration schemas.

Production build 0.71.5 2024