Skip to content

Conversation

@nmalevanec
Copy link
Contributor

Original Pull Request

#13762

As noted in issue #13595 the Magento\Theme\Block\Html\Footer block is not saved to cache. This issue has been looked at as part of #MLAU18

Description

In investigating the issue it was found that the footer block does not have a cache lifetime set and it also has code in its constructor to set the cache lifetime to false. This pull request removes the forcing of cache lifetime to false and sets the default value to 3600 as done so in the \Magento\Theme\Block\Html\Topmenu block.

Fixed Issues (if relevant)

  1. loadCache for Block Magento\Theme\Block\Html\Footer dont work #13595

Manual testing scenarios

  1. Disable full page caching so that block level caching may be observed
  2. View a page that contains the footer block. E.g. homepage
  3. Debug \Magento\Framework\View\Element\AbstractBlock::_saveCache on the first time a given footer block is loaded, and see that is is successfully saved to cache.
  4. Debug \Magento\Framework\View\Element\AbstractBlock::_loadCache on a following request and confirm that the observed footer block is loaded from cache.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-engcom-team
Copy link
Contributor

Hi @ihor-sviziev, thank you for the review.
MAGETWO-88264 has been created to process this Pull Request

@magento-engcom-team magento-engcom-team merged commit 2407845 into magento:2.3-develop Feb 26, 2018
magento-engcom-team pushed a commit that referenced this pull request Feb 26, 2018
…to false in constructor and set default cache_lifetime to 3600 #13785
@nmalevanec nmalevanec deleted the 2.3-develop-PR-port-13762 branch August 21, 2018 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants