Site Settings: Allow language change even if WPLANG is defined as constant

[donpark]

wp-admin users can change site language even if WPLANG constant is defined in wp-config.php. This PR allows Calypso user to do the same in the general settings page (/settings/general/).

Fixes Automattic/wp-calypso#33440

Changes proposed in this Pull Request:

Removes defined( 'WPLANG' ) checks from Jetpack's Core Data API endpoints used by Calypso to change site settings.

Testing instructions:

1. Test with a Jetpack site whose wp-config.php contains the line define('WPLANG', '');
2. Navigate to URL: https://wordpress.com/settings/general/{site_slug}.
3. Check that error message no longer appears.

Before:

After:

4. Also check that site language can now be changed.

Proposed changelog entry for your changes:

• allow language to be changed even if WPLANG constant is defined.

[jetpackbot]

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: August 6, 2019.
Scheduled code freeze: July 30, 2019

Generated by 🚫 dangerJS against 7f447d1

[donpark]

Core displays a notice after site language is change if WPLANG constant is defined:

I felt above message may confuse Calypso users so chose not to display a similar notice. Fact that it involves a lot more work to display the notice in Calypso also contributed.

[gravityrail]

I have confirmed that this patch fixes the issue.

[donpark]

Reason for most recent update: Core's get_locale() uses WPLANG as default if defined and non-empty. Otherwise en_US is used.

[jeherve]

This handles empty strings for WPLANG well, but it does not handle valid strings.

For example, if I set WPLANG to fr_FR, here is what happens today with master:

And here is what happens with your branch:

That is not correct here, since my interface language is actually french, not English.

Maybe when WPLANG is defined and not empty, we should send the value back to WordPress.com so it can be displayed in Calypso.

[donpark]

For example, if I set WPLANG to fr_FR, here is what happens today with master:
...
That is not correct here, since my interface language is actually french, not English.

That may be a problem in Calypso, not in JP endpoint. With same setup, lang_id returned by remote Jetpack endpoint is French as remote site's wp-admin displays:

wp-admin:

But Calypso continues to display English as site language.

Since this PR is about the endpoint, perhaps I should update the test plan to use WPCOM Console so we can at least fix the JP endpoint behavior to match that of wp-admin.

Meanwhile I'm going to look into why Calypso is misbehaving. I also noticed several /me/setttings requests being made repeatedly when Settings page reloads, all failing with reauthorization_required that is ignored by Calypso.

Maybe when WPLANG is defined and not empty, we should send the value back to WordPress.com so it can be displayed in Calypso.

Did you test with latest version? That's is what is being done now.

$value = get_option( 'WPLANG' );
if ( empty( $value ) && defined( 'WPLANG' ) ) {
	$value = WPLANG;
}
$response[ $setting ] = empty( $value ) ? 'en_US' : $value;

[donpark]

Hold on. I may be looking at wrong API response.

[donpark]

OK. I was looking at the wrong endpoint. End point that matters is:
https://public-api.wordpress.com/rest/v1.1/jetpack-blogs/{site_id}/rest-api/?http_envelope=1&path=%2Fjetpack%2Fv4%2Fsettings%2F&json=true

Finding out why the endpoint kept returning en_US as lang_id was a head-scratcher because:

1. WPLANG option was not set and
2. invoking get_option( 'WPLANG' ) from wp shell returns false but
3. same function call in the endpoint code was returning en_US as value.

Replacing the call with get_option( 'WPLANG', '' ) removed the mystery value and Calypso started displaying same language value as wp-admin with WPLANG constant defined as fr_FR:

@jeherve I think something went wrong with your test setup because black warning notice only comes up if error_const is returned as lang_id value and, with this patch, error_const is never returned. In other words, that warning notice should not come up if test setup is correct.

[jeherve]

I think something went wrong with your test setup

Yes I am sorry, I pasted the same screenshot twice in my comment, that was a mistake.

[jeherve]

This tests well now. 👍