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

Partial Preloading doesn't create a mobile file in 3.5 #2399

Closed
4 tasks done
vmanthos opened this issue Mar 9, 2020 · 2 comments
Closed
4 tasks done

Partial Preloading doesn't create a mobile file in 3.5 #2399

vmanthos opened this issue Mar 9, 2020 · 2 comments
Assignees
Labels
effort: [M] 3-5 days of estimated development time module: preload priority: high Issues which should be resolved as quickly as possible type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Milestone

Comments

@vmanthos
Copy link
Contributor

vmanthos commented Mar 9, 2020

When updating a post, partial preloading will update the cached file.

Expected behavior

In 3.5, I'd expect partial preloading would create cached files for both desktop and mobile when the following options are enabled:

  1. Preload
  2. Separate cache for mobile devices

To Reproduce
Steps to reproduce the behavior:

  1. Make sure the previously mentioned options are enabled.
  2. Update a post.
  3. Check the post's folder in /cache/wp-rocket/.
  4. Only the desktop version will be there.

Backlog Grooming

  • Reproduce the problem
  • Identify the root cause
  • Scope a solution
  • Estimate the effort
@GeekPress GeekPress added needs: grooming priority: high Issues which should be resolved as quickly as possible type: enhancement Improvements that slightly enhance existing functionality and are fast to implement labels Mar 10, 2020
@GeekPress GeekPress added this to the 3.5.2 milestone Mar 10, 2020
@Screenfeed
Copy link
Contributor

Screenfeed commented Mar 10, 2020

Reproduce the problem ✅
This is true for all processes that use partial preload.

Identify the root cause ✅
Partial preload hasn’t been included in "mobile preloading".

Scope a solution ✅
Add mobile URLs within WP_Rocket\Subscriber\Preload->maybe_dispatch(), it will work for all "partial preload"-based processes.

Estimate the effort ✅
effort: [M]

Most of the effort will be on writing automated tests.

@Screenfeed Screenfeed self-assigned this Mar 10, 2020
@Tabrisrp Tabrisrp added the effort: [M] 3-5 days of estimated development time label Mar 10, 2020
@Screenfeed Screenfeed removed their assignment Mar 10, 2020
@arunbasillal
Copy link
Contributor

Acceptance Criteria

  • Enable "Separate Cache or Mobile Devices" option in WP Rocket "Cache" tab.
  • Enable "Activate Preloading" in the "Preload" tab. Keep "Sitemap Preload" disabled.
  • Clear WP Rocket cache.
  • Preload the cache.
  • Make sure mobile specific files are created during preload in addition to desktop files.
  • Edit a post and update it.
  • Make sure the homepage and the post specific cache is updated and the necessary mobile and desktop files are generated.

What if

  • WebP is enabled and WP Rocket is indeed generating separate webp file.
    ^ This is infact not an edge case, but by design WP Rocket should be generating the webp versions for both desktop and mobile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: [M] 3-5 days of estimated development time module: preload priority: high Issues which should be resolved as quickly as possible type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Projects
None yet
Development

No branches or pull requests

6 participants