search

kimai / www.kimai.org

Website for the Kimai time-tracker
https://www.kimai.org
GNU Affero General Public License v3.0
18 stars 87 forks source link

[Docs] Lockdown period documentation seems not to align with implementation code #376

Open sharky98 opened 8 months ago

sharky98 commented 8 months ago

When looking through the docs for the translation, I was wondering how the lockdown period worked. The docs seems to not be aligned with how src/Timesheet/LockdownService.php is working in few points.

Relative to now

It seems that the lockdown period are not checked against now for neither start or end, but are used (almost) as-is to create a new DateTimeImmutable.

While I understand that PHP will check the relative string to now with string like -2 weeks, it won't do it if provided a more "fixed" relative string such as last day of next month[^1].

[^1]: OK, it still uses now to be relative to. However this is not within an arithmetic calculation. And the user could provide a fixed month and year such as last sat of July 2008 (but why?!).

I would suggest removing reference to relative to now in the docs (and translations to start, end) to avoid confusion.

Grace Relative to lockdown end period

Here, the docs seems to tell us that the relative date string will be used against the end of the lockdown period, but could be modified with the of modifier. However this seems partly true when looking at the code. It seems that the grace period is used to modify the lockdown end period. So, if I understand the code correctly, if the grace period is +2 weeks, it works as expected (2 weeks after the lockdown end period). However, if the grace period is last day of next month and this is applied on the lockdown end period, this would mean that the grace period would be extended more than expected[^2].

[^2]: Like if the lockdown is set to last day of next month and grace to last day of next month, relative to today (2024-01-26), the lockdown will end on 2024-02-29 but the grace period will be set to 2024-03-31, while the docs seems to tell us that using of will bring it to relative to today.

Activating the lockdown

The docs tell us the lockdown will be active if all three fields are populated. But the code seems to check only for start and end, not grace.

sharky98 commented 8 months ago

I have created different pull request to accept to your liking (or not). And also provided source string suggestion to the translation.