-
Notifications
You must be signed in to change notification settings - Fork 91
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
gpld-push-min-date-to-next-day-after-set-time.php
: Fixed an issue with timezones.
#1054
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThe change updates the minimum date calculation by replacing the previous use of Changes
Sequence Diagram(s)sequenceDiagram
participant C as Caller
participant F as gp-limit-dates Script
participant DT as DateTime Object
C->>F: Request minimum date calculation
F->>DT: Initialize DateTime('tomorrow midnight') with current timezone
DT-->>F: Constructed DateTime object
F->>DT: Retrieve timestamp from DateTime object
DT-->>F: Return timestamp value
F-->>C: Return minDate timestamp
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Context
⛑️ Ticket(s): https://secure.helpscout.net/conversation/2875355741/79394
Summary
The snippet above doesn't work, if the site timezone is set to something like Chicago.
Steps to recreate the issue
Here's the video demo of the issue: https://www.loom.com/share/4d5d42b268764351ac52ed29fe69d865?sid=0e71fa96-ff02-4be0-908b-52a85d4ae7b4
We had
strtotime( 'midnight tomorrow', $current_time->getTimestamp() )
which caused issues becausestrtotime()
interprets relative times in UTC, which seems to getting incorrect results in different timezonesThis fix proposed here ensures that "tomorrow midnight" is correctly calculated in the same timezone as
$current_time
by using new aDateTime
object specifying "tomorrow midnight".