Problem/Motivation

Since Drupal 7.33 you can opt in for block caching, see https://api.drupal.org/api/drupal/CHANGELOG.txt/7

For block caching to work, custom blocks should declare their caching strategy in hook_block_info. Nodeblock currently declares no cache property on its block. This results in notices when a site opts in for block caching:

Notice: Undefined property: stdClass::$cache in _block_get_cache_id() (line 967 of /[...]/modules/block/block.module).

Proposed resolution

Declare the default caching strategy for nodeblocks

Remaining tasks

  1. Write a patch
  2. Review

User interface changes

None

API changes

None

CommentFileSizeAuthor
#1 nodeblock-cache_info-2425347-1.patch1.02 KBidebr

Comments

idebr’s picture

idebr commented
Status: Active » Needs review
StatusFileSize
new nodeblock-cache_info-2425347-1.patch1.02 KB

Attached patch expands nodeblock_block_info to return a cache setting on its blocks. The default for blocks without a cache strategy is DRUPAL_CACHE_PER_ROLE, so this change can be applied without breaking any sites currently using Nodeblock.

  • Johnny vd Laar committed 55d2ef1 on 7.x-1.x 
    Issue #2425347 by idebr: Declare a caching strategy in hook_block_info...
Johnny vd Laar’s picture

Johnny vd Laar commented
Status: Needs review » Needs work

I've taken a different approach for this. I check whether there are node_grants implementing modules. If so then caching is disabled otherwise the caching is per role.

Only problem now is that there might also be modules implementing hook_node_access and do special stuff...

So perhaps it should always be DRUPAL_NO_CACHE. But it feels a bit strange to disable caching entirely while in most cases the block can be cached.

http://cgit.drupalcode.org/nodeblock/commit/?id=55d2ef1

Johnny vd Laar’s picture

Johnny vd Laar commented
Status: Needs work » Fixed

Ok I've added a variable such that you can override cachability if you want to. And I added info in the readme to make sure everyone understands that it's dangerous to do so.

http://cgit.drupalcode.org/nodeblock/commit/?id=e76ab98

Thanks for the help!

  • Johnny vd Laar committed e76ab98 on 7.x-1.x 
    Issue #2425347 by idebr: Override cachability with a variable.

Status: Fixed » Closed (fixed)

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