REST API: Add gmt_offset to base /wp-json response

[jnylen0]

API clients often have a need to retrieve information about the current site. This doesn't fit into the wp/v2/settings endpoint for a couple of reasons:

• That endpoint requires administrative privileges (current_user_can( 'manage_options' )). See also #38731.
• We decided not to implement finer-grained permissions on the settings endpoint, as a read-only piece of information isn't a read/write "setting". See also ticket:38490#comment:10.

The most immediately obvious piece of information to add is gmt_offset. This is a crucial piece of information for any clients that need to manipulate datetimes returned via the API, but it's not currently available anywhere. See also ticket:38342#comment:48 for the specific case of Quick Draft inside wp-admin.

I expect we will need to enhance the base /wp-json response to accept a ?context=edit or similar parameter - so let's use this ticket for both the approach to /wp-json and the addition of the gmt_offset field specifically.

[swissspidy]

Why only gmt_offset and not timezone_string as well?

Am I understanding this correctly: this ticket wants to add this to /wp-json/, while #39072 wants to add the setting to the settings endpoint?

[jnylen0]

Replying to swissspidy:

Why only gmt_offset and not timezone_string as well?

gmt_offset is more immediately useful to clients because it's an easily parseable number and timezone_offset isn't always set, but we could certainly do both.

Am I understanding this correctly: this ticket wants to add this to /wp-json/, while #39072 wants to add the setting to the settings endpoint?

Yes - currently the settings endpoint is admin-only, so if we stick with that decision then there is value in having it in both places. See also #38731.

[sagarkbhatt]

Added gmt_offset in default wp-json endpoint.

[jnylen0]

Hi @sagarkbhatt! Thanks for uploading a patch. However, I think we need to only show this information for an authenticated request with ?context=edit.

We also need to make sure that gmt_offset returns an appropriate value even if the timezone_string is set instead, to e.g. America/New_York. These two options behave a bit strangely together.

Finally we'll want unit tests for each of these cases - ​https://make.wordpress.org/core/handbook/testing/automated-testing/phpunit/

[sagarkbhatt]

This patch wil show gmt_offset only when context=edit

[jnylen0]

@sagarkbhatt thanks for working on this, though I think it is going to take a couple more iterations to get it finished.

Currently, I can visit e.g. ​https://nylen.io/wp-dev/wp-json?context=edit without authentication. Your latest patch would change that request to return an error, which I don't think is a good idea.

One way to work around this would be to just add the additional arguments if the request is correctly authenticated. This is backwards-compatible; however, it isn't very discoverable. Marking as dev-feedback to get other contributors' opinions here.

current_user_can( 'edit_posts' ) looks like a reasonable authentication check here, though I think simply is_user_logged_in() would also work.

I don't think wp_parse_args is the right function to use here, and we want to make sure the code reads well even after other values are added. Maybe something like this would work:

if ( 'edit' === $request['context'] ) {
    if ( is_user_logged_in() ) {
        $available['gmt_offset'] = get_option( 'gmt_offset' );
    }
}

Finally, as noted before, we need unit tests for this change. I can think of several cases to test:

• Unauthenticated request: gmt_offset does not appear in the response.
• Request with insufficient authentication: gmt_offset does not appear in the response.
• Authenticated request: gmt_offset does appear in the response.
• gmt_offset should always be a number, regardless of whether it is set or the timezone_string is set instead. (I don't recall exactly how this works, maybe we don't have to do anything special here.)

[sagarkbhatt]

Updated patch according to discussion.

[sagarkbhatt]

gmt_offset should return int value regardless setting of timezone_string.

[rmccue]

Strong -1 on is_user_logged_in(), we should always be checking authorization (capabilities) not authentication (presence of user).

Why do we need to restrict this in any case? I see no potential security or privacy issues in exposing this.

Also, if we expose gmt_offset, we should also expose timezone_string. The former is not stable (i.e. it changes regularly in timezones with daylight savings), and the latter is the only real one you can depend on over time.

[rmccue]

Replying to jnylen0:

We also need to make sure that gmt_offset returns an appropriate value even if the timezone_string is set instead, to e.g. America/New_York. These two options behave a bit strangely together.

Also on this point, gmt_offset is shimmed in wp_timezone_override_offset(), so reading it is always accurate. Only writes are tricky, but we're not handling this here, so no need to do anything special other than get_option()

[sagarkbhatt]

-Added gmt_offset and timezone_string field in wp-json response.
-A user can view both without any authentication.

[jnylen0]

Replying to rmccue:

gmt_offset is shimmed in wp_timezone_override_offset(), so reading it is always accurate. Only writes are tricky, but we're not handling this here, so no need to do anything special other than get_option()

Thanks - I always forget how this works 🙈

Given the discussion above, 39854.4.patch​ looks good to me, just needs tests.

[sagarkbhatt]

Added unit tests

[sagarkbhatt]

Added unit test

[jnylen0]

39854.5.patch​ looks good to me, thank you @sagarkbhatt.

I don't have any specific concerns with exposing this new information either, but my first thought was to restrict it since it's not really available elsewhere. Unless someone disagrees with the approach here, we'll get it committed after the 4.7.3 release ships.

[dd32]

Unless someone disagrees with the approach here, we'll get it committed after the 4.7.3 release ships.

I would prefer to see *everything*, especially "new" things committed to trunk for soak well in advance of 4.7.x merges, just to get extra testing of things prior to branch use - even though this is a small change, catching small discrepancies is hard if things get trunk commits delayed for any reason.

[jnylen0]

Noted, and makes sense. I'll get this committed as soon as we're done with shipping 4.7.3.

[jnylen0]

Also update wp-api-generated.js fixture file

[jnylen0]

When you run the entire phpunit test suite it also updates the wp-api-generated.js fixture file if needed. When we make changes to the default API responses, this file needs to be updated as well. See #39264 for more info about that.

39854.6.diff​ is the same patch but with the updates to the fixture file.

[jnylen0]

In 40238:

REST API: Add gmt_offset and timezone_string to the base /wp-json response.

The site's current timezone offset is an important piece of information for any REST API client that needs to manipulate dates. It has not been previously available.

Expose both the gmt_offset (the site's current offset from UTC in hours) and timezone_string (which also provides information about daylight savings time) via the "site info" endpoint (the base /wp-json response).

Also update the wp-api-generated.js fixture file with the changes to the default API responses.

Props sagarkbhatt.
Fixes #39854.

[jnylen0]

Reopening for backport to 4.7 branch.

[swissspidy]

In 40336:

REST API: Add gmt_offset and timezone_string to the base /wp-json response.

The site's current timezone offset is an important piece of information for any REST API client that needs to manipulate dates. It has not been
previously available.

Expose both the gmt_offset (the site's current offset from UTC in hours) and timezone_string (which also provides information about daylight
savings time) via the "site info" endpoint (the base /wp-json response).

Also update the wp-api-generated.js fixture file with the changes to the default API responses.

Props sagarkbhatt.
Fixes #39854.

Merges [40238] to the 4.7 branch.

[adamsilverstein]

@swissspidy thanks for this which will be super helpful when calculating dates.

I noticed you changed wp-api-generated.js in [40235], however, this file is generated by the phpunit test test_build_wp_api_client_fixtures and I think your changes will get overwritten when this test runs. I haven't had a chance to look carefully, I'm guessing you will need to update the generator test instead.

[swissspidy]

@adamsilverstein I think you mean [40336] , not [40235]? I am not the one to thank for as I only did the backport.

In my tests, test_build_wp_api_client_fixtures did not overwrite the wp-api-generated.js file. The test shouldn't need to be overwritten as it only prints out REST responses, which were appropriately adjusted in this ticket.

As for fixture generation, see also #40041. It's a bit weird that the file gets updated by a single PHP unit test.

[adamsilverstein]

Replying to swissspidy:

@adamsilverstein I think you mean [40336] , not [40235]? I am not the one to thank for as I only did the backport.

Ah right, looked odd to me on first glance as that file is generated and explicitly marked as 'shouldn't be edited'.
I will dig a bit deeper here to validate. Thanks.