Skip to content

Use local post date#300

Merged
NeilWJames merged 3 commits intowp-document-revisions:mainfrom
webelaine:fix/time-zone-bug
Nov 21, 2022
Merged

Use local post date#300
NeilWJames merged 3 commits intowp-document-revisions:mainfrom
webelaine:fix/time-zone-bug

Conversation

@webelaine
Copy link
Copy Markdown
Contributor

Changes proposed

  • Use the local-timezone post date rather than the GMT post date in permalinks.

What this fixes

When a WordPress site uses a time zone other than UTC, and a new document is published on the last day of the month at a time when UTC falls in the next month, the plugin generates an invalid permalink with the UTC month rather than the month in the local timezone. This causes a 404 error for the document's permalink.

How to reproduce the bug

  • Set your system time to October 31, 2022 at 9 p.m.
  • In Settings > General, set your WordPress time zone to Chicago.
  • Add a new document, complete with upload, and publish it publicly.
  • Try to view the document.
  • See that a 404 error is returned, because the permalink contains 2022/11 instead of 2022/10.

How to test the fix

  • Flush rewrite rules.
  • Follow the "reproduce the bug" steps, but see that in the end the document is viewable rather than erroring out.

@NeilWJames
Copy link
Copy Markdown
Collaborator

@webelaine,
Thank you for raising this bug. You have raised a real issue for month end and differences between time zones. Had you been in a time zone in advance of UTC, then the issue would arise in the first hours of the new month.

Since the permalink is giving parameters to do a look up on post_date, then the month and year have to match that of the post_date, not post_date_gmt. The field in the database is datetime. This does not contain any timezone element and it will be written as the number in the writer's timezone. In this case, 31 october at 21:00:00. Thus we need 2022/10 in the permalink.

However, I am concerned about the processing of this and the next line.

$timestamp = strtotime( $document->post_date );
This will take the datetime of the post date 2022-10-31 21:00:00 and make it the timestamp 2022-10-31 21:00:00 CST.
It is the same but with a time zone because that is the timezone from which it was published.
gmdate( 'Y', $timestamp ) . '/' . gmdate( 'm', $timestamp )
The next line takes that timestamp and applies gmdate to it. This expresses the timestamp in UTC - which is 2022/11/01 02:00:00 UTC So I am actually somewhat surprised that this now worked.

FWIW, my machine is normally set to Paris time. At 21:39, I created a post. In the database post_date is set as 21:39 and post_date_gmt is 20:39. I then set the WP instance to Chicago time. At 21:41, I created a new post. post_date was set to 14:41 and post_date_gmt to 20:41.

I think that the field post_date - a string valiable - already contains the required data - and the timezone conversions just confuses things.. Certainly I have spent a considerable time trying to understand the various timestamp operations.

I would like to propose for your consideration this replacement code:
` // build yyyy/mm directly from the post_date string.
$yr_mth = str_replace( '-', '/', substr( $document->post_date, 0, 7 ) );

		$link  = untrailingslashit( home_url() ) . '/' . $this->document_slug() . '/' . $yr_mth . '/';`

This avoids using timezone-related functions completely and extracts the required data directly.

Kind Regards,
Neil James

Instead of converting the timestamp to a datetime object, converting it to UTC, and outputting the year and month and date from that, just grab the month and year directly from the post_date.
@webelaine
Copy link
Copy Markdown
Contributor Author

@NeilWJames - you raise an excellent point. I agree that pulling the month and year directly from the post_date makes more sense than converting to a datetime and swapping timezones. I have pushed a new commit using this suggestion. Thanks for your help!

Copy link
Copy Markdown
Collaborator

@NeilWJames NeilWJames left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Existing use of the gmt field is consistent, but gives permalink errors when the user's time zone is different to gmt.
This resolves the error in the determination of the month of the post.

@NeilWJames NeilWJames merged commit 9e45663 into wp-document-revisions:main Nov 21, 2022
@NeilWJames NeilWJames mentioned this pull request Feb 26, 2023
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 this pull request may close these issues.

2 participants