WordPress.org

Make WordPress Core

Opened 12 months ago

Last modified 6 months ago

#39072 new defect (bug)

Add gmt_offset as a REST API setting

Reported by: jshreve Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 4.7
Component: REST API Keywords: needs-unit-tests needs-patch
Focuses: rest-api Cc:

Description

The timezone string can be seen and updated via the REST API, but there is no way to get the GMT offset (which I would imagine is more more valuable for calculating things). There is also no way to update it (aka manual offset).

The attached patch is one way to fix it. It makes sure to clear out the timezone string if you are manually overriding/setting the offset. I'm not sure if what I have is the best way to handle that case for the API, but I took a stab at it. If a timezone string is set, the offset is correctly returned for that timezone.

Attachments (1)

gmt_offset_rest_api.diff (1.9 KB) - added by jshreve 12 months ago.

Download all attachments as: .zip

Change History (16)

#2 @jnylen0
12 months ago

We need a way to expose gmt_offset and other site info, but currently we intend the settings endpoint to be used only for settings that can be updated (both reading and writing require manage_options capability). See also #38731.

#3 follow-up: @jnylen0
12 months ago

  • Keywords has-patch needs-unit-tests added

currently we intend the settings endpoint to be used only for settings that can be updated

My mistake about this part - sitting next to @jshreve and he pointed out that this is possible to update via wp-admin.

This ticket also made me think of the general need to expose gmt_offset and other data to API clients that may not have manage_options capability. For Quick Draft this is currently done via wp_localize_script which isn't a great solution (https://core.trac.wordpress.org/ticket/38342#comment:48).

#4 in reply to: ↑ 3 @rmccue
12 months ago

Replying to jnylen0:

currently we intend the settings endpoint to be used only for settings that can be updated

My mistake about this part - sitting next to @jshreve and he pointed out that this is possible to update via wp-admin.

gmt_offset is actually a bit of a strange setting; it can either be set (the manual timezone offsets in the timezone dropdown), or derived from the actual timezone. Likely, the settings endpoint should expose the raw value (either unset or the manual timezone offset), and the site info can expose the derived value (which will always have a value).

This ticket was mentioned in Slack in #core by helen. View the logs.


12 months ago

#6 @pento
12 months ago

  • Milestone changed from Awaiting Review to 4.7.1

We can chat about this a bit more, but it doesn't need to be in 4.7.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


12 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


11 months ago

#9 @dd32
11 months ago

  • Keywords needs-patch added; has-patch removed

I'm marking this as needs-patch, as it sounds like the approach in gmt_offset_rest_api.diff isn't complete for what we should be doing here.

#10 @jnylen0
11 months ago

  • Milestone changed from 4.7.1 to 4.7.2

gmt_offset and timezone_string are weird. I'd like to see this weirdness abstracted out of the API so that you can set and read either one any time that operation would make sense.

This isn't going to make it into 4.7.1 though.

This ticket was mentioned in Slack in #core by jnylen. View the logs.


10 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


9 months ago

#13 @jnylen0
9 months ago

  • Milestone changed from 4.7.3 to 4.8

Punting to 4.8 due to inactivity.

This ticket was mentioned in Slack in #core by flixos90. View the logs.


6 months ago

#15 @flixos90
6 months ago

  • Milestone changed from 4.8 to Future Release

Punting further due to inactivity.

Note: See TracTickets for help on using tickets.