Hi Kiam,

If you want the xmlsitemap_menu to work for everyone you cannot check the mlid like you do. The order in which the IDs are between menus cannot be inferred. If you want to avoid multiple identifier, then you would need to create one SELECT order with all the menus (which is easy to do, but that's not the way it is now.)

The following shows you the code that causes a problem:

  $mlid = 0;
  ...
  foreach(...) {
    ...
    $sql = "SELECT ... AND mlid > " . $mlid . "..."
    ...
    while($mlid = db_result($result)) {
       ...
    }
  }

After the while() ran, $mlid == null. This means on the next SELECT, the AND is wrong and the SQL does not work. Thus, unless your first menu includes all the links you wanted to include, it simply won't work right.

I propose to simply remove all $mlid tests in the SQL order.

I've attached a patch that makes the menu system work (hurray!)

And why did you remove "patch (to be reviewed)" from the Status?!

Thank you.
Alexis

CommentFileSizeAuthor
#2 xmlsitemap-6.x-1.x-menu-mlid-n-change.patch1.25 KBAlexisWilke
xmlsitemap-6.x-1.x-menu-mlid.patch840 bytesAlexisWilke

Comments

AlexisWilke’s picture

AlexisWilke commented

Okay... well... that got the links in the xmlsitemap table, but then you check the changed and changefreq and if they are 0, then you ignore the entries. Not too good if you ask me since that means the data is not sent out in the sitemap.xml

So, there is another patch that also sets changed to time() and changefreq to 1. I do not see another way to compute these numbers since menu links do not include a creation & modification date. Of course, you should load the corresponding node and check the data from the node...

AlexisWilke’s picture

AlexisWilke commented
StatusFileSize
new xmlsitemap-6.x-1.x-menu-mlid-n-change.patch1.25 KB

And eventually, the patch! 8-)

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Active » Needs work

I didn't change anything in the statuses list; the list of values has been changed from Drupal.org webmasters.

The SQL queries filter out the database rows which contain invalid values (it's rather difficult a node has been changed on Unix epoch time); the value of zero for the changed field can be used from the code to disable an entry.

The patch must be modified to only change the code object of this report.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Needs review

I first misunderstood what you meant. I am changing the status of the report.

AlexisWilke’s picture

AlexisWilke commented

I guess that the xmlsitemap module could add support to know when a link is created (and of course for links that already exist, put a date in the past) and what has changed. But I don't think that's very necessary.

I had not noticed the Status change in my projects, but it could be that it changed just a couple days ago... 8-}

And I can tell you that I now get the xmlmenu links in my sitemaps. So that's working great for me!

Thank you.
Alexis Wilke

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Postponed (maintainer needs more info)

Do you mean you get the links with the patch applied, or without?

AlexisWilke’s picture

AlexisWilke commented

I get the links with my patch applied.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Title: There is a patch to fix the xmlsitemap_menu » Wrong SQL query for retrieving the menu item links
Status: Postponed (maintainer needs more info) » Active
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Assigned: AlexisWilke » Unassigned
Status: Active » Fixed

This has been fixed in CVS; differently from the proposed patch the code has been changed in

  $changefreq = variable_get('xmlsitemap_menu_menus_changefreq', 3600);
  $row->changed = XMLSITEMAP_TIME - $changefreq;
  $row->changefreq = $changefreq;

where XMLSITEMAP_TIME is a constant initialized by xmlsitemap_helper.

Thanks for your report.

Status: Fixed » Closed (fixed)

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