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.

Sign up for GitHub

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

Jump to bottom

Bugfix/issue 1429 (Zeitzone Cronjobs) #1430

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Bugfix/issue 1429 (Zeitzone Cronjobs) #1430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b5f8047
added getUTCFromDate
hansmorb Nov 27, 2023
9f57d20
fix #1429
hansmorb Nov 27, 2023
cae6724
removed warning message about timezones
hansmorb Nov 27, 2023
17d1edf
changed variable name
hansmorb Dec 8, 2023
87bb3b2
Merge branch 'master' into bugfix/issue-1429
hansmorb Dec 18, 2023
0b87e65
fixed test
hansmorb Dec 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion includes/OptionsArray.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,7 @@
'name' => esc_html__( 'Time', 'commonsbooking' ),
'id' => 'pre-booking-time',
'desc' => '<br>' . commonsbooking_sanitizeHTML( __(
'Define when the reminder should be sent. The actual sending may differ from the defined value by a few hours, depending on how your WordPress is configured.'
'Define when the reminder should be sent.'
, 'commonsbooking' ) ),
'type' => 'select',
'show_option_none' => false,
Expand Down
32 changes: 32 additions & 0 deletions src/Helper/Wordpress.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,13 +296,45 @@ public static function convertTimestampToUTCDatetime( $timestamp ) {
return $dto;
}

/**
* This function does no actual conversion to another timezone, it just sets the timezone to UTC
datengraben marked this conversation as resolved.
Show resolved Hide resolved
*
* @param $datetime
*
* @return DateTime
* @throws \Exception
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

@hansmorb hast du zufällig geschaut ob die Verwendung dieser Methode denn in einer korrekten Umfeld stattfindet? Das Umfeld also nicht annimmt das hier eine umgewandelte UTC-Zeit einer lokalen Zeit rauskommt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ich denke mal die Methode hat so wie sie existiert schon ihre Berechtigung, schließlich speichern wir ja alle timestamps in lokaler Zeit und nicht in UTC. Mit dieser Methode wird einfach nur ein DateTime Objekt erzeugt. Das ist dann zwar immer noch nicht in UTC aber hat zumindest eine Zeitzone (das war glaube ich für den iCalendar relevant).

public static function getUTCDateTime($datetime = 'now'): DateTime {
$dto = new DateTime($datetime);
$dto->setTimezone(new \DateTimeZone('UTC'));

return $dto;
}

/**
* Converts a datestring to a Datetime object in the UTC timezone.
* Modelled after WordPress get_gmt_from_date().
*
* As opposed to @see self::getUTCDateTime()
* this function sees the datestring as local time and converts the time to UTC.
hansmorb marked this conversation as resolved.
Show resolved Hide resolved
* @param $localDateString
*
* @return DateTime
* @throws \Exception
*/
public static function getUTCFromDate ($localDateString): DateTime {
$datetime = date_create( $localDateString, wp_timezone() );

if ( $datetime === false ) {
//use this as fallback if date_create fails
Copy link
Contributor

Choose a reason for hiding this comment

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

Wieso der fallback? Und für welche Fälle? Wieso nicht einfach auch false zurückgeben, anstatt potentiell eine falsche Zeit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Das ist aus der WordPress Funktion übernommen, ich dachte mir das ist schon gut getestet also übernehme ich das :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Und wenn wir eine falsche Zeit zurückgeben wird zumindest der Job geschedulet, sonst führt das ganze vielleicht zu Fehlern.

return self::getUTCDateTime( $localDateString );
}

$datetime->setTimezone( new \DateTimeZone( 'UTC' ) );

return $datetime;
}

public static function getLocalDateTime($timestamp): DateTime {
$dto = new DateTime();
$dto->setTimestamp(
Expand Down
11 changes: 6 additions & 5 deletions src/Service/Scheduler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace CommonsBooking\Service;

use CommonsBooking\Helper\Wordpress;
use CommonsBooking\Settings\Settings;
use CommonsBooking\View\TimeframeExport;

Expand Down Expand Up @@ -52,15 +53,15 @@ function __construct(
$this->unscheduleJob();
return false;
}

if (empty($executionTime)){
$this->timestamp = time();
datengraben marked this conversation as resolved.
Show resolved Hide resolved
}
}
elseif ($reccurence == 'daily'){
$this->timestamp = strtotime($executionTime);
if($this->timestamp < time()) { //if timestamp is in the past, add one day
$this->timestamp = strtotime("+1 day",$this->timestamp);
$dto = Wordpress::getUTCFromDate( $executionTime );
if( $dto->getTimestamp() < current_time( 'timestamp', true ) ) { //if timestamp is in the past, add one day
$dto->modify('+1 day');
}
$this->timestamp = $dto->getTimestamp();
}
else {
return false;
Expand Down
8 changes: 8 additions & 0 deletions tests/php/Helper/WordpressTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ public function testGetRelatedPostsIdsForRestriction()
$this->assertEquals( 5, count( $related ));
}

public function testGetUTCFromDate() {
update_option( 'timezone_string', 'Europe/Berlin' );
$local = '2023-11-27 17:43:56';
$gmt = '2023-11-27 16:43:56';
$dt = Wordpress::getUTCFromDate( $local );
$this->assertEquals( $gmt, $dt->format( 'Y-m-d H:i:s' ) );
}

protected function setUp(): void {
parent::setUp();
$this->timeframeId = $this->createBookableTimeFrameIncludingCurrentDay();
Expand Down
4 changes: 2 additions & 2 deletions tests/php/Service/SchedulerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ public function testReSchedule() {
$this->jobhooks[] = $dailyJob->getJobhook();

$now = new DateTime();
//time is saved in UTC format, will be fixed in #1429
$nextTime = DateTime::createFromFormat('H:i', '13:00', new \DateTimeZone('UTC'));
$nextTime = DateTime::createFromFormat('H:i', '13:00', wp_timezone() );
$nextTime->setTimezone(new \DateTimeZone('UTC'));

if ($nextTime < $now) {
$nextTime->modify('+1 day');
Expand Down