Skip to content

Fix #36480 timesheet date comparison accepts string and timestamp#38921

Open
pixodeo wants to merge 1 commit into
Dolibarr:18.0from
Dolicraft:fix/36480-timesheet-date-string-compare
Open

Fix #36480 timesheet date comparison accepts string and timestamp#38921
pixodeo wants to merge 1 commit into
Dolibarr:18.0from
Dolicraft:fix/36480-timesheet-date-string-compare

Conversation

@pixodeo

@pixodeo pixodeo commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Fixes #36480.

addTimeSpent, updateTimeSpent and delTimeSpent compare $this->timespent_date (which is an int timestamp in the common path but can be a 'YYYY-MM-DD' string when set by some callers) directly with the integer timestamp returned by dol_time_plus_duree, so the PROJECT_TIMESHEET_PREVENT_AFTER_MONTHS guard silently allowed past entries it should refuse. Normalize the left-hand side with is_numeric / strtotime at the three comparison sites.

…tamp

addTimeSpent, updateTimeSpent and delTimeSpent compared $this->timespent_date
(which can be a 'YYYY-MM-DD' string when called from some callers) directly
with the integer timestamp returned by dol_time_plus_duree, so the
PROJECT_TIMESHEET_PREVENT_AFTER_MONTHS restriction silently allowed past
entries it should refuse. Normalize the left-hand side with a numeric/strtotime
guard at the three comparison sites.

Signed-off-by: Dolicraft <contact@dolicraft.com>
@the-dolibear-bot the-dolibear-bot Bot added the Issue for v18 maintenance Team PR is in a maintenance branch with several approvers. Waiting approval of all of them. label Jun 20, 2026
@rycks

rycks commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

mmmm such code is hard to read, so maybe you could have something more easy to read like that

$timetocompare = dol_stringtotime($this->timespent_date);
if ($timetocompare < $restrictBefore) {

that's just an idea ... and it seems to be what @atm-jonathan do in #36480 ... i'm in trouble ... so maybe we could wait for #36480 merged into develop and make a cherry-pick ?

@pixodeo

pixodeo commented Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Agreed, closing in favour of #36480 which fixes the same timesheet date comparison in task.class.php. No need for two parallel patches. @atm-jonathan's PR is the reference; once it lands on develop it will forward-merge down. Thanks @rycks for the pointer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Issue for v18 maintenance Team PR is in a maintenance branch with several approvers. Waiting approval of all of them.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants