From 49e1a1c41061a3a44469795b75de2a03f0be4ad5 Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Wed, 4 Sep 2019 10:54:21 +0200 Subject: [PATCH] use timesheet description in invoices (#1079) --- .../Calculator/AbstractMergedCalculator.php | 11 ++++---- .../AbstractSumInvoiceCalculator.php | 6 ++++ .../Calculator/ActivityInvoiceCalculator.php | 7 +++++ .../Calculator/ProjectInvoiceCalculator.php | 7 +++++ .../Calculator/UserInvoiceCalculator.php | 28 ++++--------------- templates/invoice/renderer/default.html.twig | 4 +-- .../invoice/renderer/freelancer.html.twig | 2 +- .../invoice/renderer/timesheet.html.twig | 4 +-- .../Calculator/AbstractCalculatorTest.php | 19 +++++-------- .../ActivityInvoiceCalculatorTest.php | 10 ------- .../Calculator/DateInvoiceCalculatorTest.php | 10 ------- .../Calculator/DefaultCalculatorTest.php | 3 ++ .../ProjectInvoiceCalculatorTest.php | 10 ------- .../Calculator/ShortInvoiceCalculatorTest.php | 10 ------- .../Calculator/UserInvoiceCalculatorTest.php | 15 +++------- 15 files changed, 51 insertions(+), 95 deletions(-) diff --git a/src/Invoice/Calculator/AbstractMergedCalculator.php b/src/Invoice/Calculator/AbstractMergedCalculator.php index d66c0478..44513833 100644 --- a/src/Invoice/Calculator/AbstractMergedCalculator.php +++ b/src/Invoice/Calculator/AbstractMergedCalculator.php @@ -47,11 +47,12 @@ abstract class AbstractMergedCalculator extends AbstractCalculator $invoiceItem->setEnd($entry->getEnd()); } - if (null !== $this->model->getQuery()->getActivity()) { - $invoiceItem->setActivity($this->model->getQuery()->getActivity()); - $invoiceItem->setDescription($this->model->getQuery()->getActivity()->getName()); - } elseif (null !== $this->model->getQuery()->getProject()) { - $invoiceItem->setDescription($this->model->getQuery()->getProject()->getName()); + if (!empty($entry->getDescription())) { + $description = ''; + if (!empty($invoiceItem->getDescription())) { + $description = $invoiceItem->getDescription() . PHP_EOL; + } + $invoiceItem->setDescription($description . $entry->getDescription()); } if (null === $invoiceItem->getActivity()) { diff --git a/src/Invoice/Calculator/AbstractSumInvoiceCalculator.php b/src/Invoice/Calculator/AbstractSumInvoiceCalculator.php index f1d3153c..de16c037 100644 --- a/src/Invoice/Calculator/AbstractSumInvoiceCalculator.php +++ b/src/Invoice/Calculator/AbstractSumInvoiceCalculator.php @@ -47,8 +47,14 @@ abstract class AbstractSumInvoiceCalculator extends AbstractMergedCalculator imp } $timesheet = $invoiceItems[$id]; $this->mergeTimesheets($timesheet, $entry); + $this->mergeSumTimesheet($timesheet, $entry); } return array_values($invoiceItems); } + + protected function mergeSumTimesheet(InvoiceItem $invoiceItem, Timesheet $entry) + { + // allows to set values per calculator after merging the timesheet + } } diff --git a/src/Invoice/Calculator/ActivityInvoiceCalculator.php b/src/Invoice/Calculator/ActivityInvoiceCalculator.php index e6a9beea..4702f025 100644 --- a/src/Invoice/Calculator/ActivityInvoiceCalculator.php +++ b/src/Invoice/Calculator/ActivityInvoiceCalculator.php @@ -11,6 +11,7 @@ namespace App\Invoice\Calculator; use App\Entity\Timesheet; use App\Invoice\CalculatorInterface; +use App\Invoice\InvoiceItem; /** * A calculator that sums up the timesheet records by activity. @@ -26,6 +27,12 @@ class ActivityInvoiceCalculator extends AbstractSumInvoiceCalculator implements return (string) $timesheet->getActivity()->getId(); } + protected function mergeSumTimesheet(InvoiceItem $invoiceItem, Timesheet $entry) + { + $invoiceItem->setActivity($entry->getActivity()); + $invoiceItem->setDescription($entry->getActivity()->getName()); + } + /** * @return string */ diff --git a/src/Invoice/Calculator/ProjectInvoiceCalculator.php b/src/Invoice/Calculator/ProjectInvoiceCalculator.php index 87fd875a..05106229 100644 --- a/src/Invoice/Calculator/ProjectInvoiceCalculator.php +++ b/src/Invoice/Calculator/ProjectInvoiceCalculator.php @@ -11,6 +11,7 @@ namespace App\Invoice\Calculator; use App\Entity\Timesheet; use App\Invoice\CalculatorInterface; +use App\Invoice\InvoiceItem; /** * A calculator that sums up the timesheet records by project. @@ -26,6 +27,12 @@ class ProjectInvoiceCalculator extends AbstractSumInvoiceCalculator implements C return (string) $timesheet->getProject()->getId(); } + protected function mergeSumTimesheet(InvoiceItem $invoiceItem, Timesheet $entry) + { + $invoiceItem->setProject($entry->getProject()); + $invoiceItem->setDescription($entry->getProject()->getName()); + } + /** * @return string */ diff --git a/src/Invoice/Calculator/UserInvoiceCalculator.php b/src/Invoice/Calculator/UserInvoiceCalculator.php index e67256de..26c8d173 100644 --- a/src/Invoice/Calculator/UserInvoiceCalculator.php +++ b/src/Invoice/Calculator/UserInvoiceCalculator.php @@ -9,37 +9,21 @@ namespace App\Invoice\Calculator; +use App\Entity\Timesheet; use App\Invoice\CalculatorInterface; -use App\Invoice\InvoiceItem; /** * A calculator that sums up the timesheet records by user. */ -class UserInvoiceCalculator extends AbstractMergedCalculator implements CalculatorInterface +class UserInvoiceCalculator extends AbstractSumInvoiceCalculator implements CalculatorInterface { - /** - * @return InvoiceItem[] - */ - public function getEntries() + protected function calculateSumIdentifier(Timesheet $timesheet): string { - $entries = $this->model->getEntries(); - if (empty($entries)) { - return []; + if (null === $timesheet->getUser()->getId()) { + throw new \Exception('Cannot handle un-persisted user'); } - /** @var InvoiceItem[] $invoiceItems */ - $invoiceItems = []; - - foreach ($entries as $entry) { - $id = $entry->getUser()->getId(); - if (!isset($invoiceItems[$id])) { - $invoiceItems[$id] = new InvoiceItem(); - } - $invoiceItem = $invoiceItems[$id]; - $this->mergeTimesheets($invoiceItem, $entry); - } - - return array_values($invoiceItems); + return (string) $timesheet->getUser()->getId(); } /** diff --git a/templates/invoice/renderer/default.html.twig b/templates/invoice/renderer/default.html.twig index 12983995..9c625567 100644 --- a/templates/invoice/renderer/default.html.twig +++ b/templates/invoice/renderer/default.html.twig @@ -75,9 +75,9 @@ {% endif %} {{ entry.begin|date_short }} - + {% if entry.description is not empty %} - {{ entry.description }} + {{ entry.description|nl2br }} {% else %} {{ entry.activity.name }} / {{ entry.project.name }} {% endif %} diff --git a/templates/invoice/renderer/freelancer.html.twig b/templates/invoice/renderer/freelancer.html.twig index e74c5d70..30d866c2 100644 --- a/templates/invoice/renderer/freelancer.html.twig +++ b/templates/invoice/renderer/freelancer.html.twig @@ -90,7 +90,7 @@ {% if entry.description is not empty %} - {{ entry.description }} + {{ entry.description|nl2br }} {% else %} {{ entry.activity.name }} / {{ entry.project.name }} {% endif %} diff --git a/templates/invoice/renderer/timesheet.html.twig b/templates/invoice/renderer/timesheet.html.twig index f2364dc9..f8842d22 100644 --- a/templates/invoice/renderer/timesheet.html.twig +++ b/templates/invoice/renderer/timesheet.html.twig @@ -72,9 +72,9 @@ {% if model.query.user is empty %} {{ widgets.username(entry.user) }} {% endif %} - + {% if entry.description is not empty %} - {{ entry.description }} + {{ entry.description|nl2br }} {% else %} {{ entry.activity.name }} / {{ entry.project.name }} {% endif %} diff --git a/tests/Invoice/Calculator/AbstractCalculatorTest.php b/tests/Invoice/Calculator/AbstractCalculatorTest.php index 8d44da0f..c2949e73 100644 --- a/tests/Invoice/Calculator/AbstractCalculatorTest.php +++ b/tests/Invoice/Calculator/AbstractCalculatorTest.php @@ -50,24 +50,19 @@ abstract class AbstractCalculatorTest extends TestCase $template = new InvoiceTemplate(); $template->setVat(19); + $user = $this->getMockBuilder(User::class)->setMethods(['getId'])->disableOriginalConstructor()->getMock(); + $user->method('getId')->willReturn(1); + $project = $this->getMockBuilder(Project::class)->setMethods(['getId', 'getCustomer', 'getName'])->disableOriginalConstructor()->getMock(); $project->method('getId')->willReturn(1); $project->method('getCustomer')->willReturn($customer); $project->method('getName')->willReturn('project description'); - $project1 = $this->getMockBuilder(Project::class)->setMethods(['getId', 'getName'])->disableOriginalConstructor()->getMock(); - $project1->method('getId')->willReturn(1); - $project1->method('getName')->willReturn('bar'); - $activity = $this->getMockBuilder(Activity::class)->setMethods(['getId', 'getProject', 'getName'])->disableOriginalConstructor()->getMock(); $activity->method('getId')->willReturn(1); $activity->method('getProject')->willReturn($project); $activity->method('getName')->willReturn('activity description'); - $activity1 = $this->getMockBuilder(Activity::class)->setMethods(['getId', 'getName'])->disableOriginalConstructor()->getMock(); - $activity1->method('getId')->willReturn(1); - $activity1->method('getName')->willReturn('foo'); - $query = new InvoiceQuery(); if ($addProject === true) { $query->setProject($project); @@ -81,9 +76,9 @@ abstract class AbstractCalculatorTest extends TestCase ->setBegin(new \DateTime()) ->setDuration(3600) ->setRate(293.27) - ->setUser(new User()) - ->setActivity($activity1) - ->setProject($project1); + ->setUser($user) + ->setActivity($activity) + ->setProject($project); $model = new InvoiceModel(); $model->setCustomer($customer); @@ -101,7 +96,7 @@ abstract class AbstractCalculatorTest extends TestCase } elseif ($addActivity === true) { $this->assertEquals('activity description', $result->getDescription()); } else { - $this->assertEquals('foo', $result->getDescription()); + $this->assertEquals('timesheet description', $result->getDescription()); } } } diff --git a/tests/Invoice/Calculator/ActivityInvoiceCalculatorTest.php b/tests/Invoice/Calculator/ActivityInvoiceCalculatorTest.php index 13d7da59..2198e5b8 100644 --- a/tests/Invoice/Calculator/ActivityInvoiceCalculatorTest.php +++ b/tests/Invoice/Calculator/ActivityInvoiceCalculatorTest.php @@ -128,18 +128,8 @@ class ActivityInvoiceCalculatorTest extends AbstractCalculatorTest $this->assertEquals(84, $entries[2]->getRate()); } - public function testDescriptionByTimesheet() - { - $this->assertDescription(new ActivityInvoiceCalculator(), false, false); - } - public function testDescriptionByActivity() { $this->assertDescription(new ActivityInvoiceCalculator(), false, true); } - - public function testDescriptionByProject() - { - $this->assertDescription(new ActivityInvoiceCalculator(), true, false); - } } diff --git a/tests/Invoice/Calculator/DateInvoiceCalculatorTest.php b/tests/Invoice/Calculator/DateInvoiceCalculatorTest.php index 2116acdf..a2a3f0fb 100644 --- a/tests/Invoice/Calculator/DateInvoiceCalculatorTest.php +++ b/tests/Invoice/Calculator/DateInvoiceCalculatorTest.php @@ -135,14 +135,4 @@ class DateInvoiceCalculatorTest extends AbstractCalculatorTest { $this->assertDescription(new DateInvoiceCalculator(), false, false); } - - public function testDescriptionByActivity() - { - $this->assertDescription(new DateInvoiceCalculator(), false, true); - } - - public function testDescriptionByProject() - { - $this->assertDescription(new DateInvoiceCalculator(), true, false); - } } diff --git a/tests/Invoice/Calculator/DefaultCalculatorTest.php b/tests/Invoice/Calculator/DefaultCalculatorTest.php index 7f20c3ca..fda1ffe3 100644 --- a/tests/Invoice/Calculator/DefaultCalculatorTest.php +++ b/tests/Invoice/Calculator/DefaultCalculatorTest.php @@ -35,18 +35,21 @@ class DefaultCalculatorTest extends AbstractCalculatorTest $template->setVat(19); $timesheet = new Timesheet(); + $timesheet->setDescription('foo 1'); $timesheet->setBegin(new \DateTime()); $timesheet->setDuration(3600); $timesheet->setRate(293.27); $timesheet->setActivity(new Activity()); $timesheet2 = new Timesheet(); + $timesheet2->setDescription('foo 2'); $timesheet2->setBegin(new \DateTime()); $timesheet2->setDuration(400); $timesheet2->setRate(84); $timesheet2->setActivity(new Activity()); $timesheet3 = new Timesheet(); + $timesheet3->setDescription('foo 3'); $timesheet3->setBegin(new \DateTime()); $timesheet3->setDuration(1800); $timesheet3->setRate(111.11); diff --git a/tests/Invoice/Calculator/ProjectInvoiceCalculatorTest.php b/tests/Invoice/Calculator/ProjectInvoiceCalculatorTest.php index efc40e48..385a7206 100644 --- a/tests/Invoice/Calculator/ProjectInvoiceCalculatorTest.php +++ b/tests/Invoice/Calculator/ProjectInvoiceCalculatorTest.php @@ -131,16 +131,6 @@ class ProjectInvoiceCalculatorTest extends AbstractCalculatorTest self::assertEquals(2521.12, $entries[0]->getRate() + $entries[1]->getRate() + $entries[2]->getRate()); } - public function testDescriptionByTimesheet() - { - $this->assertDescription(new ProjectInvoiceCalculator(), false, false); - } - - public function testDescriptionByActivity() - { - $this->assertDescription(new ProjectInvoiceCalculator(), false, true); - } - public function testDescriptionByProject() { $this->assertDescription(new ProjectInvoiceCalculator(), true, false); diff --git a/tests/Invoice/Calculator/ShortInvoiceCalculatorTest.php b/tests/Invoice/Calculator/ShortInvoiceCalculatorTest.php index 1f784282..8bb8b88d 100644 --- a/tests/Invoice/Calculator/ShortInvoiceCalculatorTest.php +++ b/tests/Invoice/Calculator/ShortInvoiceCalculatorTest.php @@ -190,14 +190,4 @@ class ShortInvoiceCalculatorTest extends AbstractCalculatorTest { $this->assertDescription(new ShortInvoiceCalculator(), false, false); } - - public function testDescriptionByActivity() - { - $this->assertDescription(new ShortInvoiceCalculator(), false, true); - } - - public function testDescriptionByProject() - { - $this->assertDescription(new ShortInvoiceCalculator(), true, false); - } } diff --git a/tests/Invoice/Calculator/UserInvoiceCalculatorTest.php b/tests/Invoice/Calculator/UserInvoiceCalculatorTest.php index 1b699140..60fd8509 100644 --- a/tests/Invoice/Calculator/UserInvoiceCalculatorTest.php +++ b/tests/Invoice/Calculator/UserInvoiceCalculatorTest.php @@ -46,6 +46,9 @@ class UserInvoiceCalculatorTest extends AbstractCalculatorTest $user2 = $this->getMockBuilder(User::class)->setMethods(['getId'])->disableOriginalConstructor()->getMock(); $user2->method('getId')->willReturn(2); + $user3 = $this->getMockBuilder(User::class)->setMethods(['getId'])->disableOriginalConstructor()->getMock(); + $user3->method('getId')->willReturn(3); + $timesheet = new Timesheet(); $timesheet ->setBegin(new \DateTime()) @@ -92,7 +95,7 @@ class UserInvoiceCalculatorTest extends AbstractCalculatorTest ->setEnd(new \DateTime()) ->setDuration(400) ->setRate(84) - ->setUser(new User()) + ->setUser($user3) ->setActivity($activity) ->setProject((new Project())->setName('bar')); @@ -128,14 +131,4 @@ class UserInvoiceCalculatorTest extends AbstractCalculatorTest { $this->assertDescription(new UserInvoiceCalculator(), false, false); } - - public function testDescriptionByActivity() - { - $this->assertDescription(new UserInvoiceCalculator(), false, true); - } - - public function testDescriptionByProject() - { - $this->assertDescription(new UserInvoiceCalculator(), true, false); - } }