Problem/Motivation

When we remove links on hook_simple_sitemap_links_alter it doesn't update pagination.

Steps

  1. We have 100 pages indexed on sitemap.xml
  2. We have configured only 10 links per page
  3. There are 10 sitemap.xml with 10 links each (total = 100, okay it works well)

Problem

We created a hook_simple_sitemap_links_alter, and we're removing 80 links (total = 20)
Example:

function PROJECT_simple_sitemap_links_alter(array &$links, $sitemap_variant) {
  foreach ($links as $key => $link) {
    if ($key >= 20 ) {
      unset($links[$key]);
      continue;
    }
  }
}

Espected result

Should be 2 sitemap.xml with 10 links each

Actual

There are 10 sitemap.xml

  1. 1st sitemap with 10 links
  2. 2st sitemap.xml with 10 links
  3. 3rd sitemap in blank
  4. 4th sitemap in blank
  5. 5th sitemap in blank
  6. 6th sitemap in blank
  7. 7th sitemap in blank
  8. 8th sitemap in blank
  9. 9th sitemap in blank
  10. 10th sitemap in blank

Question

There is a solution to "remove" links and update pagination?

Thanks in advanced

CommentFileSizeAuthor
#4 3153405.patch573 bytesrenatog
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

RenatoG created an issue. See original summary.

renatog’s picture

renatog
Location Campinas
CreditAttribution: renatog at CI&T for Johnson & Johnson commented
Issue summary: View changes
renatog’s picture

renatog
Location Campinas
CreditAttribution: renatog at CI&T for Johnson & Johnson commented
Issue summary: View changes
renatog’s picture

renatog
Location Campinas
CreditAttribution: renatog at CI&T for Johnson & Johnson commented
Version: 8.x-3.x-dev » 8.x-3.7
FileSize
3153405.patch573 bytes

This is a patch with a solution

If all modules alter $results will fix this problem automatically.

Follow the patch for 3.7

renatog’s picture

renatog
Location Campinas
CreditAttribution: renatog at CI&T for Johnson & Johnson commented
Status: Active » Needs review
pratik_kamble’s picture

pratik_kamble
Location Pune, India
CreditAttribution: pratik_kamble at QED42 commented
Assigned: Unassigned » pratik_kamble
pratik_kamble’s picture

pratik_kamble
Location Pune, India
CreditAttribution: pratik_kamble at QED42 commented

@RenatoG, I am not able to reproduce the issue. I have configured the simple_sitemap module to contain only 10 links per page for sitemap.xml.
I had a total of 280 content.
Added custom code as suggested by you.

function PROJECT_simple_sitemap_links_alter(array &$links, $sitemap_variant) {
  foreach ($links as $key => $link) {
    if ($key >= 20 ) {
      unset($links[$key]);
      continue;
    }
  }
}

Regenerated the links by clicking on Regenerate all sitemaps after hitting Save. It still shows 28 pagers with all the links.

When I debugged the code I found that key value was always below 10 so the if condition was never true.

@RenatoG I will need more input to review it further.

renatog’s picture

renatog
Location Campinas
CreditAttribution: renatog at CI&T for Johnson & Johnson commented

Hello @pratik_kamble

Have you changed "PROJECT" key by your project key that's right?

If yes, and you have 208 links, the link number 1 will be

  • $key = 0 => link 1 (index)
  • $key = 1 => link 2 (index)
  • $key = 2 => link 3 (index)
  • (....)
  • $key = 19 => link 20 (index)
  • $key = 20 => link 21 (remove)
  • $key = 21 => link 22 (remove)
  • $key = 22 => link 23 (remove)

So in the end you'll have only 20 links indexed in the sitemap, but you'll have a lot of os pages with other sitemal.xml (in blank)

My patch #4 implements result alter, and if we tested the same approach but with a different hook works well

function PROJECT_simple_sitemap_results_alter(&$results) {
  foreach ($results as $key => $link) {
    if ($key >= 20 ) {
      unset($results[$key]);
      continue;
    }
  }
}

Using this new hook in the final we'll receive a correct pagination

THanks a lot

gbyte’s picture

gbyte CreditAttribution: gbyte as a volunteer and at gbyte commented
Version: 8.x-3.7 » 8.x-3.x-dev
Assigned: pratik_kamble » Unassigned
Category: Feature request » Bug report
Status: Needs review » Active

There is a problem indeed, I don't like #4 however because the hook would fire for every single link but would operate on links it operated on in the previous iteration. I am going to look into it.

renatog’s picture

renatog
Location Campinas
CreditAttribution: renatog at CI&T for Johnson & Johnson commented

Yes, exactly. I checked it as well.

With #4 the hook is called many times

That's great! Any solution please let us know. Thank you so much for your help

  • gbyte.co committed 59514f2 on 8.x-3.x 
    Issue #3153405 by RenatoG, gbyte.co: Removing pages at...
gbyte’s picture

gbyte CreditAttribution: gbyte as a volunteer and at gbyte commented
Status: Active » Fixed

This took much longer than anticipated. The problem is that the results are stashed in between of generation instances and we have to stash the altered results as well as the original ones and make sure elements are processed once. At the same time we don't want each element to be processed by itself because of the diminished performance that would cause.

I think I found the cleanest way to fix the problem, please test the dev version and report back.

renatog’s picture

renatog
Location Campinas
CreditAttribution: renatog at CI&T for Johnson & Johnson commented

Okay, I'll test it

Thank you so much for your help

renatog’s picture

renatog
Location Campinas
CreditAttribution: renatog at CI&T for Johnson & Johnson commented
Status: Fixed » Closed (fixed)

Confirmed and really works well!!

Thank you so much and congrats on your job!

@gbyte.co, please can you generate a new release 8.x-3.8 with this fix?

It's important and uses a stable version is better

Have a good weekend

gbyte’s picture

gbyte CreditAttribution: gbyte as a volunteer and at gbyte commented

Confirmed and really works well!!
Thank you so much and congrats on your job!

Happy it works for you.

It's important and uses a stable version is better

If it was really that important, it would have been discovered ages ago. ;)

Sorry, I just released 3.7, 3.8 will have to wait for a couple of bugixes/features. But you can deploy the snapshot of the dev as-is, as right now it's 3.7 with this fix only. I believe you can do it like so:
composer require drupal/simple_sitemap:3.x-dev#59514f24295e575f0f4fc76eab9d4f03aa30f73d

Have a good weekend

Weekend just ended here... :/