Blocks placed only with context are not used by authcache_block [#2190837]

I've been trying to get authcache_block working with authcache_ajax for loading blocks via AJAX calls on cached pages for authenticated users.

After much trying I finally got it working. I enabled authcache_debug, and in the debug widget I was getting the error:

Canceled: No client for fragment block/[module]-[delta]

I figured out that the block wasn't appearing in the table of routes at admin/config/system/authcache/p13n/frontcontroller, and rebuilding the routes would not get it to appear.

In the end I figured out that the reason for this problem was that I had placed the block in question using a context reaction, and the block was not in use anywhere else, meaning that Drupal did not see it as being enabled (ie. In the block table, the block's status was 0).

The first line of authcache_block_authcache_p13n_fragment() loads all the blocks with a status of 1 only:

$result = db_select('block', 'b')->fields('b')->condition('b.status', 1)->execute();

I can see the efficiency logic behind this approach, but I think that it is quite common for sites to use context as their only block placement system, in which case this logic unnecessarily limits the applications of the authcache module. Given that the subsequent logic in authcache_block_authcache_p13n_fragment() checks whether each block is actually enabled for using authcache I think that it is acceptable to remove the status = 1 condition from this query, and just load all blocks.

I will post a patch to this effect, for your consideration.

Comment	File	Size	Author
#4	blocks_placed_only_with-2190837-3.patch	1.14 KB	znerol
#4
#2	blocks_placed_only_with-2190837-2.patch	1.16 KB	znerol
#2
#1	fragments-for-all-authcache-enabled-blocks-2190837-1.patch	976 bytes	jamesharv
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

jamesharv CreditAttribution: jamesharv commented 7 February 2014 at 04:46

Assigned:	jamesharv	» Unassigned
Status:	Active	» Needs review

File	Size
fragments-for-all-authcache-enabled-blocks-2190837-1.patch	976 bytes

Here's the patch.

Comment #2

znerol CreditAttribution: znerol commented 12 February 2014 at 22:26

File	Size
blocks_placed_only_with-2190837-2.patch	1.16 KB

Thank you for the patch. However I'm not very comfortable removing the status-check completely. When only the block module is enabled I'd expect that a block becomes inaccessible when it is removed.

This is probably not the perfect approach either but I prefer a special case for the context module here.

What do you think?

Comment #3

12 February 2014 at 22:33

Status:

Needs review

» Needs work

The last submitted patch, 2: blocks_placed_only_with-2190837-2.patch, failed testing.

Comment #4

znerol CreditAttribution: znerol commented 12 February 2014 at 22:38

File	Size
blocks_placed_only_with-2190837-3.patch	1.14 KB

Clean up remains from another approach...

Comment #5

znerol CreditAttribution: znerol commented 12 February 2014 at 22:39

Status:

Needs work

» Needs review

Comment #6

jamesharv CreditAttribution: jamesharv commented 13 February 2014 at 01:00

Thanks for looking into this issue @znerol.

I don't think that the status property is very meaningful for blocks. I ran this test to check whether authcache_block fragments will get rendered for blocks with status != 1.

drush eval '
$frag = new AuthcacheBlockFragment(6, "system", "powered-by");
$block = $frag->load(array(), array());
echo "// Block object\n";
print_r($block);
$block_html = $frag->render(key($block), reset($block), array());
echo "// Block HTML\n";
print_r($block_html);
'

The result was:

// Block object
Array
(
    [6] => stdClass Object
        (
            [bid] => 6
            [module] => system
            [delta] => powered-by
            [theme] => bartik
            [status] => 0
            [weight] => 10
            [region] => footer
            [custom] => 0
            [visibility] => 0
            [pages] => 
            [title] => 
            [cache] => -1
        )

)

// Block HTML

<div class="swiper-slide"><div id="block-system-powered-by" role="complementary" class="block block--system block--system-powered-by">
        <div class="block__content">
    <span>Powered by <a href="http://drupal.org">Drupal</a></span>  </div>
</div>

So that does work - you can render a block with status = 0.

Another use case that I thought of, is that sometimes developers will directly render a block themselves for some reason (eg. in their own AJAX callback, or to embed a block inside the body content of a node with a token). Some of these scenarios may represent bad practice, but some would be legitimate. Also, the fact that the context module happily ignores the status property indicates there could be other contrib modules that do the same (eg. Maybe panels would also do this - although I don't have much experience with panels so I don't know).

So personally, I still prefer the approach of removing the status check. What do you think?

Comment #7

znerol CreditAttribution: znerol commented 17 February 2014 at 20:36

So that does work - you can render a block with status = 0.

Thats right. What I'm after is a mechanism where a block is only accessible from remote (through the Ajax callback) when the block is actually displayed anywhere on the site (i.e. status = 1, placed using the context module, etc.).

embed a block inside the body content of a node with a token

At the moment Blocks need to be placed as render-arrays into regions. Otherwise Authcache Block will not pick them up and replace the markup with Ajax-links. Authcache has support for Panels, therefore Blocks displayed in Panels need to be cached on the Pane-level.

Disregarding the block.status is indeed the easiest way fix the issue at hand. However I'm not sure yet whether we can risk that.

Comment #8

jamesharv CreditAttribution: jamesharv commented 18 February 2014 at 00:35

Hi @znerol,

I guess my thinking was that users can configure which blocks are available to authcache via a setting on each block's configuration page (stored in the variable 'authcache_block-' . $block_id). So it's not like every block will automatically be available via an AJAX callback because of removing the status condition. The site owner still has control over that.

But it's up to you of course. For my purposes it looks like your context patch will work, so I'd be happy if that gets committed at least.

Thanks again for your attention to this issue.