wp-media / wp-rocket

Performance optimization plugin for WordPress
https://wp-rocket.me
GNU General Public License v2.0
701 stars 220 forks source link

Apply RocketCDN CNAME change #7095

Closed wordpressfan closed 1 week ago

wordpressfan commented 3 weeks ago

Description

Fixes #7027

Starting from 3.17.3, we will show a message if the cname changed in WPR settings page only.

Type of change

Detailed scenario

the approach that we took here is that with update, we will force refreshing the rocketcdn status which has the cname url on it and compare it with the old/previous url then set an option which will be used to show the admin notice in WPR side, so the trigger here is the WPR plugin update.

Technical description

Documentation

With update we will force getting rocketcdn cname again and if it's different that the old url, we will show the notice to let the customers know about that.

New dependencies

No

Risks

I mentioned some questions with risks here: https://wp-media.slack.com/archives/C06CQPWEJSK/p1731572752314419 to @wp-media/product team

Mandatory Checklist

Code validation

Code style

Additional Checks

codacy-production[bot] commented 3 weeks ago

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for e5bca6673a3669827f3998edebc0c785210fe561[^1] :x: 23.64% (target: 50.00%)
Coverage variation details | | Coverable lines | Covered lines | Coverage | | ------------- | ------------- | ------------- | ------------- | | Common ancestor commit (e5bca6673a3669827f3998edebc0c785210fe561) | Report Missing | Report Missing | Report Missing | | | Head commit (744a99c8d98bbea2ad85c1db699c5b0a3ddec372) | 38226 | 16726 | 43.76% | **Coverage variation** is the difference between the coverage for the head and common ancestor commits of the pull request branch: ` - `
Diff coverage details | | Coverable lines | Covered lines | Diff coverage | | ------------- | ------------- | ------------- | ------------- | | Pull request (#7095) | 55 | 13 | **23.64%** | **Diff coverage** is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: `/ * 100%`

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more [^1]: Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

wordpressfan commented 2 weeks ago

I'll be waiting for a reply here: https://wp-media.slack.com/archives/C06CQPWEJSK/p1731572752314419 to see if we will change the direction that we took here or we are fine.

wordpressfan commented 2 weeks ago

I'll change the way we handle this to detect the old and new urls when setting rocketcdn_status transient instead of WPR upgrade.

remyperona commented 2 weeks ago

PR approved to move to QA, but we should have tests coverage added to before merging

piotrbak commented 2 weeks ago

@wordpressfan There's still the question from Mathieu in this topic: https://wp-media.slack.com/archives/CUT7FLHF1/p1731692989584199?thread_ts=1731601939.328049&cid=CUT7FLHF1

Please make after merging this PR to send it to transifex.

Mai-Saad commented 1 week ago

As per slack, Trigger: On update from < 3.17.3 to >= 3.17.3 If the CDN URL is set to xxxxxxxx.rocketcdn.me (where xxxxxxxx is exactly 8 alphanumeric characters), then: change it to xxxxxxxx.delivery.rocketcdn.me display the banner with before/after values There should be no API call involved in this approach. ACs would be: (validation is WIP)

wordpressfan commented 1 week ago

Slack discussion: https://wp-media.slack.com/archives/CUKB44GNN/p1731939168323999

We need to create another issue to add more automated tests (unit/integration) to fix the codacy CI failure.