Add next/previous navigation

[AWearring]

Added next/previous navigation based upon localgov_guides

[AWearring]

This is working for me locally, but is failing tests - I'm at a loss as to why it is failing https://github.com/localgovdrupal/localgov_blogs/actions/runs/8401705613/job/23010407547?pr=31 Behat\Mink\Exception\ResponseTextException: The text "Next" was not found anywhere in the text of the current page.
I'm my testing locally the nav block is added to the page and has all the correct next/prev links

[stephen-cox]

@andybroomfield to take a look.

Feelings at merge Tuesday is that this should be optional so sites can opt in. There isn't explicit ordering for blogs so the assumption is this will be ordered by date, but this isn't guaranteed.

[AWearring]

@andybroomfield to take a look.

Feelings at merge Tuesday is that this should be optional so sites can opt in. There isn't explicit ordering for blogs so the assumption is this will be ordered by date, but this isn't guaranteed.

They are initially ordered by creation date - however this can be manipulated on the channel node:

[andybroomfield]

I've managed to do a quick test and get it working.

However I'm actully not sure this is the right approach here, trying to use the same method as guides which is a hierarchical content type with explicit ordering, and blog posts that are primarlly in date order, with some being pulled out to be featured. This is making the feature a little complicated to implement as I see that we are tracking new blog posts that are being added just so they can be re-ordered. However sometimes blog posts can be added with different published dates so this would result with them being out of order so having to be reorded each time (On our news pages, often our comms team will have several draft stories being worked on, and they will adjust the date before publishing).

Instead, I belive the better approach is to lean into this being date based content, and make the next / previous links behave as a way of moving to the next / previous blog post based on the date, using the same method as wordpress. I feel this would also be closer to how blog authors would expect it and would like to hear from them on their user needs.

• As a blog reader
• I want to see a link to the previous or next published post
• So I can continue reading posts in date order

If there is a need to re-order a post, the current established pattern in news where we change the post date.

Maybe its worth looking at Nextpre module module?
Or the alternative Flippy
And theres some code here to do a direct implementation.

Things to consider

• Do we need to track blog posts so we can establish order or can we use an entity query?
  So this PR creates a new field on the blog channel where new posts are added, but we need it to always be in date order, then its possible to use the field delta. Alternativly we could use an entity query to find the next and previous post by date, though this might be a little computationally expensive?
• Should we factor in featured blogs posts in the next / previous or ignore them and treat them like date order.

[finnlewis]

Discussing with @ekes and @andybroomfield at Merge Tuesday:

• we're not keen on storing all the blog posts in a field as this could get huge and becomes a performance and usability issue with hundreds or thousands of blog posts.
• Looking at the Nextpre module, that selects the next node based on node id. https://git.drupalcode.org/project/nextpre/-/blob/9.0.x/src/Plugin/Block/NextPreviousBlock.php?ref_type=heads#L234 - this is also not ideal.
• So maybe as @andybroomfield suggests, an entity query to select the next and previous by date / time - then sort by ID for any edge case where multiple nodes are created in the same second. This would be similar to the Nexpre module but better, for our needs.

@AWearring what do you think? Is this something you want to re-work or do you want someone else to dive in a give it a go?

[AWearring]

@finnlewis I'll have a crack at it (time allowing), if I get stuck I will happily pass on to someone else to have a go at it

[andybroomfield]

This is ready for review again @AWearring and @ekes.
I've added a test to demonstrate multiple channels work correctly (next / prev links in the correct channel).

I've also tried to account for blog posts being on the same day. The issue here is that there is insufficent granularity for the localgov_blog_date field which is just a date field. For the moment I've made changes to the way next / prev is queried so it can secondarly use the created time as a sort. The issue here is that can be out of step with the localgov_post_date so we can't just add that as a condition. Instead I've added it so that it includes the current node in the results (account for same day) and then searches the results for the next nid (sometimes it can get thrown by having one earlier). It's not the most efficent and I wonder how this will scale to many blog posts, though it seems to work for now.

If we would rarther skip this and try to change localgov_blog_date to a datetime field, then we just need to drop commit 5492910.

[andybroomfield]

Bumping as this could do with a review and a release to also cover #37

[AWearring]

I've retested this today and it all seems OK to me - not sure what the failing tests are

[millnut]

Hi @AWearring could you merge the latest changes from 1.x into your branch to see if this resolves the workflow failures

[AWearring]

Hi @AWearring could you merge the latest changes from 1.x into your branch to see if this resolves the workflow failures

@millnut Looks like it was already merged

[millnut]

Thanks @AWearring just added some comments around the linting failures

[AWearring]

@millnut All tests now pass

[AWearring]

@millnut @andybroomfield @ekes @finnlewis
I'm keen to get this released as I'm expecting to get a request in the next couple of months for adding a "Blog" to one of the sites I look after and not having this navigation would be a blocker.

[andybroomfield]

This LGTM. What where the blockers?

Changes actioned dismissing pending request

[millnut]

I've dismissed my changes request, they have been actioned not sure why github thinks they have not

[millnut]

@AWearring I'm just kicking off another workflow run as a couple of other projects had pagination test failures recently, hopefully this one will be fine

[AWearring]

This LGTM. What where the blockers?

@andybroomfield there were two change requests that were still open
@millnut has dismissed one and @ekes found that when there were 2 posts on the same day the next/prev navigation didn't show (#31 (review)), this was subsequently fixed but Github didn't close the change request

[millnut]

@AWearring I'm just kicking off another workflow run as a couple of other projects had pagination test failures recently, hopefully this one will be fine

Tests all good for the re-run 👍