From 31743cf962f57a9f2e5a2acea7f7a48ffe50702f Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Tue, 29 Mar 2022 23:24:22 +0200 Subject: [PATCH] fix billable calculation on timesheet restart (#3225) --- src/Form/TimesheetEditForm.php | 33 +--- .../Calculator/BillableCalculator.php | 53 +++++++ .../TimesheetLongRunningValidator.php | 3 +- .../Calculator/BillableCalculatorTest.php | 145 ++++++++++++++++++ .../TimesheetLongRunningValidatorTest.php | 2 +- 5 files changed, 204 insertions(+), 32 deletions(-) create mode 100644 src/Timesheet/Calculator/BillableCalculator.php create mode 100644 tests/Timesheet/Calculator/BillableCalculatorTest.php diff --git a/src/Form/TimesheetEditForm.php b/src/Form/TimesheetEditForm.php index 659bdb7e..b4b1fbfa 100644 --- a/src/Form/TimesheetEditForm.php +++ b/src/Form/TimesheetEditForm.php @@ -21,6 +21,7 @@ use App\Form\Type\TimesheetBillableType; use App\Form\Type\UserType; use App\Form\Type\YesNoType; use App\Repository\CustomerRepository; +use App\Timesheet\Calculator\BillableCalculator; use Symfony\Component\Form\AbstractType; use Symfony\Component\Form\CallbackTransformer; use Symfony\Component\Form\FormBuilderInterface; @@ -279,36 +280,8 @@ class TimesheetEditForm extends AbstractType return $record; }, function (Timesheet $record) { - switch ($record->getBillableMode()) { - case Timesheet::BILLABLE_NO: - $record->setBillable(false); - break; - case Timesheet::BILLABLE_YES: - $record->setBillable(true); - break; - case Timesheet::BILLABLE_AUTOMATIC: - $billable = true; - - $activity = $record->getActivity(); - if ($activity !== null && !$activity->isBillable()) { - $billable = false; - } - - $project = $record->getProject(); - if ($billable && $project !== null && !$project->isBillable()) { - $billable = false; - } - - if ($billable && $project !== null) { - $customer = $project->getCustomer(); - if ($customer !== null && !$customer->isBillable()) { - $billable = false; - } - } - - $record->setBillable($billable); - break; - } + $billable = new BillableCalculator(); + $billable->calculate($record); return $record; } diff --git a/src/Timesheet/Calculator/BillableCalculator.php b/src/Timesheet/Calculator/BillableCalculator.php new file mode 100644 index 00000000..aae187a6 --- /dev/null +++ b/src/Timesheet/Calculator/BillableCalculator.php @@ -0,0 +1,53 @@ +getBillableMode()) { + case Timesheet::BILLABLE_NO: + $record->setBillable(false); + break; + case Timesheet::BILLABLE_YES: + $record->setBillable(true); + break; + case Timesheet::BILLABLE_AUTOMATIC: + $billable = true; + + $activity = $record->getActivity(); + if ($activity !== null && !$activity->isBillable()) { + $billable = false; + } + + $project = $record->getProject(); + if ($billable && $project !== null && !$project->isBillable()) { + $billable = false; + } + + if ($billable && $project !== null) { + $customer = $project->getCustomer(); + if ($customer !== null && !$customer->isBillable()) { + $billable = false; + } + } + + $record->setBillable($billable); + break; + } + } +} diff --git a/src/Validator/Constraints/TimesheetLongRunningValidator.php b/src/Validator/Constraints/TimesheetLongRunningValidator.php index 9f907734..d96b1fc2 100644 --- a/src/Validator/Constraints/TimesheetLongRunningValidator.php +++ b/src/Validator/Constraints/TimesheetLongRunningValidator.php @@ -61,7 +61,8 @@ final class TimesheetLongRunningValidator extends ConstraintValidator } $duration = $timesheet->getEnd()->getTimestamp() - $timesheet->getBegin()->getTimestamp(); - $minutes = (int) $duration / 60; + // float on purpose, because one second more than the configured minutes is already too long + $minutes = $duration / 60; if ($minutes < $maxMinutes) { return; diff --git a/tests/Timesheet/Calculator/BillableCalculatorTest.php b/tests/Timesheet/Calculator/BillableCalculatorTest.php new file mode 100644 index 00000000..fbe6083f --- /dev/null +++ b/tests/Timesheet/Calculator/BillableCalculatorTest.php @@ -0,0 +1,145 @@ +setCustomer($customer); + } + $timesheet->setProject($project); + } + if ($activity !== null) { + $timesheet->setActivity(clone $activity); + } + $timesheet->setBillable($billable); + $timesheet->setBillableMode($mode); + $sut->calculate($timesheet); + self::assertEquals($mode, $timesheet->getBillableMode()); + self::assertEquals($expected, $timesheet->isBillable()); + } + + public function getTestData() + { + $customerYes = new Customer(); + $customerYes->setBillable(true); + + $customerNo = new Customer(); + $customerNo->setBillable(false); + + $projectYes = new Project(); + $projectYes->setBillable(true); + + $projectNo = new Project(); + $projectNo->setBillable(false); + + $activityYes = new Activity(); + $activityYes->setBillable(true); + + $activityNo = new Activity(); + $activityNo->setBillable(false); + + return [ + 0 => [true, Timesheet::BILLABLE_DEFAULT, true, null, null, null], + 1 => [false, Timesheet::BILLABLE_DEFAULT, false, null, null, null], + 2 => [true, Timesheet::BILLABLE_NO, false, null, null, null], + 3 => [false, Timesheet::BILLABLE_NO, false, null, null, null], + 4 => [true, Timesheet::BILLABLE_YES, true, null, null, null], + 5 => [false, Timesheet::BILLABLE_YES, true, null, null, null], + 6 => [true, Timesheet::BILLABLE_AUTOMATIC, true, null, null, null], + 7 => [false, Timesheet::BILLABLE_AUTOMATIC, true, null, null, null], + 8 => [true, Timesheet::BILLABLE_DEFAULT, true, $customerYes, $projectYes, $activityYes], + 9 => [false, Timesheet::BILLABLE_DEFAULT, false, $customerYes, $projectYes, $activityYes], + 10 => [true, Timesheet::BILLABLE_NO, false, $customerYes, $projectYes, $activityYes], + 11 => [false, Timesheet::BILLABLE_NO, false, $customerYes, $projectYes, $activityYes], + 12 => [true, Timesheet::BILLABLE_YES, true, $customerYes, $projectYes, $activityYes], + 13 => [false, Timesheet::BILLABLE_YES, true, $customerYes, $projectYes, $activityYes], + 14 => [true, Timesheet::BILLABLE_AUTOMATIC, true, $customerYes, $projectYes, $activityYes], + 15 => [false, Timesheet::BILLABLE_AUTOMATIC, true, $customerYes, $projectYes, $activityYes], + 16 => [true, Timesheet::BILLABLE_DEFAULT, true, $customerNo, $projectNo, $activityNo], + 17 => [false, Timesheet::BILLABLE_DEFAULT, false, $customerNo, $projectNo, $activityNo], + 18 => [true, Timesheet::BILLABLE_NO, false, $customerNo, $projectNo, $activityNo], + 19 => [false, Timesheet::BILLABLE_NO, false, $customerNo, $projectNo, $activityNo], + 20 => [true, Timesheet::BILLABLE_YES, true, $customerNo, $projectNo, $activityNo], + 21 => [false, Timesheet::BILLABLE_YES, true, $customerNo, $projectNo, $activityNo], + 22 => [true, Timesheet::BILLABLE_AUTOMATIC, false, $customerNo, $projectNo, $activityNo], + 23 => [false, Timesheet::BILLABLE_AUTOMATIC, false, $customerNo, $projectNo, $activityNo], + 24 => [true, Timesheet::BILLABLE_DEFAULT, true, $customerNo, $projectYes, $activityNo], + 25 => [false, Timesheet::BILLABLE_DEFAULT, false, $customerNo, $projectYes, $activityNo], + 26 => [true, Timesheet::BILLABLE_NO, false, $customerNo, $projectYes, $activityNo], + 27 => [false, Timesheet::BILLABLE_NO, false, $customerNo, $projectYes, $activityNo], + 28 => [true, Timesheet::BILLABLE_YES, true, $customerNo, $projectYes, $activityNo], + 29 => [false, Timesheet::BILLABLE_YES, true, $customerNo, $projectYes, $activityNo], + 30 => [true, Timesheet::BILLABLE_AUTOMATIC, false, $customerNo, $projectYes, $activityNo], + 31 => [false, Timesheet::BILLABLE_AUTOMATIC, false, $customerNo, $projectYes, $activityNo], + 32 => [true, Timesheet::BILLABLE_DEFAULT, true, $customerNo, $projectNo, $activityYes], + 33 => [false, Timesheet::BILLABLE_DEFAULT, false, $customerNo, $projectNo, $activityYes], + 34 => [true, Timesheet::BILLABLE_NO, false, $customerNo, $projectNo, $activityYes], + 35 => [false, Timesheet::BILLABLE_NO, false, $customerNo, $projectNo, $activityYes], + 36 => [true, Timesheet::BILLABLE_YES, true, $customerNo, $projectNo, $activityYes], + 37 => [false, Timesheet::BILLABLE_YES, true, $customerNo, $projectNo, $activityYes], + 38 => [true, Timesheet::BILLABLE_AUTOMATIC, false, $customerNo, $projectNo, $activityYes], + 39 => [false, Timesheet::BILLABLE_AUTOMATIC, false, $customerNo, $projectNo, $activityYes], + 40 => [true, Timesheet::BILLABLE_DEFAULT, true, $customerYes, $projectNo, $activityNo], + 41 => [false, Timesheet::BILLABLE_DEFAULT, false, $customerYes, $projectNo, $activityNo], + 42 => [true, Timesheet::BILLABLE_NO, false, $customerYes, $projectNo, $activityNo], + 43 => [false, Timesheet::BILLABLE_NO, false, $customerYes, $projectNo, $activityNo], + 44 => [true, Timesheet::BILLABLE_YES, true, $customerYes, $projectNo, $activityNo], + 45 => [false, Timesheet::BILLABLE_YES, true, $customerYes, $projectNo, $activityNo], + 46 => [true, Timesheet::BILLABLE_AUTOMATIC, false, $customerYes, $projectNo, $activityNo], + 47 => [false, Timesheet::BILLABLE_AUTOMATIC, false, $customerYes, $projectNo, $activityNo], + 48 => [true, Timesheet::BILLABLE_DEFAULT, true, $customerYes, $projectNo, $activityYes], + 49 => [false, Timesheet::BILLABLE_DEFAULT, false, $customerYes, $projectNo, $activityYes], + 50 => [true, Timesheet::BILLABLE_NO, false, $customerYes, $projectNo, $activityYes], + 51 => [false, Timesheet::BILLABLE_NO, false, $customerYes, $projectNo, $activityYes], + 52 => [true, Timesheet::BILLABLE_YES, true, $customerYes, $projectNo, $activityYes], + 53 => [false, Timesheet::BILLABLE_YES, true, $customerYes, $projectNo, $activityYes], + 54 => [true, Timesheet::BILLABLE_AUTOMATIC, false, $customerYes, $projectNo, $activityYes], + 55 => [false, Timesheet::BILLABLE_AUTOMATIC, false, $customerYes, $projectNo, $activityYes], + 56 => [true, Timesheet::BILLABLE_DEFAULT, true, $customerYes, $projectYes, $activityNo], + 57 => [false, Timesheet::BILLABLE_DEFAULT, false, $customerYes, $projectYes, $activityNo], + 58 => [true, Timesheet::BILLABLE_NO, false, $customerYes, $projectYes, $activityNo], + 59 => [false, Timesheet::BILLABLE_NO, false, $customerYes, $projectYes, $activityNo], + 60 => [true, Timesheet::BILLABLE_YES, true, $customerYes, $projectYes, $activityNo], + 61 => [false, Timesheet::BILLABLE_YES, true, $customerYes, $projectYes, $activityNo], + 62 => [true, Timesheet::BILLABLE_AUTOMATIC, false, $customerYes, $projectYes, $activityNo], + 63 => [false, Timesheet::BILLABLE_AUTOMATIC, false, $customerYes, $projectYes, $activityNo], + 64 => [true, Timesheet::BILLABLE_DEFAULT, true, $customerNo, $projectYes, $activityYes], + 65 => [false, Timesheet::BILLABLE_DEFAULT, false, $customerNo, $projectYes, $activityYes], + 66 => [true, Timesheet::BILLABLE_NO, false, $customerNo, $projectYes, $activityYes], + 67 => [false, Timesheet::BILLABLE_NO, false, $customerNo, $projectYes, $activityYes], + 68 => [true, Timesheet::BILLABLE_YES, true, $customerNo, $projectYes, $activityYes], + 69 => [false, Timesheet::BILLABLE_YES, true, $customerNo, $projectYes, $activityYes], + 70 => [true, Timesheet::BILLABLE_AUTOMATIC, false, $customerNo, $projectYes, $activityYes], + 71 => [false, Timesheet::BILLABLE_AUTOMATIC, false, $customerNo, $projectYes, $activityYes], + ]; + } +} diff --git a/tests/Validator/Constraints/TimesheetLongRunningValidatorTest.php b/tests/Validator/Constraints/TimesheetLongRunningValidatorTest.php index b795c780..aea83ebd 100644 --- a/tests/Validator/Constraints/TimesheetLongRunningValidatorTest.php +++ b/tests/Validator/Constraints/TimesheetLongRunningValidatorTest.php @@ -79,7 +79,7 @@ class TimesheetLongRunningValidatorTest extends ConstraintValidatorTestCase $timesheet = new Timesheet(); $timesheet->setBegin(new \DateTime()); $timesheet->setEnd(new \DateTime()); - $timesheet->setDuration(31536001); + $timesheet->setDuration(31536060); $this->validator->validate($timesheet, new TimesheetLongRunning());