get_adjacent_post() when using in term doesn't account for child terms

[WazzaJB]

When using any of the next or previous post functions with $in_term set to true only top level terms are accounted for. This means if two posts are in the same child term, and a third is not but is between the posts date range then the third will take precedence.

Although I am sure it will be up for discussion, I believe child terms should take priority.

Take the following posts for example:

Post 1
Date: 01/01/2015
Categories: Parent -> Child

Post 2
Date: 02/01/2015
Categories: Parent

Post 3
Date: 01/01/2015
Categories: Parent -> Child

Expected Functionality (In my opinion):
Post 1 get_adjacent_post(true) should return Post 3

Actual Functionality:
Post 1 get_adjacent_post(true) returns Post 2, which does not match the child term.

My attached patch simply removes parent terms from the query, where a child of it is set. This means that only the deepest selected terms will be used, where children and parent terms are set.

I have also attached an export of my posts to highlight the issue more clearly, along with a patch to Twenty Fifteen purely to help represent the issue.

This really is just an initial draft suggestion for the code, backwards compatibility is obviously extremely important however more discussion about this is needed. There is another issue related to this function involving dates .

Sorry if this should be tagged as an enhancement rather than a bug.

[WazzaJB]

Patch for function

[WazzaJB]

Posts import to reproduce issue

[WazzaJB]

Apologies I made an error in my original post, post 3 date is supposed to be 03/01/2015.

Placing the following in single.php within the loop will show the previous and next links which use the aforementioned function.

<?php
// Previous/next post navigation.
echo the_post_navigation( array(
    'in_same_term'       => true,
    'taxonomy'           => 'category',
) );

[boonebgorges]

Hi @WazzaJB - Thanks for the ticket, and welcome to Trac!

I am a bit on the fence about your proposal, though I think I lean toward agreeing with you. The $in_same_term parameter of get_adjacent_post() is (obviously) somewhat ambiguous. Given two terms parent and its child term child, it's not 100% clear whether a post belonging only to parent is "in the same term as" a post belonging only to child. In most places in WordPress, the answer is "no", so I suppose it makes sense to follow suit here, as you've suggested. We will need to spell out the implications with some automated tests, though, and announce the change in behavior.

It looks like your patch may have been generated against an old version of WordPress. See [34088]. Can you refresh for current trunk?

[WazzaJB]

Patch with unit test

[WazzaJB]

Hey @boonebgorges, great to be here!

Thanks for the pointers, and also thanks for your advice over Slack. I've updated the changes against trunk and also added a unit test which helps illustrate my point.

Although I know this change will not effect any of my current sites, I do believe this could potentially create problems on sites where they are happy with the way it currently fetches adjacent posts within terms.

I feel we should be concerned about backwards compatibility for a change like this, however don't know the standard protocol in terms of whether we add an additional argument to the function or whether we provide a way for developers to 'turn off' this new functionality by way of a filter?

Happy to modify my patch either way.

Thanks again!

PS: Really sorry if i've got the workflow keywords wrong, getting to grips with things :)

[boonebgorges]

Hi @WazzaJB - Thanks for the patch! Format etc looks good.

Looking this over more closely, I'm not totally clear what we're trying to accomplish. Your unit test creates four posts:

1. In 'parent' and 'child'
2. In 'parent'
3. In 'parent' and 'child'
4. In 'child'

Given this, it seems *correct* to me that each post is adjacent to its immediate neighbors:

• 1 -> 2, because they're both in 'parent'
• 2 -> 3, because they're both in 'parent'
• 3 -> 4, because they're both in 'child'

Your patch seems to say that only leaf nodes should be counted by in_same_term. This strikes me as incorrect. Based on your original ticket, I thought the problem was with posts like this:

1. In 'child'
2. In 'parent'
3. In 'child'
4. In 'parent'

But, when I rewrite your test to test *this*, the tests pass. Which suggests that there may be no bug after all :)

Can you say more about what you are trying to accomplish with the ticket?

[WazzaJB]

Hi @boonebgorges,

Thanks for the response.

Given your first example I'm actually suggesting that post 1 should not go through to post 2, because their deepest terms do not match. As only the top level terms match I do not deem them 'in the same term'.

I believe only leaf nodes should be counted when the leaf nodes parent also exists within the assigned terms.

Take the following real-world example:

We have two categories, 'Tutorials' and child categories of 'NodeJS' and 'WordPress'.

We have a Post 1 that is in the category 'Tutorials' and 'NodeJS'.

Post 2 is in the category 'Tutorials', but is in a child category of 'WordPress'.

Post 3 is in the category 'Tutorials' and 'NodeJS'.

If I were to use the existing adjacent posts function on post 2, then the previous and next posts would be posts 1 & 3.

So using this real world example, if I wanted to go onto the next WordPress 'Tutorial' I would actually arrive at a 'NodeJS' tutorial.

I hope this has explains things a little better, thanks for bearing with me!

[boonebgorges]

Thanks for the clarification, @WazzaJB.

My reading of the parameter in_same_term is that it should return the next/prev post that *shares any term* with the current post. Your suggested change is that only leaf terms should be matched. This is a pretty big change in functionality, and I think it's not desirable for many use cases. Changing the behavior of in_same_term to mean this is, I think, not doable. (There are also many edge cases to consider - term hierarchies with branches of differing depths, etc.)

That said, I think it's worth considering how to make it easier to do what you want to do. Moving to WP_Query for get_adjacent_post() would give you more robust filters; see #26937. Barring that, we might consider a new filter that would allow this behavior to be modified in a non-insane way (ie, a technique that does not require filtering the entire SQL string).

[WazzaJB]

I was always on the fence about whether this was a bug or should be tagged as enhancement, I think it entirely rests on the use case for the taxonomy. My example above is a really sound reason of why you'd expect this behaviour, however some users may want/expect the current approach which is much looser.

I agree, this is quite a massive change and as you state it could have dire consequences for other sites. I'll have a deeper look at #26937 and see if there is anything I can potentially add into the mix however I would say that moving to WP_Query is probably not viable, performance and backwards compatibility being the primary reasons.

I'll have a look at creating a patch with a filter approach so that more can be manipulated with this.