Previous/next for same content type

Hi,
this is my first patch for a drupal module, and very proud of it :-)

I was also annoyed by the fact that the only sort criteria is nid, so I've made the following patch to have a new option: the search criteria. For now, you can choose between Node ID (default option), Post date, Updated date and Title.

If this value is changed, the index will be re-built on next cron.

In a near future, I'm also thinking about doing this per content-type, and also to have an option to be able to choose if the next/previous item has to be of the same type or not. Good idea?

Finally, I've already made the changes to my previous patch…

This one allows to change ordering criteria by content type, and to choose if the prev/next node has to be of the same content type or not.

I'm thinking about selecting the search criteria with token, if token module is installed, but I need to study how to do that :-)

You need to run the coder module on the patch to fix the code style. For example:

+  } else $cond = " AND type = '" . $node_type . "'";

There is a lot of code that is suboptimal, unclear and can introduce errors. For example:

+function _prev_next_node_types_sql($node_type) {
+
+  $same_type = variable_get(PREV_NEXT_NODE_TYPE . $node_type . '_same_type', 0);

It is called with (0) is some places.

I wonder why this was changed, without changing the name of PREV_NEXT_NODE_TYPE? This is very confusing.

-      '#title'         => node_get_types('name', $type),
+      '#title'         => t('Include'),

Also, does this now do separate indexing methods for each node type? If so, then how come the database schema remains the same? Is the performance still good with all this filtering?

Do we need a separate submit handler? Can't we do this the normal way without overcomplicating things?

Finally, has this been tested?

I also seek input from other users of this module.

Hi, thanks for the advices.

I've made changes to the patch.

I had no idea of this 'coder' module (this is my first patch) but I downloaded it, ran it on the modules and correct 70+ errors. The only ones that are still there are false positive for SQL queries.

There was no need to change the database schema, because I don't need more fields.
I think the performance are still good, I've made some tests.
For the _prev_next_node_types_sql(0), I've changed the function declaration to set a default value, so this is now _prev_next_node_types_sql(), as before.
And for the submit handler, perhaps there's another way to do that, but when I created the submit callback, it didn't seem to execute it unless I declare the handler.

Finally, I implemented a hook_block, so I don't need the manually edit the themes files to make it appear (not sure if this is best way, but… tell me!)

Here's an updated version of my patch.
Some bug fixes (db_query_range not working as expected), and changes to hook_nodeapi because now, we can choose a criteria that could change on update (a nid is always the same, but not the title or update date) so we have to rebuild some parts of the index when updating a node.
And now, a node can be the next (or previous) of more than one other node, depending of the criteria of each content type, so I had to take that in mind when creating a new entry in the database.

I've tested it on my own website, and it seems to be working as expected and still very fast to reindex.
And the new block works as well, but I need to think about it, because as now, if I put the block in the content area, it appears after the comments (I'd prefer before) and when the nodes are displayed in views, it doesn't appear at all! (any idea?)

This is my final version of the patch.
It includes all the previous modifications (see above), and the final version of the block hook, so you don't have to put code in your theme to use it. Just add blocks!
You can set the number of available blocks, customize the link text, and if the TOKEN module is installed, use it to use any fields you want!

I use it on my own websites, and already, I can't live without it :-)
Can someone check this patch, please?

Thanks very much for the patch -- makes this module actually useful. Things are looking pretty good but I do see a problem.

I'm indexing nodes of type "Story" by date. When I first indexed the nodes the prev/next table was populated correctly. When I add a new Story node the new node gets the correct table entry but the previous node's entry does not get updated to point at the new "next" node.

to be clear:

Initially my table looks like this:

nid          prev_nid          next_nid            changed
---------------------------------------------------
2            1                    0                     xxxxxxxxx

Then I add a new node and it looks like this

nid          prev_nid          next_nid            changed
---------------------------------------------------
2            1                    0                     xxxxxxxxx
3            2                    0                     xxxxxxxxx

Note that "next_nid" for node 2 did not get updated.

Even running cron again does not fix this.

Here's a patch to fix the issue I reported above. It's the first patch I've posted on drupal.org so I'm not sure if I've done things properly. It's not a CVS patch. Regardless, I think it should work fine. It is a modified version of jdanthinne's patch so it should be used INSTEAD of his/her patch -- not on top of it.

I'd love others to review this. The logic in line 370 - 386 of jdanthinne's patch made no sense to me (when would you ever want to set the next_nid or prev_nid equal to the current nid?). Maybe I'm missing something, but I couldn't figure out what he/she was trying to do there so I reverted that section to the module's original code. Things are working for my use-case but it's entirely likely that I've missed something.

EDIT: Ha -- I just looked at the patch I attached and it turns out I accidentally uploaded the wrong one so the comments I made about it above don't make sense. Doesn't matter now that jdanthinne has updated his patch.

Hi, (by the way, I'm a man, so you can say 'he' and 'his' :-) )

thanks for the review. And I've found my little mistake (those strange lines you were talking about 370-386). I was inverting next and prev so the tables were not right and deleted the strange test (I think I've made a wrong copy-paste). I corrected it and also added quotes in the SQL queries (that was needed when comparing strings). But those lines are absolutely necessary and I can't revert to the original code. It would be ok if we work with nid or dates, because those data never change, but if we work with the title, a node can be inserted in the middle of the index, so I have to check the index to correctly do that.

So there is the updated patch. I think it's definitively right this time. Can you check?

Thanks for the update and explanation. I figured I was missing something :).

I'll test it more thoroughly some time next week but a glance at the patch looks like it's sorting in descending value, is that true? So if you sort by nid the "next" node has a lower nid? This would seem to be the opposite of the original functionality. Can you confirm? I suppose descending order would be preferable in some situations, ascending in others. Of course folks can always arbitrarily swap them wherever they might use the api but still...might be nice to have as an option...

No, I haven't changed the original sorting order of the module.But I have to admit that the first time I used it myself, I also thought that it was in the wrong order, but in fact, it's just a matter of how you understand 'prev' and 'next'… and after a little time, you get used to it.
I won't do any more changes before the maintainer of the module tell us what he thinks about all those changes and commit them to a new version (or not!).

how do you guys apply this patch? I have no experience in coding..

How to apply patches: http://drupal.org/patch/apply

It will be great if the current patch will be added to the official module...

Here's an update of my patch.
Corrected a small bug for special cases.
Still hoping to be ported :-)

I'm waiting for the patch to be ported for a long time now…
Do you think I have to start a new project?

Committed.

Thanks for the patch.

Please wait for 12 hours then download the 6.x-1.x-dev tarball and test it.

Thanks! Tested and working fine for me.