Settings: Recognize valid Akismet keys from wp-config and restrict input

[mattgawarecki]

Fixes #8226.

Changes proposed in this Pull Request:

• Add a check in the Antispam component to look for indications of a globally-set WPCOM_API_KEY for Akismet.
• If a globally-set API key is detected, inform the user and disable input.

Jetpack product discussion

See #8226.

Does this pull request change what data or activity we track or use?

No.

Testing instructions:

How to globally define an Akismet API key:

• In the Jetpack > Akismet Anti-spam menu, if an API key is already set, disconnect or remove it.
• In your site's wp-config.php file, add the following line: define( 'WPCOM_API_KEY', 'your api key' ); where your api key is your desired API key.

To test changes in this PR:

• Globally define a valid Akismet API key for your site (see instructions above).
• Verify that in Jetpack > Settings > Security, the Anti-spam card shows "Your site is protected from spam." and the text input is disabled, like in the below screenshot.

To test for regressions:

• Globally define an invalid Akismet API key for your site (see instructions above).
• Verify that in Jetpack > Settings > Security, the Anti-spam card shows "Your site is not protected from spam." and all UI elements appear/behave as they did before this PR.
• Remove all global and UI-based API key definitions from your site.
• Verify that in Jetpack > Settings > Security, the Anti-spam card shows "Your site needs an Antispam key." and all UI elements appear/behave as they did before this PR.
• Verify that the state of the Anti-spam card changes correctly based on the state of the Your API key text input.

Screenshot (After)

Proposed changelog entry for your changes:

• Settings: Globally-configured Akismet API keys (e.g., in wp-config.php) now show as valid and cannot be altered from the UI.

[eeeeevon13]

This is looking good from a design standpoint. In the wording (inside the disabled input) is there a way to be more specific? I had originally had it say "a valid key has been set in wp-config.php" Just wondering if a user who did not make this change would be better served by understanding specifically where it is set?

[jetpackbot]

	Warnings
⚠️	pre-commit hook was skipped for one or more commits

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

E2E results is available here (for debugging purposes): https://jetpack-e2e-dashboard.herokuapp.com/pr-16542

Generated by 🚫 dangerJS against d3e27c6

[BrookeDot]

I had originally had it say "a valid key has been set in wp-config.php" Just wondering if a user who did not make this change would be better served by understanding specifically where it is set?

I would <3 to be more specific here but the problem is we can't assume that the constant is set in wp-config.php Maybe the text can be filterable so it can be changed at the provider level?

For example, I could add the key to any plugin or theme and it would still be set.

I'm tempted to use WPCOM_API_KEY in the message but it's a poorly named constant which may make things more confusing. If we overlook that maybe this is more clear,

A key has been set using the WPCOM_API_KEY constant

or

The WPCOM_API_KEY constant is set.

[mattgawarecki]

@eeeeevon13 @BrookeDot More than happy to oblige with whatever copy y'all think is best. 😄

On a separate note, to everyone: code-wise, is this PR looking okay?

[BrookeDot]

@beaulebens Do you have thoughts, on the text of the message itself or know who should chime in here?

[beaulebens]

I doubt I'm the best person to comment here, but there are really only 2 instances that I can think of where this handling/message are relevant;

1. Because the user themselves set a constant in code somewhere (very intentional, very specific, they should know what's going on), or, probably more likely;
2. The sysadmin/host/someone else set a constant, so they know what's going on, but the specific user does not.

I'd lean towards optimizing for the second one, but either way just making the message something pretty generic like "Your site has been configured with a valid antispam key" or something like that.

Addressed the change

[mattgawarecki]

1. Because the user themselves set a constant in code somewhere (very intentional, very specific, they should know what's going on), or, probably more likely;
2. The sysadmin/host/someone else set a constant, so they know what's going on, but the specific user does not.

@beaulebens Does my proposed copy, "A valid key has been set in your site's configuration," cover both of these cases?

Specifically looking at case 2, I think this wording lets the user know that even though they may not have set a key themselves, a valid key exists somewhere and has been recognized, whether put there by a provider or another site admin. I'm not a copywriter or a designer, though, only a developer 😅 What are your thoughts?

something pretty generic like "Your site has been configured with a valid antispam key" or something like that.

Since we also have help text below the input that reads "Your Antispam key is valid," I wonder if mentioning Antispam again woulld come across as too repetitive?

[beaulebens]

That change works for me, yep. I think it strikes a good enough balance of detail and simplicity.