wp2static icon indicating copy to clipboard operation
wp2static copied to clipboard

DetectCategoryURLs & DetectCategoryPaginationURLs returns all taxonomies

Open Flynsarmy opened this issue 5 years ago • 3 comments

A few issues here:

  • This class is named incorrectly for what it does. Suggest DetectTaxonomyPaginationURLs or DetectCustomTaxonomyPaginationURLs depending on the below
  • Do we want it to cover ALL taxonomies or just custom ones? Are there detectors for any other taxonomies returning duplicate URLs?

Flynsarmy avatar Sep 01 '20 00:09 Flynsarmy

It would be nice to break it down to:

  • categories
  • tags
  • custom taxonomies

and paginations for all of those, where possible.

We de-duplicate at the end, so I'm less concerned about detecting too much vs not enough, if there is some double-up.

Happy for you to make the call regarding this area

leonstafford avatar Sep 01 '20 01:09 leonstafford

Hmm. If we split this up into DetectCategoryURLs, DetectTagURLs and DetectCustomTaxonomyURLs we get a lot of duplicate code and violate DRY. In fact the code for all 3 classes (and their respective tests) would be identical except for the argument passed to get_taxonomies. This would also result in 6 detection classes to maintain instead of 2 when we include the pagination version of each.

Sidenote: In both cases we're going to need a filter for which taxonomies to crawl. We don't currently have this.

The logical breakup of built-in and custom taxonomies into separate classes makes sense, as does joining them all into a single detection class as all behavior between custom and built-in is the same so it's just a personal preference thing. We'll need to use this same philosophy for Posts and custom post types. I'm happy to go either way on this though. Let me know and I'll make it happen.

@john-shaffer do you have any thoughts to add?

Flynsarmy avatar Sep 01 '20 03:09 Flynsarmy

I haven't looked at the detection code much, but if it's identical apart from a change of argument, then it makes sense to keep it all in one class.

john-shaffer avatar Sep 01 '20 14:09 john-shaffer