-
Notifications
You must be signed in to change notification settings - Fork 218
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
RocketCDN CNAME change #7027
Comments
Scope a solution ✅Add new method to handle the notice for RocketCDN here and the newly created method to the The method created above should only be displayed based on this conditions here Then in this file add a new method
Add the APiClient as constructor to Subscriber Finally, add the method created as a callback to Add test, update existing ones. Estimation[S] |
As the approach is based on the RocketCDN API, the plugin release with this change must happen after this is released: https://github.com/wp-media/rocket-cdn/issues/546 |
Removing the 3.17.2 milestone as the RocketCDN API won't be ready for 3.17.2. |
This point is still not handled by this grooming, correct? @Khadreal |
It's the notice in WPR is dismissible by default, so I don't think it's needed to cover that again. |
I'm not talking about the dismiss, I'm talking about the notice itself, I believe we need to show it in WPR settings along with rocketCDN settings page. |
@wordpressfan Yes, the notice should be there. @DahmaniAdame I think we should display a |
@piotrbak I agree. It's informational and non-critical. Blue will do. We can use orange if we need a second message later in the process if we notice that the former CNAME is still heavily used. |
Grooming to complete with the notice, as pointed out above, before moving it to Grooming to Review. |
Everything related to RocketCDN should be handled in RocketCDN classes. The update of the CDN cname can be done in the We also need to take into account saving the old CDN URL to be able to display it in the notice. Moving it back to grooming to do. |
GroomingWill take the grooming mentioned above by @Khadreal with some small changes. Proposed SolutionIn this subscriber:
Add a new callback to that array
For showing the notice, We will create a new view file here: with the notice HTML contents, then we will create a new method here:
To get the old cdn url from the option We may need to remove the option Then adjust tests. Effort[S] |
LGTM |
Is your feature request related to a problem? Please describe.
In the upcoming version we'll need to change the RocketCDN CNAMEs automatically
Describe the solution you'd like
The Acceptance Criteria is described here:
https://www.notion.so/wpmedia/RocketCDN-CNAME-change-d72cd2baa88b49b7b4cb65b348440564?pvs=4
The text was updated successfully, but these errors were encountered: