Goto Last Page defaults to the last page of comments (instead of the first one) for configurable node types.

Summary

Suppose you have a node with a lot of comments, threaded and sorted older --> newer.
Default behavior will show the first page of comments (which could be 1 or 2 years old...) and user needs to click again to show the last page; node is then shown at least twice (the first is useless).
Goto Last Page changes this default behavior by showing the last page of comments (instead of the first one) for configurable node types.
It also allows adding a custom fragment (like #comments) to pager links, so the user will go straight to comments section while paging through comments.

Description

If "page" query parameter is not set, Drupal defaults to the first page of a pager; but this is not always the best choice.

Example
-------
Node: http://example.org/mynode
Node comments: 500
Comments / page: 25
Comments sorting: Threaded list - Expanded / Date - Oldest first

When a visitor opens http://example.org/mynode , Drupal will return the node together with the first page of comments (1 to 25).
But these comments are the oldest, so the user must click again on the comments pager to see the more recent ones... and this could require more than one click.

Goto Last Page changes this behavior (on configurable node types) by returning the last page by default.

What changes?

Let's see how rendered pager links will be changed by Goto Last Page module when an user request the same URI as above.

Without Goto Last Page module

URL: http://example.org/mynode
Returned comments page: first
First page: http://example.org/mynode
Prev. page: http://example.org/mynode
Page 2: http://example.org/mynode?page=1
Page 3: http://example.org/mynode?page=2
Next page: http://example.org/mynode?page=1
Last page: http://example.org/mynode?page=19

With Goto Last Page module

URL: http://example.org/mynode
Returned comments page: last
First page: http://example.org/mynode?page=0
Prev. page: http://example.org/mynode?page=18
Page 2: http://example.org/mynode?page=1
Page 3: http://example.org/mynode?page=2
Next page: http://example.org/mynode
Last page: http://example.org/mynode

As you can see now the default page is the last; to see the first one, page=0 query parameter must be added to the URI.

Installation

Install as usual, see https://drupal.org/node/895232 for further information.

Configuration

By default the module is active on all node types.
Open configuration page (admin/config/system/gotolastpage) to change it.

Links

Drupal version: 7.x
Project page: https://drupal.org/sandbox/nicorac/2205137

Git

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/nicorac/2205137.git gotolastpage

Reviews of other projects

[D7] More dates
[D7] Drupal Front End Developer
[D7] Varnish All
[D7] Disable JS
[D7] Converse.js Integration
[D7] Real Time Poll

CommentFileSizeAuthor
#20 patch_0001.patch755 bytesnicorac

Comments

nicorac’s picture

nicorac commented
Title: [D7] gotolastpage » [D7] Goto Last Page
nicorac’s picture

nicorac commented
Issue summary: View changes
PA robot’s picture

PA robot commented

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

contactvishaljain’s picture

contactvishaljain commented
Status: Needs review » Needs work

There are few problem with the project:-

  • PAReview is reporting issue on the code. Please see: http://pareview.sh/pareview/httpgitdrupalorgsandboxnicorac2205137git
  • Use hook_theme or theme(function) instead of direct call to any theme function.
  • Please remove master branch and create new branch (like 7.x-1.x) then delete master branch also provide new link for git in issue summary.
nicorac’s picture

nicorac commented
Status: Needs work » Needs review
nicorac’s picture

nicorac commented

Fixed PAReview issues (see #4).

Anonymous’s picture

Anonymous (not verified) commented
Category: Task » Bug report
Status: Needs review » Needs work

I made some small changes to the function below to make it work on lower versions than php 5.5 (i was using 5.4)

function gotolastpage_node_view($node, $view_mode, $langcode) {

  // Test if we're enabled.
  if (drupal_static('gotolastpage_active', FALSE)) {

    // Test if we need to add a custom fragment.
    $fragment = variable_get('gotolastpage_fragment', GOTOLASTPAGE_FRAGMENT);
    if (!empty($fragment)) {

      // Add our custom post_render function to all pagers.
      $all_children = _gotolastpage_find_all_children_pagers($node->content);
      foreach ($all_children as &$pager) {
        $pager['#post_render'][] = 'gotolastpage_append_fragment';
      }
      
    }

  }

}
nicorac’s picture

nicorac commented
Issue summary: View changes
nicorac’s picture

nicorac commented
Category: Bug report » Task
Status: Needs work » Needs review

Ooops, you're right.
I was using PHP 5.5 for another project of mine and I forgot to switch it back.

Your patch was committed.

I also fixed GIT instructions to point to the right 7.x-1.x branch:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/nicorac/2205137.git gotolastpage

Thank you.

tanvirahmad’s picture

tanvirahmad commented
Status: Needs review » Needs work

Unable to clone

git clone --branch 7.x-1.x-dev http://git.drupal.org/sandbox/nicorac/2205137.git gotolastpage
Cloning into 'gotolastpage'...
fatal: Remote branch 7.x-1.x-dev not found in upstream origin
Unexpected end of command stream

Binu Varghese’s picture

Binu Varghese commented

@nicorac, Fix it by changing your Git Clone URL to: e.g.

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/nicorac/2205137.git goto_last_page

It appears you are working on branch 7.x-1.x (not 7.x-1.x-dev).

nicorac’s picture

nicorac commented
Issue summary: View changes
Status: Needs work » Needs review

I'm working in 7.x-1.x branch, as said here: https://drupal.org/node/1015226
Sadly I forgot to update this page...
@Binu Varghese: thanks for pointing it out, now fixed.

nicorac’s picture

nicorac commented
Issue summary: View changes
nicorac’s picture

nicorac commented
Issue summary: View changes
nicorac’s picture

nicorac commented
Issue summary: View changes
nicorac’s picture

nicorac commented
Issue summary: View changes
nicorac’s picture

nicorac commented
Issue summary: View changes
Issue tags: +PAreview: review bonus
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Assigned: Unassigned » kscheirer
Status: Needs review » Reviewed & tested by the community

Review of the 7.x-1.x branch:

  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
    FILE: /home/klausi/pareview_temp/gotolastpage.module
--------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
--------------------------------------------------------------------------------
   5 | ERROR | Doc comment short description must be on a single line, further
     |       | text should be a separate paragraph
 126 | ERROR | Paramater tags must be grouped together in a doc commment
 132 | ERROR | Type hint "array" missing for $element
 182 | ERROR | Type hint "array" missing for $element
--------------------------------------------------------------------------------

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

  1. gotolastpage_node_load(): why hook_node_load()? This will will trigger if any node is loaded from the database for any purpose, even if it will not be displayed on the page. I think you should not mess around with $_GET['page'] here, but rather only in hook_node_view() where we can be certain that a node is going to be displayed. You can use hook_module_implements_alter() to make sure that run before comment module.
  2. _gotolastpage_find_all_children_pagers(): so this will trigger on any pager that is found in the node content, not only comments? What if I have an EVA view attached to my node that also uses a pager? Why do you apply that function to $node->content and not to $node->content['comments'] (or whereever the comments are located)?
  3. gotolastpage_url_outbound_alter(): this will also trigger on every link on every page that has nothing to do with nodes or comments (many Views I would assume). So you are not only changing the paging behaviour of comments on nodes but for anything on you Drupal site that uses a Pager. I think that should have a big fat warning on the project page?

But otherwise looks RTBC to me.

Assigning to kscheirer as he might have time to take a final look at this.

nicorac’s picture

nicorac commented

@klausi: thanks for your review, really appreciated.

Coder Sniffer has found some issues with your code

I fixed them.
I'm using PHP_CodeSniffer through PEAR with Drupal, DrupalPractice and DrupalSecure standards enabled, but I can't get the warning you reported (and I like to).
The same with Coder Sniffer module (I must disable the PEAR package otherwise the code module gives a "function redefinition" error). Which standards have you configured?

I think you should not mess around with $_GET['page'] here, but rather only in hook_node_view() where we can be certain that a node is going to be displayed.

Great suggestion. I used hook_node_load() because it surely runs before the comment module comes into the game, but hook_module_implements_alter() is a better way. Now I've set gotolastpage_node_view() to run at first.
Running gotolastpage_node_view() earlier required another change: the code to add link fragment to pager links is now moved to gotolastpage_node_view_alter(), run when the whole content is ready to be rendered and so comment module already have added its content.

Why do you apply that function to $node->content and not to $node->content['comments'] (or whereever the comments are located)?

Good suggestion again (that's the community added value...).
While moving the fragment code to the new gotolastpage_node_view_alter() I also changed it to scan into &$build['comments'] only.

gotolastpage_url_outbound_alter(): this will also trigger on every link on every page that has nothing to do with nodes

I added a test to run the "URL cleanup" only if we're active.
Anyway the removal happened only when a "page=99999999" parameter existed, which is not so usual ;)

Thanks again for your detailed review.

nicorac’s picture

nicorac commented
StatusFileSize
new patch_0001.patch755 bytes

I'm having some issues with GIT access to my sandbox.
I'll attach a patch file here, while I'll try to sort it out.

Fixed, just committed the fixes to GIT.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Reviewed & tested by the community » Fixed

no objections for more than a week, so ...

Thanks for your contribution, nicorac!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.