Automattic / msm-sitemap

Comprehensive sitemaps for your WordPress VIP site. Joint collaboration between Metro.co.uk, WordPress VIP, Alley Interactive, Maker Media, 10up, and others.
74 stars 37 forks source link

Track and rebuild sitemaps for deleted posts #135

Open kraftbj opened 6 years ago

kraftbj commented 6 years ago

Fixes #112

When a post is deleted, the post ID and date is added to an option which is used to augment the existing process of seeking sitemaps to regenerate based on a post's last modified time.

In this instance, we are opting for keeping the post_modified process since it requires less processing as posts are moved—no added filters or functions during that action.

Additionally, this PR adds a specified start time in a couple of places to remove the possibility that a post is modified in the short time between when processing started and when processing ended on rebuilding for modified posts.

kraftbj commented 6 years ago

Note: This needs unit tests, but I'm afk this week so I wanted to give the team a chance to review this without it being blocked until I wrote tests.

mattoperry commented 6 years ago

Just thinking out loud about some edge cases here @kraftbj ... what happens if there are a very large number of posts deleted? Like perhaps a site is trimming its archive or something and deletes 10s of 1000s of posts? I'm asking in the sense of "would anything bad happen" but also in the sense of would the plugin work under those conditions and regenerate the right sitemaps? Otherwise this looks like a good approach to me.

kraftbj commented 6 years ago

@mattoperry That's a good point. I reworked it a little in https://github.com/Automattic/msm-sitemap/pull/135/commits/c3fccc1db386126c74a7c92399e8edaf4d4b9b45 . Previously if we had 10,000 posts deleted covering 30 days, we would have a 10k item array to inform us of 30 days we needed to regenerate. The revised approach only keeps one array item per date, keeping the option down to 30 under this scenario.

The solution we have right now is 5.5+. What is VIP's policy on doing something like a Composer dependency or just including a library? https://github.com/ramsey/array_column provides a backfill and is what WP-CLI uses to polyfill array_columns.

kraftbj commented 6 years ago

Opted to not polyfill and have a suboptimal experience pre-5.5. MSM should be declared "5.5+ recommended", but will still function on 5.2+