Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

text_datetime_timestamp_timezone field returns wrong datetime object #1465

Open
mklasen opened this issue Oct 18, 2022 · 10 comments · May be fixed by #1468
Open

text_datetime_timestamp_timezone field returns wrong datetime object #1465

mklasen opened this issue Oct 18, 2022 · 10 comments · May be fixed by #1468

Comments

@mklasen
Copy link

mklasen commented Oct 18, 2022

Describe the bug

When using the text_datetime_timestamp_timezone field, setting a date, time and timezone, the saved Datetime object returns the wrong date.

Example:

  • I set 20/10/2022 at 03:00PM Europe/Amsterdam
  • The saved object contains: 20/10/2022 at 05:00PM Europe/Amsterdam

Steps to reproduce:

  1. Go to https://gist.github.com/mklasen/7931140ed5499abf7dc8207ab6df40f8
  2. Copy the code into a plugin file
  3. Create a new post, set a date, time and timezone
  4. See the output below the field

Screenshots

Markup on 2022-10-18 at 17:10:17

CMB2 Field Registration Code:

<?php
/**
 * Plugin Name: CMB2 text_datetime_timestamp_timezone Test
 */

add_action(
	'cmb2_admin_init',
	function() {

		$details = new_cmb2_box(
			array(
				'id'           => 'cmb2_test',
				'title'        => __( 'Details', 'cmb2' ),
				'object_types' => array( 'post' ),
			)
		);

		$details->add_field(
			array(
				'name'        => 'Date',
				'type'        => 'text_datetime_timestamp_timezone',
				'id'          => 'date',
				'after_field' => function() {
					$serialized_date = get_post_meta( get_the_ID(), 'date', true );
					if ( ! empty( $serialized_date ) ) {
						$date = maybe_unserialize( $serialized_date );
						echo '<br><br>Date/Time: ' . $date->format( get_option( 'date_format' ) . ' ' . get_option( 'time_format' ) . ' e' ) . '<br>';

					}
				},
			)
		);

	}
);

Your Environment

Ubuntu, when running date in terminal results in: Tue Oct 18 15:11:45 UTC 2022

Additional context

I read some other issues that mention this field, but don't think they are related to what I am experiencing here.

@tw2113
Copy link
Contributor

tw2113 commented Oct 18, 2022

Have you confirmed your WordPress timezone settings are matching what you're expecting for things? From /wp-admin/options-general.php ?

@mklasen
Copy link
Author

mklasen commented Oct 19, 2022

Hmm, I would expect this setting to be unrelated? Let me run some tests though..

Also: updated the code sample so it doesn't fail on first load (forgot to check if post meta value was empty)

With UTC+0:
Markup on 2022-10-19 at 11:18:36

With Europe/Amsterdam and new post with new date/time/timezone:
Markup on 2022-10-19 at 11:20:08

When saving with UTC:
Markup on 2022-10-19 at 11:20:28

@mdibrahimk48
Copy link

Good work!

@tw2113
Copy link
Contributor

tw2113 commented Oct 19, 2022

I also wonder if the server's settings are playing a part in this, not just WP's timezone

@mklasen
Copy link
Author

mklasen commented Oct 19, 2022

Even if it does, I don't think it should. I'll have to admit though, I always get confused with timezones.

date returns Wed Oct 19 14:17:24 UTC 2022 in my container at the this moment.

When using the text_datetime_timestamp_timezone field, I would imagine CMB2 creates a new Datetime object, with the timezone set to the timezone specified by the user, based on UTC time (which seems correct).

@tw2113 @mdibrahimk48 - are you able to reproduce this? If not, then obviously something is wrong with my environment.

@mklasen
Copy link
Author

mklasen commented Oct 19, 2022

Ah, I just realized CMB2 also saves a UTC timestamp, so we can get the right results, see below:

				'after_field' => function() {
					$serialized_date = get_post_meta( get_the_ID(), 'date', true );
					$utc_date = get_post_meta( get_the_ID(), 'date_utc', true );

					if ( ! empty( $utc_date ) ) {

						$utc_date_object = new Datetime();
						$utc_date_object->setTimestamp( $utc_date );
						echo '<br><br><strong>Date/Time from UTC saved date:</strong><br/> ' . $utc_date_object->format( get_option( 'date_format' ) . ' ' . get_option( 'time_format' ) . ' e' ) . '<br>';

						$utc_date_object->setTimezone( new DateTimeZone( wp_timezone_string() ) );
						echo '<br><br><strong>Date/Time from UTC saved date in current timezone:</strong><br/> ' . $utc_date_object->format( get_option( 'date_format' ) . ' ' . get_option( 'time_format' ) . ' e' ) . '<br>';
					}

					if ( ! empty( $serialized_date ) ) {
						$date = maybe_unserialize( $serialized_date );
						echo '<br><br><strong>Date/Time from saved object:</strong><br/> ' . $date->format( get_option( 'date_format' ) . ' ' . get_option( 'time_format' ) . ' e' ) . '<br>';
					}
				},

Markup on 2022-10-19 at 16:54:47

@tw2113
Copy link
Contributor

tw2113 commented Oct 19, 2022

I'd need to set up and confirm all parts in the local environment to confirm. However it sounds like you may have a way forward at the moment.

@mklasen
Copy link
Author

mklasen commented Oct 19, 2022

Yes, using the saved UTC date does work. I'll dig into this a bit later and submit a PR.

@deltafactory
Copy link

Throwing my experience into the mix. Testing on a dev server/site where the site TZ is set to America/Los Angeles. I don't think the server's TZ is factoring in.

The date_utc value is correct but obviously has no TZ reference.
The serialized DateTime object is incorrect: the time value saved is based on the specified time and then offsets it based on the selected timezone. Example:

Date: 12/2/2022
Time: 12:00AM
TZ: Los Angeles

DateTime object:
O:8:"DateTime":3:{s:4:"date";s:26:"2022-12-01 16:00:00.000000";s:13:"timezone_type";i:3;s:8:"timezone";s:19:"America/Los_Angeles";}

I believe that the "date" should be "2022-12-02 00:00:00.000000"

The confusing bit is that the UI is correctly displaying the originally selected time. That is addressed by this line.

I see a few possible areas to address this but not without disrupting existing users of the text_datetime_timestamp field. I haven't looked at the pull request yet.

@mklasen
Copy link
Author

mklasen commented Dec 12, 2022

Yup, I realized the same thing - discarding the serialized object and using UTC only works, but that solution wouldn't follow CMB2 standards at all.

@jtsternberg and I agreed on updating the docs to clarify the UTC value is the right one to use in #1468.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants