Fix #3947: Add a notice on request fail on SAAS server from RUCSS

[CrochetFeve0251]

Description

In this PR we had a notice for the user to check if the IP from the SAAS server is authorized when we can't reach it. For that we did a check on the RUCSS API client and linked a notice to it.

Fixes #3947

Type of change

• [ ] Enhancement (non-breaking change which improves an existing functionality)

Is the solution different from the one proposed during the grooming?

No instead of an option I used a transient to ensure it will disappear in the time and not stay for months on the admin from the user.

How Has This Been Tested?

• [ ] Automated Test
• [ ] On my local env

Checklist:

• [ ] My code follows the style guidelines of this project
• [ ] I have performed a self-review of my own code
• [ ] I have commented my code, particularly in hard-to-understand areas
• [ ] I have made corresponding changes to the documentation
• [ ] My changes generate no new warnings
• [ ] I have added tests that prove my fix is effective or that my feature works
• [ ] New and existing unit tests pass locally with my changes
• [ ] Any dependent changes have been merged and published in downstream modules

[Mai-Saad]

@CrochetFeve0251 @piotrbak please find exploratory test notes/Qs below:

• Success/process notice is displayed with the blocking notices once enable RUCSS.

• Shouldn't it be just a notice (blue) not an error (red) like CPCSS? and shouldn't it be dismissable?

• Blocked notice is not displayed if we clear used CSS from the admin bar => I think it shall be displayed. what do you think?

• Do we need to do anything if we clear used CSS for this URL from page view? @piotrbak

• Enhancement for the notice UI, msg just beside WP Rocket + the options to allow IP to be bullet points like that below: What do you think?

WP Rocket: It seems a security plugin or the server's firewall prevents WP Rocket from accessing the Remove Unused CSS generator. The following IP address 135.125.83.227 should be allowlisted:

• In the security plugin, if you are using one
• The server's firewall - your host can help you with this

[piotrbak]

1. Success/process notice is displayed with the blocking notices once enable RUCSS. We should not display the success message when the Used CSS is not created for the homepage. The requests are blocked, so the Used CSS is not generated.
2. Shouldn't it be just a notice (blue) not an error (red) like CPCSS? and shouldn't it be dismissable? It should be red
3. Blocked notice is not displayed if we clear used CSS from the admin bar While on the dashboard page, we should display the red notice unless it's dismissed. Scenario should be handled:

• RUCSS is enabled and working
• Requests are blocked in wp-config.php or in the firewall
• We clear Used CSS
• The notice is displayed in the dashboard.

4. Do we need to do anything if we clear used CSS for this URL from page view? No, the notice should be displayed only in our dashboard.
5. Enhancement for the notice UI, msg just beside WP Rocket + the options to allow IP to be bullet points like that below: What do you think? Yes, totally agree about the bullet points 👍

[CrochetFeve0251]

1. Success/process notice is displayed with the blocking notices once enable RUCSS. We should not display the success message when the Used CSS is not created for the homepage. The requests are blocked, so the Used CSS is not generated.

   2. Shouldn't it be just a notice (blue) not an error (red) like CPCSS? and shouldn't it be dismissable? It should be red

   3. Blocked notice is not displayed if we clear used CSS from the admin bar While on the dashboard page, we should display the red notice unless it's dismissed. Scenario should be handled:

   • RUCSS is enabled and working

   • Requests are blocked in wp-config.php or in the firewall

   • We clear Used CSS

   • The notice is displayed in the dashboard.

   4. Do we need to do anything if we clear used CSS for this URL from page view? No, the notice should be displayed only in our dashboard.

   5. Enhancement for the notice UI, msg just beside WP Rocket + the options to allow IP to be bullet points like that below: What do you think? Yes, totally agree about the bullet points +1

For 3. if it doesn't show up that means we didn't done any request and if so it means that we have no way to know if we will have an error. For the me in this case the notice doesn't have to display.

For 4. do we need to add custom css if we want a bullet list do we still need to had it.

[piotrbak]

@CrochetFeve0251 The list without bullets doesn't look good, but we have the same behaviour for all our notices, let's don't change this for now. There's an improvement to do though:

1. Make sure each
2. starts from he capitilised letter
3. In the server's firewall instead of the server's firewall

I'll consult wording here and we might change it on the develop though.

[vmanthos]

@CrochetFeve0251 I know this is still in-progress but I gave it a run because I wanted to create the test cases. Please find my notes:

1. I'm not sure if you worked on this already but item 1 is still unresolved:

a. The notice has both the "Dismiss this notice" which lacks styling, and WordPress's x icon on the top right. We should use one of the two, I'm guessing that should be "Dismiss this notice".

b. Also, although the notice is dismissable it keeps popping up with every page refresh which, in a way, makes it non-dismissable.

In my opinion, the notice should be closed either permanently or for a set amount of time, e.g. 24 hours, and displayed/redisplayed when activating/reactivating Remove Unused CSS.

@piotrbak please clarify how these two points should be handled.

2. For the notice to appear the wp_rocket_rucss_errors_count has to be set: https://github.com/wp-media/wp-rocket/blob/0b7289bfa5df947b4394278ce5176e2dfc466d58/inc/Engine/Optimization/RUCSS/Admin/Subscriber.php#L415

For that a request to the SaaS should take place, i.e. treeshake() needs to run on rocket_buffer. If, for any reason, this takes a while - in my testing this was common, the notice will be displayed later on and only if the user goes back to WP Rocket's settings page. So, they won't know immediately that the used CSS cannot be generated.

Ideally, the notice should be displayed directly after enabling the option - maybe by making a request to the SaaS when the option is enabled.

Also, if the SaaS isn't reachable, maybe we should bail out early and not try to perform the requests to the API when rocket_buffer fires.

3. Maybe we can also check:

if ( defined( 'WP_HTTP_BLOCK_EXTERNAL' ) && WP_HTTP_BLOCK_EXTERNAL && ! is_wp_rocket_me_allowlisted() )

and display a different message prompting the customer to add *.wp-rocket.me to:

define( 'WP_ACCESSIBLE_HOSTS', '*.wordpress.org,localhost' );

* is_wp_rocket_me_allowlisted() should check if *.wp-rocket.me is in WP_ACCESSIBLE_HOSTS.

[Mai-Saad]

Thanks for the updates @CrochetFeve0251. Now the msg is more readable, however, still the counter and success notice are displayed while having a failure. can you please check?

[Mai-Saad]

Currently, if user dismiss the notice, then did any action that shall trigger it, it won't be displayed. As per discussion with @piotrbak it shall be displayed after transient expire

[Mai-Saad]

@CrochetFeve0251 Thanks for the updates. The transient wp_rocket_rucss_errors_count isnot deleted after deleting WPR. Can you please check :pray: Note: currently, if we dismiss the notice then do any action that shall trigger it, the transient will reset while notice won't be displayed

[vmanthos]

1. If, while RUCSS is blocked, the user clears the used CSS the notice is displayed:

If there isn't any used CSS in wpr_rucss_used_css to clear maybe we shouldn't display the notice. Note that if blocking of the SaaS came after Remove Unused CSS has been working for a while, it makes sense to display the notice.

@piotrbak What do you think?

2. Also, when the blocking is lifted, but the notice hasn't been dismissed it will not be automatically dismissed. It will be displayed until a successful request is made to the SaaS.

Is there anything we can/want to do about this?

3. About the bullets in this comment, it's my understanding we won't go with the CSS changes.

Instead, maybe we can prefix each line with a dash: -

[piotrbak]

@vmanthos If there isn't any used CSS in wpr_rucss_used_css to clear maybe we shouldn't display the notice. I think that's a good enhancement for the future.

Also, when the blocking is lifted, but the notice hasn't been dismissed it will not be automatically dismissed. It will be displayed until a successful request is made to the SaaS. That sounds totally okay, no changes needed.

Instead, maybe we can prefix each line with a dash: - Yes, that sounds reasonable 🆗

[vmanthos]

Thank you, @piotrbak! :pray:

@CrochetFeve0251 We should implement item number 3, here: https://github.com/wp-media/wp-rocket/pull/5240#issuecomment-1607333940

[Mai-Saad]

@CrochetFeve0251 Thanks for the updates. the transient is deleted now after deleting WPR and the - is added. Remain the following notes: 1- Currently, when the notice is displayed: if user 1 dismisses it, notice will never be displayed again to user 1, and whatever actions triggering it are made. User 2, will see the notice till user 2 dismisses it. => Nothing is needed here as per discussion with @piotrbak 2- We recommend change - In the server's firewall - your host can help you with this to - In the server's firewall, your host can help you with this or - In the server's firewall. Your host can help you with this => @piotrbak what do you think?

[piotrbak]

@CrochetFeve0251 Sorry for the wording changes... We'll go with this one: In the server's firewall. Your host can help you with this

[piotrbak]

Since the requirements have changed, we need the wording to be as follows:

It seems a security plugin or the server's firewall prevents WP Rocket from accessing the Remove Unused CSS generator. IPs listed here in our documentation should be added to your allowlists:
 - In the security plugin, if you are using one
 - In the server's firewall. Your host can help you with this

here in our documentation will be linking to https://docs.wp-rocket.me/article/1529-remove-unused-css#basic-requirements

The French doc is here: https://fr.docs.wp-rocket.me/article/1577-supprimer-les-ressources-css-inutilisees#basic-requirements