search

pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
306 stars 445 forks source link

[OJS] Dates are not good (timezone mistake) in ReviewAssignmentEmailVariable (I think...) #10579

Open forgive38 opened 1 week ago

forgive38 commented 1 week ago

Describe the bug I use "Europe/Paris" as timezone in config.inc.php

When a reviewer receive a review remind, the dates due are one day before (exemple : 15.10.2024 for 16.10.2024 in the Web UI)

I think it's happening here: https://github.com/pkp/pkp-lib/blob/43511e76869976eecb34af042d1cf40cfefc828e/classes/mail/variables/ReviewAssignmentEmailVariable.php#L83

in fact, Carbon converts a date string (without tz) and uses the application's timezone (positioned here: (https://github.com/pkp/pkp-lib/blob/43511e76869976eecb34af042d1cf40cfefc828e/classes/core/PKPApplication.php#L273) ) to convert the date into utc So in my case, with 'Europe/Paris' if $dateDue = '2024-11-12 00:00:00' Carbon converts this string as '2024-11-11 22:00:00' with the timezone UTC (+0) which is correct. BUT when translatedFormat('d.m.Y') is used, 11.11.2024 is displayed instead of 12.11.2024. So you need to tell Carbon to manage the date with the application's timezone this is done with Carbon->setTimezone(date_default_timezone_get())

PR is coming

It's probably related to: #10264

(maybe use of UTC everywhere in the application is better, and use timezone only for user interactions)

What application are you using? OJS 3.4 / main

asmecher commented 1 week ago

@forgive38, we should be consistent about where this is applied; I suspect everywhere else that Carbon::translatedFormat is called, this should also be added. Would you mind expanding the scope of this issue a little bit?

forgive38 commented 5 days ago

oh I was wrong the problem isn't exactly that. Carbon does parse a date taking into account the application's timezone, but not if it's a timestamp since Carbon version 3. From Carbon doc: You can create instances from [unix timestamps](https://en.wikipedia.org/wiki/Unix_time). createFromTimestamp() create a Carbon instance equal to the given timestamp and will set the timezone to the given timezone as second parameter, or to UTC if non given (since Carbon 3) (in previous versions it defaulted to date_default_timezone_get())

and I looked at the code and didn't see the problem elsewhere I'll rewrite the PR. HTH