From 7a3388646b46fe9758b905f390e8fbe67287863a Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Thu, 10 Dec 2020 15:00:14 +0100 Subject: [PATCH] allow custom export repositories (#2182) --- src/Controller/ExportController.php | 33 +++-------- .../Compiler/ExportServiceCompilerPass.php | 5 ++ src/Export/Base/RendererTrait.php | 31 ++++++++++ src/Export/ExportRepositoryInterface.php | 38 +++++++++++++ src/Export/ServiceExport.php | 32 ++++++++++- src/Export/TimesheetExportRepository.php | 57 +++++++++++++++++++ src/Form/Toolbar/ExportToolbarForm.php | 11 ++-- src/Kernel.php | 3 + src/Repository/Query/ExportQuery.php | 10 ++-- .../TimesheetInvoiceItemRepository.php | 14 +++-- src/Repository/TimesheetRepository.php | 24 ++++---- templates/export/index.html.twig | 8 ++- templates/export/pdf-layout.html.twig | 23 ++++---- templates/export/renderer/default.html.twig | 2 +- tests/Controller/ExportControllerTest.php | 4 +- .../Export/TimesheetExportRepositoryTest.php | 47 +++++++++++++++ tests/Repository/Query/ExportQueryTest.php | 10 ++-- .../TimesheetInvoiceItemRepositoryTest.php | 40 +++++++++++++ 18 files changed, 320 insertions(+), 72 deletions(-) create mode 100644 src/Export/ExportRepositoryInterface.php create mode 100644 src/Export/TimesheetExportRepository.php create mode 100644 tests/Export/TimesheetExportRepositoryTest.php create mode 100644 tests/Repository/TimesheetInvoiceItemRepositoryTest.php diff --git a/src/Controller/ExportController.php b/src/Controller/ExportController.php index a2f11d9b..bffeaef0 100644 --- a/src/Controller/ExportController.php +++ b/src/Controller/ExportController.php @@ -13,8 +13,6 @@ use App\Entity\Timesheet; use App\Export\ServiceExport; use App\Form\Toolbar\ExportToolbarForm; use App\Repository\Query\ExportQuery; -use App\Repository\TimesheetRepository; -use App\Timesheet\UserDateTimeFactory; use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; use Symfony\Component\Form\FormInterface; use Symfony\Component\Form\SubmitButton; @@ -30,29 +28,14 @@ use Symfony\Component\Routing\Annotation\Route; */ class ExportController extends AbstractController { - /** - * @var TimesheetRepository - */ - protected $timesheetRepository; - /** * @var ServiceExport */ - protected $export; - /** - * @var UserDateTimeFactory - */ - protected $dateFactory; + private $export; - /** - * @param TimesheetRepository $timesheet - * @param ServiceExport $export - */ - public function __construct(TimesheetRepository $timesheet, ServiceExport $export, UserDateTimeFactory $dateTime) + public function __construct(ServiceExport $export) { - $this->timesheetRepository = $timesheet; $this->export = $export; - $this->dateFactory = $dateTime; } /** @@ -100,7 +83,7 @@ class ExportController extends AbstractController $form = $this->getToolbarForm($query, 'POST'); $form->handleRequest($request); - $type = $query->getType(); + $type = $query->getRenderer(); if (null === $type) { throw $this->createNotFoundException('Missing export renderer'); } @@ -114,8 +97,9 @@ class ExportController extends AbstractController $entries = $this->getEntries($query); $response = $renderer->render($entries, $query); + // TODO check entries if user is allowed to update export state - see https://github.com/kevinpapst/kimai2/issues/1473 if ($query->isMarkAsExported()) { - $this->timesheetRepository->setExported($entries); + $this->export->setExported($entries); } return $response; @@ -123,8 +107,8 @@ class ExportController extends AbstractController protected function getDefaultQuery(): ExportQuery { - $begin = $this->dateFactory->createDateTime('first day of this month 00:00:00'); - $end = $this->dateFactory->createDateTime('last day of this month 23:59:59'); + $begin = $this->getDateTimeFactory()->getStartOfMonth(); + $end = $this->getDateTimeFactory()->getEndOfMonth(); $query = new ExportQuery(); $query->setOrder(ExportQuery::ORDER_ASC); @@ -150,13 +134,14 @@ class ExportController extends AbstractController $query->getEnd()->setTime(23, 59, 59); } - return $this->timesheetRepository->getTimesheetsForQuery($query, true); + return $this->export->getExportItems($query); } protected function getToolbarForm(ExportQuery $query, string $method): FormInterface { return $this->createForm(ExportToolbarForm::class, $query, [ 'action' => $this->generateUrl('export', []), + 'include_user' => $this->isGranted('view_other_timesheet'), 'method' => $method, 'attr' => [ 'id' => 'export-form' diff --git a/src/DependencyInjection/Compiler/ExportServiceCompilerPass.php b/src/DependencyInjection/Compiler/ExportServiceCompilerPass.php index dfef97df..55e25aa3 100644 --- a/src/DependencyInjection/Compiler/ExportServiceCompilerPass.php +++ b/src/DependencyInjection/Compiler/ExportServiceCompilerPass.php @@ -42,6 +42,11 @@ class ExportServiceCompilerPass implements CompilerPassInterface $definition->addMethodCall('addTimesheetExporter', [new Reference($id)]); } + $taggedRepository = $container->findTaggedServiceIds(Kernel::TAG_EXPORT_REPOSITORY); + foreach ($taggedRepository as $id => $tags) { + $definition->addMethodCall('addExportRepository', [new Reference($id)]); + } + $path = \dirname(\dirname(\dirname(__DIR__))) . DIRECTORY_SEPARATOR; foreach ($container->getParameter('kimai.export.documents') as $exportPath) { if (!is_dir($path . $exportPath)) { diff --git a/src/Export/Base/RendererTrait.php b/src/Export/Base/RendererTrait.php index 3128b7d7..794344f1 100644 --- a/src/Export/Base/RendererTrait.php +++ b/src/Export/Base/RendererTrait.php @@ -46,6 +46,8 @@ trait RendererTrait } $id = $customerId . '_' . $projectId; + $type = $exportItem->getType(); + $category = $exportItem->getCategory(); if (!isset($summary[$id])) { $summary[$id] = [ @@ -56,6 +58,24 @@ trait RendererTrait 'rate' => 0, 'rate_internal' => 0, 'duration' => 0, + 'type' => [], + 'types' => [], + ]; + } + + if (!isset($summary[$id]['type'][$type])) { + $summary[$id]['type'][$type] = [ + 'rate' => 0, + 'rate_internal' => 0, + 'duration' => 0, + ]; + } + + if (!isset($summary[$id]['types'][$type][$category])) { + $summary[$id]['types'][$type][$category] = [ + 'rate' => 0, + 'rate_internal' => 0, + 'duration' => 0, ]; } @@ -75,12 +95,23 @@ trait RendererTrait } $summary[$id]['rate'] += $exportItem->getRate(); + $summary[$id]['type'][$type]['rate'] += $exportItem->getRate(); + $summary[$id]['types'][$type][$category]['rate'] += $exportItem->getRate(); + if (method_exists($exportItem, 'getInternalRate')) { $summary[$id]['rate_internal'] += $exportItem->getInternalRate(); + $summary[$id]['type'][$type]['rate_internal'] += $exportItem->getInternalRate(); + $summary[$id]['types'][$type][$category]['rate_internal'] += $exportItem->getInternalRate(); } else { $summary[$id]['rate_internal'] += $exportItem->getRate(); + $summary[$id]['type'][$type]['rate_internal'] += $exportItem->getRate(); + $summary[$id]['types'][$type][$category]['rate_internal'] += $exportItem->getRate(); } + $summary[$id]['duration'] += $duration; + $summary[$id]['type'][$type]['duration'] += $duration; + $summary[$id]['types'][$type][$category]['duration'] += $duration; + $summary[$id]['activities'][$activityId]['rate'] += $exportItem->getRate(); $summary[$id]['activities'][$activityId]['duration'] += $duration; } diff --git a/src/Export/ExportRepositoryInterface.php b/src/Export/ExportRepositoryInterface.php new file mode 100644 index 00000000..369f9241 --- /dev/null +++ b/src/Export/ExportRepositoryInterface.php @@ -0,0 +1,38 @@ +repositories[] = $repository; + + return $this; + } + + public function getExportItems(ExportQuery $query) + { + $items = []; + + foreach ($this->repositories as $repository) { + $items = array_merge($items, $repository->getExportItemsForQuery($query)); + } + + return $items; + } + + public function setExported(array $items): void + { + foreach ($this->repositories as $repository) { + $repository->setExported($items); + } + } } diff --git a/src/Export/TimesheetExportRepository.php b/src/Export/TimesheetExportRepository.php new file mode 100644 index 00000000..d550918f --- /dev/null +++ b/src/Export/TimesheetExportRepository.php @@ -0,0 +1,57 @@ +repository = $repository; + } + + /** + * @param Timesheet[] $items + */ + public function setExported(array $items): void + { + $timesheets = []; + + foreach ($items as $item) { + if ($item instanceof Timesheet) { + $timesheets[] = $item; + } + } + + if (empty($timesheets)) { + return; + } + + $this->repository->setExported($timesheets); + } + + public function getExportItemsForQuery(ExportQuery $query): iterable + { + return $this->repository->getTimesheetsForQuery($query, true); + } + + public function getType(): string + { + return 'timesheet'; + } +} diff --git a/src/Form/Toolbar/ExportToolbarForm.php b/src/Form/Toolbar/ExportToolbarForm.php index a9898dff..f8943338 100644 --- a/src/Form/Toolbar/ExportToolbarForm.php +++ b/src/Form/Toolbar/ExportToolbarForm.php @@ -29,12 +29,14 @@ class ExportToolbarForm extends AbstractToolbarForm $this->addSearchTermInputField($builder); $this->addExportStateChoice($builder); $this->addTimesheetStateChoice($builder); - $this->addUsersChoice($builder); + if ($options['include_user']) { + $this->addUsersChoice($builder); + } $this->addDateRangeChoice($builder); $this->addCustomerMultiChoice($builder, ['start_date_param' => null, 'end_date_param' => null, 'ignore_date' => true], true); $this->addProjectMultiChoice($builder, ['ignore_date' => true], true, true); $this->addActivityMultiChoice($builder, [], true); - $this->addExportType($builder); + $this->addExportRenderer($builder); $this->addTagInputField($builder); $builder->add('markAsExported', CheckboxType::class, [ 'label' => 'label.mark_as_exported', @@ -48,9 +50,9 @@ class ExportToolbarForm extends AbstractToolbarForm /** * @param FormBuilderInterface $builder */ - protected function addExportType(FormBuilderInterface $builder) + protected function addExportRenderer(FormBuilderInterface $builder) { - $builder->add('type', HiddenType::class, []); + $builder->add('renderer', HiddenType::class, []); } /** @@ -61,6 +63,7 @@ class ExportToolbarForm extends AbstractToolbarForm $resolver->setDefaults([ 'data_class' => ExportQuery::class, 'csrf_protection' => false, + 'include_user' => true, ]); } } diff --git a/src/Kernel.php b/src/Kernel.php index 8651b23e..0d0b8bb6 100644 --- a/src/Kernel.php +++ b/src/Kernel.php @@ -15,6 +15,7 @@ use App\DependencyInjection\Compiler\ExportServiceCompilerPass; use App\DependencyInjection\Compiler\InvoiceServiceCompilerPass; use App\DependencyInjection\Compiler\TwigContextCompilerPass; use App\DependencyInjection\Compiler\WidgetCompilerPass; +use App\Export\ExportRepositoryInterface; use App\Export\RendererInterface as ExportRendererInterface; use App\Export\TimesheetExportInterface; use App\Invoice\CalculatorInterface as InvoiceCalculator; @@ -50,6 +51,7 @@ class Kernel extends BaseKernel public const TAG_WIDGET = 'widget'; public const TAG_WIDGET_RENDERER = 'widget.renderer'; public const TAG_EXPORT_RENDERER = 'export.renderer'; + public const TAG_EXPORT_REPOSITORY = 'export.repository'; public const TAG_INVOICE_RENDERER = 'invoice.renderer'; public const TAG_INVOICE_NUMBER_GENERATOR = 'invoice.number_generator'; public const TAG_INVOICE_CALCULATOR = 'invoice.calculator'; @@ -74,6 +76,7 @@ class Kernel extends BaseKernel { $container->registerForAutoconfiguration(TimesheetCalculator::class)->addTag(self::TAG_TIMESHEET_CALCULATOR); $container->registerForAutoconfiguration(ExportRendererInterface::class)->addTag(self::TAG_EXPORT_RENDERER); + $container->registerForAutoconfiguration(ExportRepositoryInterface::class)->addTag(self::TAG_EXPORT_REPOSITORY); $container->registerForAutoconfiguration(InvoiceRendererInterface::class)->addTag(self::TAG_INVOICE_RENDERER); $container->registerForAutoconfiguration(NumberGeneratorInterface::class)->addTag(self::TAG_INVOICE_NUMBER_GENERATOR); $container->registerForAutoconfiguration(InvoiceCalculator::class)->addTag(self::TAG_INVOICE_CALCULATOR); diff --git a/src/Repository/Query/ExportQuery.php b/src/Repository/Query/ExportQuery.php index 10609dbf..de91f31c 100644 --- a/src/Repository/Query/ExportQuery.php +++ b/src/Repository/Query/ExportQuery.php @@ -14,20 +14,20 @@ class ExportQuery extends TimesheetQuery /** * @var string */ - private $type; + private $renderer; /** * @var bool */ private $markAsExported = false; - public function getType(): ?string + public function getRenderer(): ?string { - return $this->type; + return $this->renderer; } - public function setType(string $type): ExportQuery + public function setRenderer(string $renderer): ExportQuery { - $this->type = $type; + $this->renderer = $renderer; return $this; } diff --git a/src/Repository/TimesheetInvoiceItemRepository.php b/src/Repository/TimesheetInvoiceItemRepository.php index c14af756..1155e8ad 100644 --- a/src/Repository/TimesheetInvoiceItemRepository.php +++ b/src/Repository/TimesheetInvoiceItemRepository.php @@ -36,16 +36,22 @@ final class TimesheetInvoiceItemRepository implements InvoiceItemRepositoryInter } /** - * @param Timesheet[] $invoiceItems + * @param InvoiceItemInterface[] $invoiceItems */ public function setExported(array $invoiceItems) { + $timesheets = []; + foreach ($invoiceItems as $item) { - if (!$item instanceof Timesheet) { - throw new \InvalidArgumentException('TimesheetInvoiceItemRepository only supports Timesheet entities'); + if ($item instanceof Timesheet) { + $timesheets[] = $item; } } - $this->repository->setExported($invoiceItems); + if (empty($timesheets)) { + return; + } + + $this->repository->setExported($timesheets); } } diff --git a/src/Repository/TimesheetRepository.php b/src/Repository/TimesheetRepository.php index 74f64ac3..4e9ef668 100644 --- a/src/Repository/TimesheetRepository.php +++ b/src/Repository/TimesheetRepository.php @@ -957,17 +957,21 @@ class TimesheetRepository extends EntityRepository $em = $this->getEntityManager(); $em->beginTransaction(); - $qb = $em->createQueryBuilder(); - $qb - ->update(Timesheet::class, 't') - ->set('t.exported', ':exported') - ->where($qb->expr()->in('t.id', ':ids')) - ->setParameter('exported', true, PDO::PARAM_BOOL) - ->setParameter('ids', $timesheets) - ->getQuery() - ->execute(); + try { + $qb = $em->createQueryBuilder(); + $qb + ->update(Timesheet::class, 't') + ->set('t.exported', ':exported') + ->where($qb->expr()->in('t.id', ':ids')) + ->setParameter('exported', true, PDO::PARAM_BOOL) + ->setParameter('ids', $timesheets) + ->getQuery() + ->execute(); - $em->commit(); + $em->commit(); + } catch (\Exception $ex) { + $em->rollback(); + } } /** diff --git a/templates/export/index.html.twig b/templates/export/index.html.twig index e8257f61..190fc07b 100644 --- a/templates/export/index.html.twig +++ b/templates/export/index.html.twig @@ -41,7 +41,9 @@ {{ form_row(form.projects) }} {{ form_row(form.activities) }} {{ form_row(form.tags) }} - {{ form_row(form.users) }} + {% if form.users is defined %} + {{ form_row(form.users) }} + {% endif %} {{ form_row(form.exported) }} {{ form_row(form.state) }} {{ form_row(form.markAsExported) }} @@ -289,13 +291,13 @@ }); $('body').on('click', '#export-buttons .startExportBtn', function() { - $('#type').val($(this).attr('data-type')); + $('#renderer').val($(this).attr('data-type')); var $form = $("#export-form"); var prevAction = $form.attr('action'); var prevMethod = $form.attr('method'); $form.attr('target', '_blank').attr('method', 'POST').attr('action', '{{ path('export_data') }}'); $form.submit(); - $('#type').val(''); + $('#renderer').val(''); $form.removeAttr('target').attr('action', prevAction).attr('method', prevMethod); }); }); diff --git a/templates/export/pdf-layout.html.twig b/templates/export/pdf-layout.html.twig index 2bea6280..b7cc1125 100644 --- a/templates/export/pdf-layout.html.twig +++ b/templates/export/pdf-layout.html.twig @@ -1,13 +1,13 @@ {% set showUserColumn = showUserColumn ?? true %} {% set showInternalRate = showInternalRate ?? false %} -{% set showRateColumn = showRateColumn ?? true %} +{% set showRateColumn = showRateColumn ?? is_granted('view_rate_other_timesheet') %} {% set showRateBudget = showRateBudget ?? false %} {% set showTimeBudget = showTimeBudget ?? false %} {% set decimal = decimal ?? false %} -{% if query.user %} - {# this is only triggered, if a user exports from his personal timesheet screen #} +{# this is only triggered, if a user exports from his personal timesheet screen #} +{% if query.user is not null %} {% set showUserColumn = false %} - {# if exporting via the admin screen, users without view_rate_own_timesheet might still see their own rates - maybe merge view_rate_own_timesheet and view_rate_other_timesheet into a new view_rate permission? #} + {# if exporting via the admin screen, users without view_rate_own_timesheet might still see their own rates #} {% set showRateColumn = is_granted('view_rate_own_timesheet') %} {% endif %} @@ -268,15 +268,12 @@ mpdf--> {{ entry.duration|duration(decimal) }} {% if showRateColumn %} - {% if is_granted('view_rate', entry) %} - {% set rate = rate + entry.rate %} - {% set rateInternal = rateInternal + entry.internalRate %} - {% set entryRate = entry.rate|money(entry.project.customer.currency) %} - {% set entryRateInternal = entry.internalRate|money(entry.project.customer.currency) %} - {% else %} - {% set entryRate = '–' %} - {% set entryRateInternal = '–' %} - {% endif %} + {# no check for is_granted('view_rate', entry) because it is only available for timesheets, + but maybe missing for other potential export repositories #} + {% set rate = rate + entry.rate %} + {% set rateInternal = rateInternal + entry.internalRate %} + {% set entryRate = entry.rate|money(entry.project.customer.currency) %} + {% set entryRateInternal = entry.internalRate|money(entry.project.customer.currency) %} {% if showInternalRate %} {{ entryRateInternal }} {% endif %} diff --git a/templates/export/renderer/default.html.twig b/templates/export/renderer/default.html.twig index 4a19dcc2..431acba6 100644 --- a/templates/export/renderer/default.html.twig +++ b/templates/export/renderer/default.html.twig @@ -451,7 +451,7 @@ {% endif %} - {% if entry.tags is not empty %} + {% if entry.tags is defined and entry.tags is not empty %} {{ entry.tagsAsArray|join(', ') }} {% endif %} diff --git a/tests/Controller/ExportControllerTest.php b/tests/Controller/ExportControllerTest.php index ea66fa0f..0f3221b0 100644 --- a/tests/Controller/ExportControllerTest.php +++ b/tests/Controller/ExportControllerTest.php @@ -178,7 +178,7 @@ class ExportControllerTest extends ControllerBaseTest $node->setAttribute('method', 'POST'); $client->submit($form, [ - 'type' => 'default' + 'renderer' => 'default' ]); $response = $client->getResponse(); @@ -212,7 +212,7 @@ class ExportControllerTest extends ControllerBaseTest // don't add daterange to make sure the current month is the default range $client->submit($form, [ - 'type' => 'default.html.twig', + 'renderer' => 'default.html.twig', 'markAsExported' => 1 ]); diff --git a/tests/Export/TimesheetExportRepositoryTest.php b/tests/Export/TimesheetExportRepositoryTest.php new file mode 100644 index 00000000..8bba9b22 --- /dev/null +++ b/tests/Export/TimesheetExportRepositoryTest.php @@ -0,0 +1,47 @@ +createMock(TimesheetRepository::class); + $repository->expects($this->once())->method('setExported')->willReturnCallback(function (array $items) { + self::assertCount(2, $items); + }); + + $sut = new TimesheetExportRepository($repository); + + /* @phpstan-ignore-next-line */ + $sut->setExported([new Timesheet(), null, new \stdClass(), new Timesheet(), new Activity()]); + // test else for empty array + /* @phpstan-ignore-next-line */ + $sut->setExported([new Customer(), new Project()]); + } + + public function testSetType() + { + $repository = $this->createMock(TimesheetRepository::class); + $sut = new TimesheetExportRepository($repository); + self::assertEquals('timesheet', $sut->getType()); + } +} diff --git a/tests/Repository/Query/ExportQueryTest.php b/tests/Repository/Query/ExportQueryTest.php index a7f39352..85e37464 100644 --- a/tests/Repository/Query/ExportQueryTest.php +++ b/tests/Repository/Query/ExportQueryTest.php @@ -32,7 +32,7 @@ class ExportQueryTest extends TimesheetQueryTest $this->assertActivity($sut); $this->assertState($sut); $this->assertExported($sut); - $this->assertType($sut); + $this->assertRenderer($sut); $this->assertMarkAsExported($sut); } @@ -44,15 +44,15 @@ class ExportQueryTest extends TimesheetQueryTest $this->assertTrue($sut->isMarkAsExported()); } - protected function assertType(ExportQuery $sut) + protected function assertRenderer(ExportQuery $sut) { - $this->assertNull($sut->getType()); + $this->assertNull($sut->getRenderer()); $exportTypes = ['html', 'csv', 'pdf', 'xlsx', 'ods']; foreach ($exportTypes as $type) { - $sut->setType($type); - $this->assertEquals($type, $sut->getType()); + $sut->setRenderer($type); + $this->assertEquals($type, $sut->getRenderer()); } } } diff --git a/tests/Repository/TimesheetInvoiceItemRepositoryTest.php b/tests/Repository/TimesheetInvoiceItemRepositoryTest.php new file mode 100644 index 00000000..4ea52e9e --- /dev/null +++ b/tests/Repository/TimesheetInvoiceItemRepositoryTest.php @@ -0,0 +1,40 @@ +createMock(TimesheetRepository::class); + $repository->expects($this->once())->method('setExported')->willReturnCallback(function (array $items) { + self::assertCount(2, $items); + }); + + $sut = new TimesheetInvoiceItemRepository($repository); + + /* @phpstan-ignore-next-line */ + $sut->setExported([new Timesheet(), null, new \stdClass(), new Timesheet(), new Activity()]); + // test else for empty array + /* @phpstan-ignore-next-line */ + $sut->setExported([new Customer(), new Project()]); + } +}