Wp-rocket: Cloudflare error while saving Optimal Settings

[piotrbak]

Describe the bug
When filling in Cloudflare credentials and enabling Optimal Settings in one step, we are receiving an error and settings are not saved.

To Reproduce
Steps to reproduce the behavior:

1. On WP Rocket install with Cloudflare
2. Fill the correct Cloudflare credentials
3. Enable Optimal Settings
4. Save settings
5. The error should be outputted, CF settings not changed.

Expected behavior
Optimal settings should be enabled correctly
OR
Optimal settings button should be disabled until credentials are filled in

Screenshots
Error screenshot:
https://i.vgy.me/1Xinkg.jpg

Additional context
The problem was investigated by @vmanthos and @crystinutzaa here:
https://wp-media.slack.com/archives/CBJ2KQ0MU/p1583302791209100

This bug is a continuation of the following one:
https://github.com/wp-media/wp-rocket/issues/2295#issue-559886603

Backlog Grooming

• [x] Reproduce the problem
• [x] Identify the root cause
• [x] Scope a solution
• [x] Estimate the effort

[crystinutzaa]

Reproduce the issue ✅
Reproduced on my local test site

Identify the root cause ✅
The root cause is related to the order of the operations.
Optimal Settings are saved with AJAX call, while Cloudflare credentials are saved when submitting WP Rocket settings page.

Scope a solution ✅
1.Disable Optimal Settings (and all other CF options which run with AJAX call) until there are some valid credentials.

1. Send CF credentials while doing the AJAX call and save & validate.

Estimate the effort ✅
Depending on the solution selected the effort should be [XS] if Solution 1 is selected or [S] if solution 2 is selected.

[GeekPress]

@crystinutzaa The solution 2 seems to be most appropriate in term of user experience.

[arunbasillal]

Acceptance Criteria

• Enable Cloudflare add-on, enter valid credentials
• Enable Optimal Settings
• Click Save Changes
• No error message should appear. And Optimal settings should be applied correctly (check with Cloudflare account manually). The Optimal settings notice should appear.

What If

Couldn't think of edge cases. Suggestions are welcome

[Tabrisrp]

I think we need more considerations on this one:

Since it's an AJAX call, how do we display success/error to the user when he checks this option?

• We can't wait for after they click on save changes, it's already too late, the option has been updated already.
• We have to take into account the possibility that the credentials added are not correct. We also have to sanitize & validate them, while they have not been submitted yet with the form, else we introduce a security risk.
• What if the user already had credentials, but they changed it (without saving yet), and they check the optimal settings. Which credentials should be used?
• Does it make sense that an AJAX action like this is possible when we don't have sanitized/validated credentials yet?

Personal take
The development mode and optimal settings options should not be updated with AJAX. They need valid, secure credentials before being switched on/off, to make sure we perform the CF API calls as expected.

It's very likely our users don't know that those are updated via AJAX anyway, so they will be clicking on save changes after, creating a double request for the same thing.

We could simplify this and improve performance by making this change.

[GeekPress]

@Tabrisrp Does the acceptance criteria will be still the same? If so, we can go with your solution.

EDIT: Myself didn't know that development mode and optimal settings are updated with AJAX. I always click on Saves changes after updating the options 😕

[GeekPress]

@Tabrisrp I guess the effort won't be the same anymore with this solution, isn't it?

[Tabrisrp]

@crystinutzaa I'd like your input on what I suggested, what do you think?

Acceptance criteria would be the same, I believe the task is simpler with that solution? Credentials validation is already done when we save changes.

[GeekPress]

@Tabrisrp @crystinutzaa As the acceptance criteria will be the same, feel free to decide the solution you think it will be the best to implement.

[crystinutzaa]

@Tabrisrp yes, this is an even better solution. Does not need the Ajax Call in there, so on form submit these 2 options can be saved too.

@GeekPress the acceptance criteria will be exact the same.

What will happen, the credentials are validated first. If the credentials are ok, these 2 options development mode and optimal settings will be modified.
However, if the credentials fail what should happen if these 2 options are modified at the same time:

1. If credentials were modified with any of these options: do nothing. CF credentials error will be displayed. (This error should not be duplicated).
2. If the credentials did not modified and were already invalid: should display an error message only once (even if both options are changed) that the CF credentials are invalid.
3. If any other error related to any of these options happen, should display the error.

What do you think? Is this going to be ok?

[arunbasillal]

@crystinutzaa I agree that the notice should only be displayed once and that one should be the invalid credentials notice.

What if: Credentials are wrong, but Optimal settings etc was enabled. When we save settings, will Optimal settings be saved as enabled?

• I think the best user experience is to save the state of the "switcher" buttons because the user intended to enable them.
• They simply made a mistake with the credentials.
• So once they correct the wrong credentials, they do not have to do anything else and everything works as expected.