This is a great module.

All that is needed, at the given time is a little tweaking, to resolve the only 10 posts bug; a solution that can be found here
Also I noticed that when a custom pager is placed "In a sidebar block" and your website has block caching enable from Site configuration -> Performance the pager stops working correctly.
I found that by editing the custom_pager.module file around line 112, we can set the custom pager block to have no cache:

function custom_pagers_block($op = 'list', $delta = 0) {
  if ($op == 'list') {
    $pagers = _custom_pagers_load_all_pagers();
    foreach ($pagers as $pager) {
      if ($pager->position == 'block') {
        $blocks[$pager->pid]['info'] = $pager->title;
+	$blocks[$pager->pid]['cache'] = BLOCK_NO_CACHE;
      }
    }
    return $blocks;

In order for this to work you need to change the existing customer pager to another option say "Above the node's body" and save, refresh the Site building ->Blocks page(so that the custom pager block no longer exists ), then change the "Pager position" again to sidebar block.
Now it should work.
Hopefully some of these features(?bugs?) will be added(resolved) in the future.

CommentFileSizeAuthor
#8 custom_pagers-block_caching-796782-8.patch4.46 KBMikeActually
#6 custom_pagers-block_caching-796782-6.patch4.43 KBMikeActually
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

OldAccount’s picture

OldAccount CreditAttribution: OldAccount commented

Thanks, I was having a heck of a time trying to figure out what happened to my pager when it started acting strange (showing correct total number but not progressing past 3 out of 12, etc.) I tried this and it seems to have done the trick.

asb’s picture

asb CreditAttribution: asb commented

Is this fixed in beta2?

akosipax’s picture

akosipax CreditAttribution: akosipax commented
Version: 6.x-1.x-dev » 6.x-1.0-beta2

No it isn't fixed in beta2...

doublejosh’s picture

doublejosh CreditAttribution: doublejosh commented

Tried to solve this via Block Cache Alter but it is not respected and the pager is always the same for everyone node in the view. This updates to whatever I look at first after clearing the cache. Even setting the block to "Do Not Cache" rather than "Per page" does not solve the problem.

rooby’s picture

rooby CreditAttribution: rooby commented
Title: Custom pager in Sidebar block with Block cache enabled » Add block caching option to custom pagers
Version: 6.x-1.0-beta2 » 7.x-1.x-dev
Assigned: dwb17 » Unassigned

This is also a problem in the drupal 7 version.

I will make a patch soon to allow you to select a caching option per pager.

Just setting to BLOCK_NO_CACHE is not a great solution because then you lose all caching.
For example, for my use DRUPAL_CACHE_PER_PAGE is preferrable.

MikeActually’s picture

MikeActually CreditAttribution: MikeActually commented
Issue summary: View changes
FileSize
custom_pagers-block_caching-796782-6.patch4.43 KB

I threw this patch together to help resolve this issue;

Maybe there's a module out there that can do per block cache definitions? If there is, I'm not aware of it.

This should allow you to select the type of caching on a per pager basis. It can probably use some work, but I think will help most people.

MikeActually’s picture

MikeActually CreditAttribution: MikeActually commented
Status: Active » Needs review
MikeActually’s picture

MikeActually CreditAttribution: MikeActually commented
FileSize
custom_pagers-block_caching-796782-8.patch4.46 KB

Updated patch; this is a bit better in that it doesn't make the drop-down a required field (silly oversight on my part) and also allows for a selection with no value.

rooby’s picture

rooby CreditAttribution: rooby commented
Status: Needs review » Needs work

Being able to configure caching would be a nice addition.

patch review:

  1. +++ b/TODO.txt
@@ -3,4 +3,6 @@
-  so that custom pagers can be easily cached inside of Views.
\ No newline at end of file
+  so that custom pagers can be easily cached inside of Views.
+* Better implement setting block specific caching to rather than using a
+  switch case to define the Drupal env variable
\ No newline at end of file

    I don't think we should have this. I think if we're fixing it with this patch just do it the preferred way now instead of adding a todo. 

  2. +++ b/custom_pagers.admin.inc
@@ -189,7 +189,29 @@ function custom_pagers_form($form, &$form_state, $pid = NULL) {
-
+  ¶

    Trailing whitespace. 

  3. +++ b/custom_pagers.admin.inc
@@ -189,7 +189,29 @@ function custom_pagers_form($form, &$form_state, $pid = NULL) {
+    $form['cache'] = array(
+      '#type' => 'fieldset',
+      '#collapsible' => TRUE,
+      '#collapsed' => TRUE,
+      '#title' => t('Drupal caching'),
+      '#description' => t("The following sets what type of caching will be used when the \"sidebar block\" is selected. <a href=\"https://api.drupal.org/api/drupal/modules!block!block.api.php/function/hook_block_info/7\" target=\"_blank\">Click Here</a> for more information on Drupal's caching options."),
+    );

    I think this should have a more specific name. Since we are specifically dealing with block caching it should say block caching instead of drupal caching. I think the database column should also reflect this and be block_cache or block_cache_granularity or something.

    Instead of escaping your internal double quotes just use single quotes to wrap the string.

    URLs should not go in t() because they will break if translated. See https://api.drupal.org/api/drupal/includes!common.inc/function/t/6 for the prefered way to put URLs in translated strings. 

  4. +++ b/custom_pagers.admin.inc
@@ -189,7 +189,29 @@ function custom_pagers_form($form, &$form_state, $pid = NULL) {
+		  t('') => t(''),

    There is no need top use t() for an empty string. It's also not a good idea to use t() for the option key because translations could give you inconsistent values. 

  5. +++ b/custom_pagers.admin.inc
@@ -189,7 +189,29 @@ function custom_pagers_form($form, &$form_state, $pid = NULL) {
+		  'DRUPAL_CACHE_PER_ROLE' => t("DRUPAL_CACHE_PER_ROLE (drupal default)"),
+		  'DRUPAL_CACHE_PER_USER' => t("DRUPAL_CACHE_PER_USER"),
+		  'DRUPAL_CACHE_PER_PAGE' => t("DRUPAL_CACHE_PER_PAGE"),
+		  'DRUPAL_CACHE_GLOBAL' => t("DRUPAL_CACHE_GLOBAL"),
+		  'DRUPAL_NO_CACHE' => t("DRUPAL_NO_CACHE"),
+		),

    For these options it's possible it would be better to use human readable text for the array values instead of the constants. Although I can see the value in having them match the API docs, site admins aren't necessarily technical people and probably don't want to go and read an API page.
    If you are going to use the constant values to match the API then don't use t() because you don't want translations making it so the values don't match the API docs.
    Also in this case I don't think it is necessary to have (drupal default) for the role option, it would be better to just remove the empty option and have the role option as the default, seeing as that is what will happen if no caching type is set.
    If a user was to select the empty string and Drupal one day changes the default value the functionality of the user's website will change unexpectedly so I think it's better to force a value.

    Also, it is best to keep using single quotes for consistency and only use double quotes if your string contains single quotes or you want to put variables in the string (which you wouldn't want with t()). 

  6. +++ b/custom_pagers.admin.inc
@@ -189,7 +189,29 @@ function custom_pagers_form($form, &$form_state, $pid = NULL) {
+		'#description' => t('The node type(s) this custom pager will apply to.'),

    This description needs changing. If you want to mention what the default value is you can also add it here to the description instead of as part of the option text itself. 

  7. +++ b/custom_pagers.install
@@ -72,8 +72,23 @@ function custom_pagers_schema() {
+      'caching' => array(
+        'type' => 'varchar',
+        'length' => 50,
+        'not null' => TRUE,
+        'default' => '',
+        'description' => 'The cache setting for the block version of this {custom_pager}.',
+      ),

    Again I think it would be better to specify block cache for naming and describing this. 

  8. +++ b/custom_pagers.install
@@ -72,8 +72,23 @@ function custom_pagers_schema() {
+/*
+ * Implements hook_update_N()
+ * 7000 adds block caching control.
+ */

    missing the second asterisk on the first line.

    In this specific case (update functions) it is best not to use "Implements hook_update_N()" because the comment text of the function gets displayed to the user when they run the update. So it should just describe what the update is doing.

    There should be an empty line (with leading asterisk) between the first line (short description) and any additional longer description. This won't affect the description being shown to the user that I just mentioned because that has new lines stripped out.

  9. +++ b/custom_pagers.install
@@ -72,8 +72,23 @@ function custom_pagers_schema() {
+function custom_pagers_update_7000() {

    You should not use 7000 as the number. 7000 is generally reserved for updates specific to upgrading from a 6.x version to a 7.x version.
    7100, 7200, etc. are generally reserved for updates specific to upgrading between 7.x-1.x and 7.x-2.x etc., so I would recommend using 7101 for this update.

  10. +++ b/custom_pagers.install
@@ -72,8 +72,23 @@ function custom_pagers_schema() {
+	$schema = custom_pagers_schema();

    You shouldn't call a hook_schema implementation from an update function because in future the schema that hook_schema() returms may have changed, which then changes what your update function is doing but updates that update functions perform should remain the same forever. So it is best to hard code the field array here even though it is code that is currently duplicated in hook_schema(). It could save broken updates in future. 

  11. +++ b/custom_pagers.module
@@ -147,6 +147,27 @@ function custom_pagers_block_info() {
+	  if($pager->caching != null && $pager->caching != '') {
+		//$blocks[$pager->pid]['cache'] = php_eval($pager->caching);
+		switch ($pager->caching) {
+			//TODO: there must be a better way than a switch case; tried php_eval but did not work properly - miheath
+			case 'DRUPAL_CACHE_PER_ROLE':
+				$blocks[$pager->pid]['cache'] = DRUPAL_CACHE_PER_ROLE;
+				break;
+			case 'DRUPAL_CACHE_PER_USER':
+				$blocks[$pager->pid]['cache'] = DRUPAL_CACHE_PER_USER;
+				break;
+			case 'DRUPAL_CACHE_PER_PAGE':
+				$blocks[$pager->pid]['cache'] = DRUPAL_CACHE_PER_PAGE;
+				break;
+			case 'DRUPAL_CACHE_GLOBAL':
+				$blocks[$pager->pid]['cache'] = DRUPAL_CACHE_GLOBAL;
+				break;
+			case 'DRUPAL_NO_CACHE':
+				$blocks[$pager->pid]['cache'] = DRUPAL_NO_CACHE;
+				break;
+		}
+	  }

    For this section you should be able to do something like this:

    <?php
if (!empty($pager->caching)) {
  $blocks[$pager->pid]['cache'] = constant($pager->caching);
}
?>
  12. There are a bunch of indentation issues throughout. Indenting should be 2 spaces. See https://www.drupal.org/coding-standards#indenting
  13. Since this block caching setting is specific to the value the user selected for the position setting I think this new field should go directly underneath the position field instead of down the bottom. Then use element #states to set the cache field to visible only when the position value = block. Since it is one setting and showing and hiding only when the relevant position is selected I think it would be best to also remove the fieldset. You can move any description you want from the fieldset to the description on the cache field itself.