Caching does not bring much:
- In my test it takes less than 15 ms to recalculate the page list.
- The progress block is supposed to be displayed only on checkout pages, which usually take a little percentage of whole page views.

Caching has a big problem: it is not context-aware. For example, the pages are languague-dependent (fixed in #1376526: Caching of commerce_checkout_pages is not localization-aware), and could be role-dependant, product-dependant... (pages list is change in hook_commerce_checkout_page_info_alter() which is not cached). Of course this can be fixed in every hook_commerce_checkout_page_info_alter implementation with something like:

if (module_exists('commerce_checkout_progress')) {
  commerce_checkout_progress_get_items(TRUE);
}

And this is a pain for contrib modules. So I propose that we remove caching.

CommentFileSizeAuthor
#5 1471552.5-commerce_checkout_progress-remove-cache.patch6.13 KBmrfelton

Comments

jcisio’s picture

jcisio commented

The above method does not work, neither, needs this (before this issue is fixed):

cache_clear_all('commerce_checkout_progress:pages', 'cache', TRUE);
jcisio’s picture

jcisio commented
Title: Page list should not be cached » Block and page list should not be cached
Category: feature » bug

Even the whole block should not be cached. Different roles may have different checkout pages (e.g. an registration step for anonymous user).

If the maintainers are ok with those ideas, I'll submit a patch.

webmasterkai’s picture

webmasterkai commented

A patch would be great. Thanks.

mrfelton’s picture

mrfelton commented

I found myself having to flush the cache all the time in order to have checkout pages that are optional, and where inclusion is dynamically calculated based on properties of the order.

/**
 * Implements hook_commerce_checkout_page_info_alter().
 *
 * Modify the 'Continue to next step' button text
 */
function cw_commerce_commerce_checkout_page_info_alter(&$pages) {
  global $user;
  global $language;

  if (isset($pages['cw_commerce_currency_page'])) {
    // We need to clear commerce_checkout_progress's cache,
    // as we alter this based on various non-static conditions.
    $cache_id = 'commerce_checkout_progress:pages:' . $language->language;
    cache_clear_all($cache_id, 'cache');
  }
}
mrfelton’s picture

mrfelton commented
Status: Active » Needs review
StatusFileSize
new 1471552.5-commerce_checkout_progress-remove-cache.patch6.13 KB

Here is a patch that removes all caching.

jcisio’s picture

jcisio commented
Status: Needs review » Reviewed & tested by the community

The patch looks good, and it is exactly what I thought.

jcisio’s picture

jcisio commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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