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

RocketCDN CNAME change #7027

Open
piotrbak opened this issue Oct 10, 2024 · 10 comments
Open

RocketCDN CNAME change #7027

piotrbak opened this issue Oct 10, 2024 · 10 comments
Labels
module: CDN module: rocketcdn needs: documentation Issues which need to create or update a documentation needs: grooming type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Milestone

Comments

@piotrbak
Copy link
Contributor

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

@piotrbak piotrbak added module: CDN module: rocketcdn needs: grooming type: enhancement Improvements that slightly enhance existing functionality and are fast to implement labels Oct 10, 2024
@piotrbak piotrbak added this to the 3.17.2 milestone Oct 10, 2024
@piotrbak piotrbak added the needs: documentation Issues which need to create or update a documentation label Oct 10, 2024
@Khadreal
Copy link
Contributor

Khadreal commented Oct 17, 2024

Scope a solution ✅

Add new method to handle the notice for RocketCDN here and the newly created method to the admin_notices filter here.

The method created above should only be displayed based on this conditions here

Then in this file add a new method

public function update_cdn_name( $new_version, $old_version ): void {
		if ( version_compare( $old_version, '3.17.2', '>' ) ) {
			return;
		}
                delete_transient('rocketcdn_status');

		$this->api_client->get_subscription_data();
	}

Add the APiClient as constructor to Subscriber

Finally, add the method created as a callback to wp_rocket_upgrade

Add test, update existing ones.

Estimation

[S]

@MathieuLamiot
Copy link
Contributor

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
Depending on the delay on the service team side, we might want to take this out of 3.17.2

@MathieuLamiot MathieuLamiot removed this from the 3.17.2 milestone Oct 18, 2024
@MathieuLamiot
Copy link
Contributor

Removing the 3.17.2 milestone as the RocketCDN API won't be ready for 3.17.2.

@wordpressfan
Copy link
Contributor

We'll display a (success/warning/info) dismissible notice in the WP Rocket plugin settings page

This point is still not handled by this grooming, correct? @Khadreal

@Khadreal
Copy link
Contributor

It's the notice in WPR is dismissible by default, so I don't think it's needed to cover that again.

@wordpressfan
Copy link
Contributor

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.
cc @piotrbak

@piotrbak
Copy link
Contributor Author

@wordpressfan Yes, the notice should be there.

@DahmaniAdame I think we should display a blue, notification notice. WDYT?

@DahmaniAdame
Copy link
Contributor

@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.

@MathieuLamiot
Copy link
Contributor

Grooming to complete with the notice, as pointed out above, before moving it to Grooming to Review.

@MathieuLamiot MathieuLamiot added this to the 3.17.3 milestone Oct 23, 2024
@remyperona
Copy link
Contributor

Everything related to RocketCDN should be handled in RocketCDN classes. The update of the CDN cname can be done in the DataManagerSubscriber class, which already has the API client as a dependency.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: CDN module: rocketcdn needs: documentation Issues which need to create or update a documentation needs: grooming type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Projects
None yet
Development

No branches or pull requests

6 participants