From 0eb5c0a08ad31901d9e907c39e178c9598b8f422 Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Mon, 29 Aug 2022 01:03:44 +0200 Subject: [PATCH] fixed multiple budget validation checks (#3489) * fix budget check for monthly budgets for records with a changed month * fix API update of begin and end * fix update of project and activity --- .../TimesheetBudgetUsedValidator.php | 42 +++++++++++++------ .../TimesheetBudgetUsedValidatorTest.php | 1 + 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/src/Validator/Constraints/TimesheetBudgetUsedValidator.php b/src/Validator/Constraints/TimesheetBudgetUsedValidator.php index f94bec0b..d47c73e9 100644 --- a/src/Validator/Constraints/TimesheetBudgetUsedValidator.php +++ b/src/Validator/Constraints/TimesheetBudgetUsedValidator.php @@ -74,16 +74,16 @@ final class TimesheetBudgetUsedValidator extends ConstraintValidator return; } - $duration = $timesheet->getDuration(); - if (null === $duration || 0 === $duration) { - $duration = $timesheet->getEnd()->getTimestamp() - $timesheet->getBegin()->getTimestamp(); - } - // this validator needs a project to calculate the rates if ($timesheet->getProject() === null) { return; } + // when changing the date via the calendar and/or the API, the duration will not be reset by the + // duration calculator (which runs after validation!) so we manually reset the duration before + $timesheet->setDuration(null); + $duration = $timesheet->getDuration(); + $timeRate = $this->rateService->calculate($timesheet); $rate = $timeRate->getRate(); @@ -93,21 +93,28 @@ final class TimesheetBudgetUsedValidator extends ConstraintValidator $projectRate = $rate; $customerDuration = $duration; $customerRate = $rate; + $monthWasChanged = false; if ($timesheet->getId() !== null) { $rawData = $this->timesheetRepository->getRawData($timesheet); - // if an existing entry was updated, but "duration", "rate" and "billable" were not changed: - // do not validate! this could for example happen when export flag is changed OR if "prevent overbooking" - // config was recently activated and this is an old entry - if ($duration === $rawData['duration'] && $rate === $rawData['rate'] && $timesheet->isBillable() === $rawData['billable'] && $timesheet->getBegin()->format('Y.m.d') === $rawData['begin']->format('Y.m.d')) { + $activityId = (int) $rawData['activity']; + $projectId = (int) $rawData['project']; + $customerId = (int) $rawData['customer']; + + // if an existing entry was updated, but the relevant fields for budget calculation were not touched: do not validate! + // this could for example happen when export flag is changed OR if "prevent overbooking" config was recently activated and this is an old entry + if ($duration === $rawData['duration'] && + $rate === $rawData['rate'] && + $timesheet->isBillable() === $rawData['billable'] && + $timesheet->getBegin()->format('Y.m.d') === $rawData['begin']->format('Y.m.d') && + $timesheet->getProject()->getId() === $projectId && + ($timesheet->getActivity() === null || $timesheet->getActivity()->getId() === $activityId) + ) { return; } // the duration of an existing entry could be increased or lowered - $activityId = (int) $rawData['activity']; - $projectId = (int) $rawData['project']; - $customerId = (int) $rawData['customer']; // only subtract the previously logged data in case the record was billable // if it wasn't billable, then its values are not included in the statistic models used later on @@ -129,6 +136,8 @@ final class TimesheetBudgetUsedValidator extends ConstraintValidator } } } + + $monthWasChanged = $timesheet->getBegin()->format('Y.m') !== $rawData['begin']->format('Y.m'); } $now = new DateTime('now', $timesheet->getBegin()->getTimezone()); @@ -136,6 +145,9 @@ final class TimesheetBudgetUsedValidator extends ConstraintValidator if (null !== ($activity = $timesheet->getActivity()) && $activity->hasBudgets()) { $dateTime = $activity->isMonthlyBudget() ? $recordDate : $now; + if ($activity->isMonthlyBudget() && $monthWasChanged) { + $activityDuration = $duration; + } $stat = $this->activityStatisticService->getBudgetStatisticModel($activity, $dateTime); $this->checkBudgets($constraint, $stat, $timesheet, $activityDuration, $activityRate, 'activity'); } @@ -143,11 +155,17 @@ final class TimesheetBudgetUsedValidator extends ConstraintValidator if (null !== ($project = $timesheet->getProject())) { if ($project->hasBudgets()) { $dateTime = $project->isMonthlyBudget() ? $recordDate : $now; + if ($project->isMonthlyBudget() && $monthWasChanged) { + $projectDuration = $duration; + } $stat = $this->projectStatisticService->getBudgetStatisticModel($project, $dateTime); $this->checkBudgets($constraint, $stat, $timesheet, $projectDuration, $projectRate, 'project'); } if (null !== ($customer = $project->getCustomer()) && $customer->hasBudgets()) { $dateTime = $customer->isMonthlyBudget() ? $recordDate : $now; + if ($customer->isMonthlyBudget() && $monthWasChanged) { + $customerDuration = $duration; + } $stat = $this->customerStatisticService->getBudgetStatisticModel($customer, $dateTime); $this->checkBudgets($constraint, $stat, $timesheet, $customerDuration, $customerRate, 'customer'); } diff --git a/tests/Validator/Constraints/TimesheetBudgetUsedValidatorTest.php b/tests/Validator/Constraints/TimesheetBudgetUsedValidatorTest.php index 2d50250b..d03512c7 100644 --- a/tests/Validator/Constraints/TimesheetBudgetUsedValidatorTest.php +++ b/tests/Validator/Constraints/TimesheetBudgetUsedValidatorTest.php @@ -411,6 +411,7 @@ class TimesheetBudgetUsedValidatorTest extends ConstraintValidatorTestCase $timesheet->method('getRate')->willReturn($rawData['rate']); $timesheet->method('getBegin')->willReturn($begin); $timesheet->method('getEnd')->willReturn($end); + $timesheet->method('getDuration')->willReturn($end->getTimestamp() - $begin->getTimestamp()); $timesheet->method('getUser')->willReturn(new User()); $timesheet->method('getProject')->willReturn($project); $timesheet->method('getActivity')->willReturn($activity);