-
Notifications
You must be signed in to change notification settings - Fork 210
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
Elementor templates clearing cache in full #5848
Comments
Acceptance Criteria
|
Reproduce the problem ✅I was able to reproduce the problem. Identify the root cause ✅Just like @DahmaniAdame Stated Scope a solution ✅To stop full cache when updating a template in elementor, we can do a check in wp-rocket/inc/functions/files.php Line 587 in 1f8dfc9
To remove associated posts when elementor modules are updated are 2 ways
We can do checks against this values and clear as expected. Estimate the effort ✅[S] @engahmeds3ed What do you think? |
@jeawhanlee @engahmeds3ed The effort here is [M], not [S], right? |
@piotrbak can we add clearing the Used CSS as a requirement for this one? Or should I open a separate issue for it? |
@DahmaniAdame Just to be sure, it's about clearing the Used CSS of related posts, right? |
@piotrbak yes, exactly. If the template is edited, the style will change. What will happen is that we will be shipping the wrong style on related posts if we just clear cache. Both cache and Used CSS should be cleared. |
@piotrbak Can you also provide the ability to disable cache clearing when templates are edited? In my case, I have many templates that affect only one page. The site has over 1000 pages, and every time the editor makes changes to the text, the entire cache is cleared. |
@piotrbak It's the same issue. When we edit Elementor templates, the cache is cleared in full. I just like to have the possibility to disable clearing the cache in full with a filter when templates are edited. |
@DahmaniAdame I think this should be limited to specific templates only. As per @wp-media/qa and I also confirmed this on my website, if you edit the regular template using free Elementor, the changes are not applied to the pages that contain this template. Steps to reproduce :
The templates that are auto-updating are for sure:
|
@valmirselmani Yes, I can see that. For now you could modify the following helper plugin to achieve what you want: |
The regular templates are like partials to be added to pages. They are not dynamic, as far as I know. Updating a template won't update the page where it was inserted. It's not expected that the page's template content will change if the template is update. And we are not expected to clear the page's cache if the template is updated.
I believe it already is. It's supposed to target only templates adding entries to the post's @valmirselmani current issue is what this ticket is about. WP Rocket converts the template's path to become This is what this part will fix - wp-rocket/inc/functions/files.php Lines 590 to 592 in 457d4c5
@valmirselmani for now, using the helper plugin to disable automatic cache clearing as advised by Piotr should help. |
@DahmaniAdame The issue initially was about the all templates, was surprised while checking the QA report + doing some research 😁 Sending it back to QA then. Do you have a list of templates that are triggering the cache clearing or it should be explored? |
@piotrbak the first part, where all cache is cleared, is about all templates! So, what needs to be checked by @wp-media/qa is if cache is still fully clear if any template updated. It's the second part where specific posts are cleared related to For this one, what needs to be checked is if all posts with the specific popup for example added are the only ones being cleared.
Unfortunately, no. I tested with Popups initially. But Header/Footer should have the same logic. Add-ons with popups or handling at least these 3 types are likely piggybacking on that logic as well. Basically, whatever adds a template element with an entry on the post's |
@piotrbak @DahmaniAdame I already have this https://github.com/wp-media/wp-rocket-helpers/blob/master/cache/wp-rocket-no-cache-auto-purge/wp-rocket-no-cache-auto-purge.php in place, but it still clears the cache in full. |
@valmirselmani can you open a ticket for someone to have a closer look? |
I'm blocking this one for now, it needs a product discussion a bit, will move it back to grooming as soon as we have all the answers. |
@DahmaniAdame This issue became more complicated than we thought it'll be. To avoid releasing important regressions, I'd suggest adding to
Also, we should preserve the current functionality already introduced in this PR:
If we introduce the above, there'll be regression related to not clearing cache of the Templates with Conditions, as per @jeawhanlee: Handling all those scenarios with cache clearing will be complex and time-consuming, on the other hand, right now when saving the template we're clearing everything. |
@piotrbak let's go with that.
Do we have a way to know if it's just text or get Elementor involved to provide a way to do so? If yes, we can use that to clear the Used CSS selectively if not just text is changed. If not, we should go with clearing the Used CSS to avoid risks of displaying a broken page. |
@DahmaniAdame Are we okay with this part?
We could either:
|
Yes. That's acceptable. Can we involve Elementor and see if they can offer a hook for caching plugins to handle that part? Otherwise, option 2 looks like a good middle ground. 1 might cause an unnecessary cache/RUCSS cycle. And 3 will be high maintenance. |
To follow-up on this one, with new acceptance criteria. We'll preserve the current functionality of this PR:
And for the second scenario, with the condition templates:
|
Reproduce the problemI saw @jeawhanlee was able to reproduce the problem on the first grooming and explained the root cause. Scope a solutionThe solution we gonna have here will have to reuse as much as possible. 'rocket_pre_clean_post' => [ 'exclude_post_type_cache_clearing', 10, 2 ], /**
* Exclude elementor library post type from cache clearing.
*
* @param mixed $clear_post A preemptive return value. Default null.
* @param Post $post Post Object.
* @return boolean
*/
public function exclude_post_type_cache_clearing( $clear_post, $post ) {
if ( 'elementor_library' === $post->post_type ) {
return true;
}
return $clear_post;
} Then concerning the modal we will have to first handle the logic that will detect a change on the elementor template. function setup_transient($check, $object_id, $meta_key, $meta_value, $prev_value) {
if('_elementor_conditions' !== $meta_key || $meta_value === $prev_value || ! $this->options->get('', false)) {
return;
}
set_transient('wpr_elementor_need_purge', true);
$boxes = get_user_meta( get_current_user_id(), 'rocket_boxes', true );
unset($boxes['maybe_clear_cache_change_notice'']);
} Once we done that then we have to handle the notice by adding the following callback on public function maybe_clear_cache_change_notice() {
$boxes = get_user_meta( get_current_user_id(), 'rocket_boxes', true );
if ( in_array( __FUNCTION__, (array) $boxes, true ) ) {
return;
}
if ( ! current_user_can( 'rocket_manage_options' ) ) {
return;
}
$notice = get_transient( 'wpr_elementor_need_purge' );
if ( ! $notice ) {
return;
}
$args = [
'status' => 'warning',
'dismissible' => '',
'dismiss_button' => __FUNCTION__,
'message' => sprintf(
// translators: %1$s = <strong>, %2$s = </strong>, %3$s = <a>, %4$s = </a>.
__( '%1$sWP Rocket:%2$s Your Elementor template was updated. Clear the Used CSS if the layout, design or CSS styles were changed.', 'rocket' ),
'<strong>',
'</strong>',
'</a>'
),
'action' => 'clear_cache',
];
rocket_notice_html( $args );
} Also we need to handle the clearing from related posts when an Elementor module is cleared. For that we need to hook the following callback to public function clear_related_post_cache( int $post_id ) {
global $wpdb;
$template_id = '%' . $wpdb->esc_like( $post_id ) . '%';
// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching
$results = $wpdb->get_results(
$wpdb->prepare(
"SELECT post_id FROM `$wpdb->postmeta` WHERE `meta_key` = %s AND `meta_value` LIKE %s",
[ '_elementor_data', $template_id ]
)
);
if ( ! $results ) {
return;
}
foreach ( $results as $result ) {
if ( 'publish' === get_post_status( $result->post_id ) ) {
rocket_clean_post( $result->post_id );
if ( $this->options->get( 'remove_unused_css', 0 ) ) {
$url = get_permalink( $result->post_id );
$this->used_css->delete_used_css( $url );
}
}
}
} Estimate the effortEffort |
@piotrbak I would need a elementor pro license to test the implmentation the old one look like expired |
For the value check with the previous values. In the current state it will be always different as the previous value always return |
Related case here: https://secure.helpscout.net/conversation/2481477875/469222?folderId=2675957 |
Before submitting an issue please check that you’ve completed the following steps:
3.13
Describe the bug
In that specific case, it's the popup module. But the same logic should apply to all Elementor's templates.
Templates don't have a permalink structure.
Links look like:
When these popups are updated and
rocket_clean_post
is triggered, we extract the path to the root cache folder.And we end up purging
/public_html/wp-content/cache/wp-rocket/domain.ext/
wp-rocket/inc/functions/files.php
Lines 590 to 600 in 1f8dfc9
rocket_clean_files
should be safeguarded against processing that kind of URLs.We could possibly add an additional check here:
wp-rocket/inc/functions/files.php
Line 588 in 1f8dfc9
To see if
$parsed_url['path'] !== '/'
.In addition to that, ideally, we shouldn't pick up the post permalink when dealing with Elementor's templates.
Instead, we should restrict cache purging to posts where the template, in this example, a popup, is being used.
Assuming that the template post id is
12345
The element will be referenced as part of the
_elementor_data
custom field when being used on a page.Inside these data, an entry like the following will be found. Here is an example for popups:
We can have a SQL query with the template ID to check the
wp_postmeta
table to collect all related posts, and purge them from cache.We could maybe have a new function to replace
rocket_clean_post
as the hook to process Elementor templates cache clearing to not over complicaterocket_clean_post
logic?To Reproduce
Steps to reproduce the behavior:
Expected behavior
We shouldn't clear the cache in full in that context.
Screenshots
N/A
Additional context
Ticket - https://secure.helpscout.net/conversation/2179456687/407285/
Issue temporarily fixable using the following helper plugin - https://drive.google.com/file/d/1DAmftPxPWgld3MEyXPWMhmukOqwzsi98/view?usp=sharing
Backlog Grooming (for WP Media dev team use only)
The text was updated successfully, but these errors were encountered: