Automattic / msm-sitemap

Comprehensive sitemaps for your WordPress VIP site. Joint collaboration between Metro.co.uk, WordPress VIP, Alley Interactive, Maker Media, 10up, and others.
74 stars 37 forks source link

adding fix to ticket 157 setting the query date to use the timezone o… #158

Closed juankk closed 1 year ago

juankk commented 4 years ago

…f the site

tomjn commented 4 years ago

Should it not use UTC timestamps for all data saved and queried? WP stores all time and date values as UTC timestamps, then converts into localised time when displayed. The server too should be using UTC time.

Needing to use localised time to query implies that either somebody incorrectly used localised times somewhere and stripped out the timezones so that 2pm Sydney time became 2pm UTC, or that the timestamps are being stored incorrectly by the plugin. by my understanding this PR will break things for some or all users

t-wright commented 4 years ago

Should it not use UTC timestamps for all data saved and queried?

It currently queries post_date which is a localised timestamp. An alternative could be to query post_date_gmt but this column is not indexed.

technosailor commented 4 years ago

@tomjn I work with @juankk and trying to follow up on this to find a resolution. I understand your concern and @t-wright's thought on using post_date_gmt is sort of a good idea except UTC !== GMT for things like DST.

Regardless, perhaps a way forward here, and I can submit a PR with a different approach, is to use filters here. And also maybe hardened libraries for this.

Idea:

$start_date = apply_filters( 'msm_sitemap/start_date', function ( $start_date ) {
    $date = new \DateTime( $time, new \DateTimeZone( 'Australia/Sydney' ) );

    return $date->format( 'Y-m-d H:i:s' );
} );

$end_date = apply_filters( 'msm_sitemap/end_date', function ( $end_date ) {
    $date = new \DateTime( $time, new \DateTimeZone( 'Australia/Sydney' ) );

    return $date->format( 'Y-m-d H:i:s' );
} );

Thoughts?