Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add full page cache support to post listings #293

Open
wants to merge 1 commit into
base: 2.4
Choose a base branch
from

Conversation

aapokiiso
Copy link

@aapokiiso aapokiiso commented Oct 24, 2020

Description

Magento gathers all identities from blocks and adds them to the X-Magento-Tags header for the page.
By adding the post cache tag to the post list identities, pages cached to the full page cache
containing post lists are cleared from the full page cache when any post is saved in Magento.

Additionally fixed a bug where the post model didn't implement the identity interface.
Without implementing the interface, Magento is not aware that the post model has identities.

Manual testing scenarios

  1. Load a post list in frontend, make sure it is loaded from full page cache.
  2. Create a new post in Magento admin and save it.
  3. Load the post list again. It should be cleared from full page cache and show the updated list with the new post.

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)

@gocom
Copy link

gocom commented Oct 24, 2020

This PR fixes #287

Code checks failing sucks when its not really the change's issue. It is implenting a mandatory interface and then using the proper public class constant from the entity. In my opinion the rules should be reworked to better meet the code base's requirements.

@phutx
Copy link
Contributor

phutx commented Oct 27, 2020

hi @aapokiiso,
Thanks for your contribution.
We will review and update this as soon as possible.

We highly appreciate and idea like you to make our product better.
Thank you very much!

@aapokiiso
Copy link
Author

@phutx Hi, thanks for the comment! It would be great if eventually the module would not use the cacheable="false" workaround at all, and instead always just clear relevant identities when saving posts / topics / categories / tags etc 🙂

@zaietsv
Copy link

zaietsv commented Aug 30, 2022

I can confirm the cache issue still persists. Blog pages are not cacheable.
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants