From c0378b4f51d6f5f604816fbe3dd47d6acf0d6e91 Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Sun, 14 Mar 2021 13:32:47 +0100 Subject: [PATCH] add user preference: default report (#2430) --- src/Controller/ReportingController.php | 29 +++++++- .../UserPreferenceSubscriber.php | 24 +++---- src/Form/Type/ReportType.php | 58 ++++++++++++++++ .../Hydrator/InvoiceModelDefaultHydrator.php | 68 ++++++++++++++----- .../ProjectView/ProjectViewService.php | 7 +- src/Reporting/ReportingService.php | 8 ++- templates/activity/index.html.twig | 20 ++++-- templates/customer/index.html.twig | 20 ++++-- templates/project/index.html.twig | 20 ++++-- templates/reporting/project_view.html.twig | 16 ++--- .../UserPreferenceSubscriberTest.php | 25 ++++++- tests/Reporting/ReportingServiceTest.php | 2 +- translations/messages.de.xlf | 8 +++ translations/messages.en.xlf | 8 +++ translations/reporting.de.xlf | 4 ++ translations/reporting.en.xlf | 4 ++ 16 files changed, 264 insertions(+), 57 deletions(-) create mode 100644 src/Form/Type/ReportType.php diff --git a/src/Controller/ReportingController.php b/src/Controller/ReportingController.php index 010c9117..97c7fb4e 100644 --- a/src/Controller/ReportingController.php +++ b/src/Controller/ReportingController.php @@ -14,6 +14,7 @@ use App\Reporting\MonthByUser; use App\Reporting\MonthByUserForm; use App\Reporting\MonthlyUserList; use App\Reporting\MonthlyUserListForm; +use App\Reporting\ReportingService; use App\Reporting\WeekByUser; use App\Reporting\WeekByUserForm; use App\Repository\Query\UserQuery; @@ -54,9 +55,33 @@ final class ReportingController extends AbstractController * * @return Response */ - public function defaultReport(): Response + public function defaultReport(ReportingService $reportingService): Response { - return $this->redirectToRoute('report_user_week'); + $user = $this->getUser(); + $route = null; + + $defaultReport = $user->getPreferenceValue('reporting.initial_view', ReportingService::DEFAULT_VIEW); + $allReports = $reportingService->getAvailableReports($user); + + foreach ($allReports as $report) { + if ($report->getId() === $defaultReport) { + $route = $report->getRoute(); + break; + } + } + + // fallback, if the configured report could not be found + // eg. when it was deleted or replaced by an enhanced version with a new id + if ($route === null && \count($allReports) > 0) { + $report = $allReports[array_keys($allReports)[0]]; + $route = $report->getRoute(); + } + + if ($route === null) { + throw $this->createNotFoundException('Unknown default report'); + } + + return $this->redirectToRoute($route); } private function canSelectUser(): bool diff --git a/src/EventSubscriber/UserPreferenceSubscriber.php b/src/EventSubscriber/UserPreferenceSubscriber.php index 3f955d21..07aceaba 100644 --- a/src/EventSubscriber/UserPreferenceSubscriber.php +++ b/src/EventSubscriber/UserPreferenceSubscriber.php @@ -18,8 +18,10 @@ use App\Form\Type\CalendarViewType; use App\Form\Type\FirstWeekDayType; use App\Form\Type\InitialViewType; use App\Form\Type\LanguageType; +use App\Form\Type\ReportType; use App\Form\Type\SkinType; use App\Form\Type\ThemeLayoutType; +use App\Reporting\ReportingService; use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\Form\Extension\Core\Type\CheckboxType; @@ -30,24 +32,15 @@ use Symfony\Component\Validator\Constraints\Range; final class UserPreferenceSubscriber implements EventSubscriberInterface { - /** - * @var EventDispatcherInterface - */ private $eventDispatcher; - /** - * @var AuthorizationCheckerInterface - */ private $voter; - /** - * @var SystemConfiguration - */ private $configuration; - public function __construct(EventDispatcherInterface $dispatcher, AuthorizationCheckerInterface $voter, SystemConfiguration $formConfig) + public function __construct(EventDispatcherInterface $eventDispatcher, AuthorizationCheckerInterface $voter, SystemConfiguration $systemConfiguration) { - $this->eventDispatcher = $dispatcher; + $this->eventDispatcher = $eventDispatcher; $this->voter = $voter; - $this->configuration = $formConfig; + $this->configuration = $systemConfiguration; } public static function getSubscribedEvents(): array @@ -153,6 +146,13 @@ final class UserPreferenceSubscriber implements EventSubscriberInterface ->setSection('behaviour') ->setType(CalendarViewType::class), + (new UserPreference()) + ->setName('reporting.initial_view') + ->setValue(ReportingService::DEFAULT_VIEW) + ->setOrder(650) + ->setSection('behaviour') + ->setType(ReportType::class), + (new UserPreference()) ->setName('login.initial_view') ->setValue(InitialViewType::DEFAULT_VIEW) diff --git a/src/Form/Type/ReportType.php b/src/Form/Type/ReportType.php new file mode 100644 index 00000000..a1f4ef95 --- /dev/null +++ b/src/Form/Type/ReportType.php @@ -0,0 +1,58 @@ +reportingService = $reportingService; + } + + /** + * {@inheritdoc} + */ + public function configureOptions(OptionsResolver $resolver) + { + $resolver->setDefault('required', true); + $resolver->setDefault('translation_domain', 'reporting'); + $resolver->setDefault('choices', function (Options $options) { + /** @var User $user */ + $user = $options['user']; + + $choices = []; + foreach ($this->reportingService->getAvailableReports($user) as $report) { + $choices[$report->getLabel()] = $report->getId(); + } + + return $choices; + }); + } + + /** + * {@inheritdoc} + */ + public function getParent() + { + return ChoiceType::class; + } +} diff --git a/src/Invoice/Hydrator/InvoiceModelDefaultHydrator.php b/src/Invoice/Hydrator/InvoiceModelDefaultHydrator.php index 62729091..e03e0401 100644 --- a/src/Invoice/Hydrator/InvoiceModelDefaultHydrator.php +++ b/src/Invoice/Hydrator/InvoiceModelDefaultHydrator.php @@ -21,13 +21,28 @@ class InvoiceModelDefaultHydrator implements InvoiceModelHydrator $total = $model->getCalculator()->getTotal(); $subtotal = $model->getCalculator()->getSubtotal(); $formatter = $model->getFormatter(); + $entries = $model->getCalculator()->getEntries(); - return [ + $begin = null; + if ($model->getQuery()->getBegin() !== null) { + $begin = $model->getQuery()->getBegin(); + } elseif (!empty($entries)) { + $begin = $entries[0]; + } + + $end = null; + if ($model->getQuery()->getEnd() !== null) { + $end = $model->getQuery()->getEnd(); + } elseif (!empty($entries)) { + $end = array_keys($entries)[\count($entries) - 1]; + } + + $values = [ 'invoice.due_date' => $formatter->getFormattedDateTime($model->getDueDate()), 'invoice.date' => $formatter->getFormattedDateTime($model->getInvoiceDate()), 'invoice.number' => $model->getInvoiceNumber(), 'invoice.currency' => $currency, - 'invoice.language' => $model->getTemplate()->getLanguage(), // since 1.9 + 'invoice.language' => $model->getTemplate()->getLanguage(), // since 1.9 'invoice.currency_symbol' => $formatter->getCurrencySymbol($currency), 'invoice.vat' => $model->getCalculator()->getVat(), 'invoice.tax' => $formatter->getFormattedMoney($tax, $currency), @@ -52,20 +67,41 @@ class InvoiceModelDefaultHydrator implements InvoiceModelHydrator 'template.contact' => $model->getTemplate()->getContact(), 'template.payment_details' => $model->getTemplate()->getPaymentDetails(), - 'query.begin' => $formatter->getFormattedDateTime($model->getQuery()->getBegin()), - 'query.day' => $model->getQuery()->getBegin()->format('d'), // @deprecated - 'query.month' => $formatter->getFormattedMonthName($model->getQuery()->getBegin()), // @deprecated - 'query.month_number' => $model->getQuery()->getBegin()->format('m'), // @deprecated - 'query.year' => $model->getQuery()->getBegin()->format('Y'), // @deprecated - 'query.begin_day' => $model->getQuery()->getBegin()->format('d'), - 'query.begin_month' => $formatter->getFormattedMonthName($model->getQuery()->getBegin()), - 'query.begin_month_number' => $model->getQuery()->getBegin()->format('m'), - 'query.begin_year' => $model->getQuery()->getBegin()->format('Y'), - 'query.end' => $formatter->getFormattedDateTime($model->getQuery()->getEnd()), // since 1.9 - 'query.end_day' => $model->getQuery()->getEnd()->format('d'), // since 1.9 - 'query.end_month' => $formatter->getFormattedMonthName($model->getQuery()->getEnd()), // since 1.9 - 'query.end_month_number' => $model->getQuery()->getEnd()->format('m'), // since 1.9 - 'query.end_year' => $model->getQuery()->getEnd()->format('Y'), // since 1.9 + 'query.begin' => '', + 'query.day' => '', // @deprecated + 'query.month' => '', // @deprecated + 'query.month_number' => '', // @deprecated + 'query.year' => '', // @deprecated + 'query.begin_day' => '', + 'query.begin_month' => '', + 'query.begin_month_number' => '', + 'query.begin_year' => '', + 'query.end' => '', // since 1.9 + 'query.end_day' => '', // since 1.9 + 'query.end_month' => '', // since 1.9 + 'query.end_month_number' => '', // since 1.9 + 'query.end_year' => '', // since 1.9 ]; + + if ($begin !== null) { + $values = array_merge($values, [ + 'query.begin' => $formatter->getFormattedDateTime($begin), + 'query.day' => $begin->format('d'), // @deprecated + 'query.month' => $formatter->getFormattedMonthName($begin), // @deprecated + 'query.month_number' => $begin->format('m'), // @deprecated + 'query.year' => $begin->format('Y'), // @deprecated + 'query.begin_day' => $begin->format('d'), + 'query.begin_month' => $formatter->getFormattedMonthName($begin), + 'query.begin_month_number' => $begin->format('m'), + 'query.begin_year' => $begin->format('Y'), + 'query.end' => $formatter->getFormattedDateTime($end), // since 1.9 + 'query.end_day' => $end->format('d'), // since 1.9 + 'query.end_month' => $formatter->getFormattedMonthName($end), // since 1.9 + 'query.end_month_number' => $end->format('m'), // since 1.9 + 'query.end_year' => $end->format('Y'), // since 1.9 + ]); + } + + return $values; } } diff --git a/src/Reporting/ProjectView/ProjectViewService.php b/src/Reporting/ProjectView/ProjectViewService.php index 831f2cb8..e585a2ef 100644 --- a/src/Reporting/ProjectView/ProjectViewService.php +++ b/src/Reporting/ProjectView/ProjectViewService.php @@ -66,7 +66,12 @@ final class ProjectViewService } if (!$query->isIncludeNoBudget()) { - $qb->andWhere($qb->expr()->gt('p.timeBudget', 0)); + $qb->andWhere( + $qb->expr()->orX( + $qb->expr()->gt('p.budget', 0), + $qb->expr()->gt('p.timeBudget', 0) + ) + ); } $this->repository->addPermissionCriteria($qb, $user); diff --git a/src/Reporting/ReportingService.php b/src/Reporting/ReportingService.php index d998b1f7..5bab3e52 100644 --- a/src/Reporting/ReportingService.php +++ b/src/Reporting/ReportingService.php @@ -16,6 +16,8 @@ use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; final class ReportingService { + public const DEFAULT_VIEW = 'week_by_user'; + /** * @var EventDispatcherInterface */ @@ -40,7 +42,7 @@ final class ReportingService $event = new ReportingEvent($user); if ($this->security->isGranted('view_reporting')) { - $event->addReport(new Report('week_by_user', 'report_user_week', 'report_user_week')); + $event->addReport(new Report(self::DEFAULT_VIEW, 'report_user_week', 'report_user_week')); $event->addReport(new Report('month_by_user', 'report_user_month', 'report_user_month')); if ($this->security->isGranted('budget_project')) { $event->addReport(new Report('project_view', 'report_project_view', 'report_project_view')); @@ -48,9 +50,9 @@ final class ReportingService if ($this->security->isGranted('view_other_timesheet')) { $event->addReport(new Report('monthly_users_list', 'report_monthly_users', 'report_monthly_users')); } - } - $this->dispatcher->dispatch($event); + $this->dispatcher->dispatch($event); + } return $event->getReports(); } diff --git a/templates/activity/index.html.twig b/templates/activity/index.html.twig index 774864ed..d05faed7 100644 --- a/templates/activity/index.html.twig +++ b/templates/activity/index.html.twig @@ -15,8 +15,8 @@ }) %} {% endfor %} {% set columns = columns|merge({ - 'budget': {'class': 'hidden-xs hidden-sm hidden', 'title': 'label.budget'|trans}, - 'timeBudget': {'class': 'hidden-xs hidden-sm hidden', 'title': 'label.timeBudget'|trans}, + 'budget': {'class': 'hidden-xs hidden-sm hidden text-center w-min', 'title': 'label.budget'|trans}, + 'timeBudget': {'class': 'hidden-xs hidden-sm hidden text-center w-min', 'title': 'label.timeBudget'|trans}, 'team': {'class': 'text-center w-min', 'orderBy': false}, 'visible': {'class': 'text-center hidden w-min'}, 'actions': {'class': 'actions alwaysVisible'}, @@ -60,8 +60,20 @@ {{ tables.datatable_meta_column(entry, field) }} {% endfor %} - {{ entry.budget|money((entry.project is null ? defaultCurrency : entry.project.customer.currency)) }} - {{ entry.timeBudget|duration }} + + {% if entry.hasBudget() %} + {{ entry.budget|money((entry.project is null ? defaultCurrency : entry.project.customer.currency)) }} + {% else %} + – + {% endif %} + + + {% if entry.hasTimeBudget() %} + {{ entry.timeBudget|duration }} + {% else %} + – + {% endif %} + {{ widgets.badge_team_access(entry.teams) }} {{ widgets.label_visible(entry.visible) }} {{ actions.activity(entry, 'index') }} diff --git a/templates/customer/index.html.twig b/templates/customer/index.html.twig index 8867bc1b..9db57679 100644 --- a/templates/customer/index.html.twig +++ b/templates/customer/index.html.twig @@ -26,8 +26,8 @@ }) %} {% endfor %} {% set columns = columns|merge({ - 'budget': {'class': 'hidden-xs hidden-sm hidden', 'title': 'label.budget'|trans}, - 'timeBudget': {'class': 'hidden-xs hidden-sm hidden', 'title': 'label.timeBudget'|trans}, + 'budget': {'class': 'hidden-xs hidden-sm hidden text-center w-min', 'title': 'label.budget'|trans}, + 'timeBudget': {'class': 'hidden-xs hidden-sm hidden text-center w-min', 'title': 'label.timeBudget'|trans}, 'team': {'class': 'text-center w-min', 'orderBy': false}, 'visible': {'class': 'text-center hidden w-min'}, 'actions': {'class': 'actions alwaysVisible'}, @@ -71,8 +71,20 @@ {{ tables.datatable_meta_column(entry, field) }} {% endfor %} - {{ entry.budget|money(entry.currency) }} - {{ entry.timeBudget|duration }} + + {% if entry.hasBudget() %} + {{ entry.budget|money(entry.currency) }} + {% else %} + – + {% endif %} + + + {% if entry.hasBudget() %} + {{ entry.timeBudget|duration }} + {% else %} + – + {% endif %} + {{ widgets.badge_team_access(entry.teams) }} {{ widgets.label_visible(entry.visible) }} {{ actions.customer(entry, 'index') }} diff --git a/templates/project/index.html.twig b/templates/project/index.html.twig index d8934006..b92d3994 100644 --- a/templates/project/index.html.twig +++ b/templates/project/index.html.twig @@ -19,8 +19,8 @@ }) %} {% endfor %} {% set columns = columns|merge({ - 'budget': {'class': 'hidden-xs hidden-sm hidden', 'title': 'label.budget'|trans}, - 'timeBudget': {'class': 'hidden-xs hidden-sm hidden', 'title': 'label.timeBudget'|trans}, + 'budget': {'class': 'hidden-xs hidden-sm hidden text-center w-min', 'title': 'label.budget'|trans}, + 'timeBudget': {'class': 'hidden-xs hidden-sm hidden text-center w-min', 'title': 'label.timeBudget'|trans}, 'team': {'class': 'text-center w-min', 'orderBy': false}, 'visible': {'class': 'text-center hidden w-min'}, 'actions': {'class': 'actions alwaysVisible'}, @@ -57,8 +57,20 @@ {{ tables.datatable_meta_column(entry, field) }} {% endfor %} - {{ entry.budget|money(entry.customer.currency) }} - {{ entry.timeBudget|duration }} + + {% if entry.hasBudget() %} + {{ entry.budget|money(entry.customer.currency) }} + {% else %} + – + {% endif %} + + + {% if entry.hasBudget() %} + {{ entry.timeBudget|duration }} + {% else %} + – + {% endif %} + {{ widgets.badge_team_access(entry.teams) }} {{ widgets.label_visible(entry.visible) }} {{ actions.project(entry, 'index') }} diff --git a/templates/reporting/project_view.html.twig b/templates/reporting/project_view.html.twig index 3d9a32a2..4666bddb 100644 --- a/templates/reporting/project_view.html.twig +++ b/templates/reporting/project_view.html.twig @@ -4,16 +4,16 @@ {% block report_title %}{{ 'report_project_view'|trans({}, 'reporting') }}{% endblock %} {% set columns = { - 'name': {'class': 'alwaysVisible'}, - 'today': {'class': 'text-nowrap text-right', 'title': 'daterangepicker.today'|trans({}, 'daterangepicker')}, - 'week': {'class': 'text-nowrap text-right', 'title': 'agendaWeek'|trans}, - 'month': {'class': 'text-nowrap text-right', 'title': 'month'|trans}, - 'durationTotal': {'class': 'hidden-md hidden-sm hidden-xs text-nowrap text-right', 'title': 'stats.durationTotal'|trans}, + 'name': {'class': 'alwaysVisible w-min'}, + 'today': {'class': 'text-nowrap text-center w-min', 'title': 'daterangepicker.today'|trans({}, 'daterangepicker')}, + 'week': {'class': 'text-nowrap text-center w-min', 'title': 'agendaWeek'|trans}, + 'month': {'class': 'text-nowrap text-center w-min', 'title': 'month'|trans}, + 'durationTotal': {'class': 'hidden-md hidden-sm hidden-xs text-nowrap text-center w-min', 'title': 'label.total'|trans}, 'timeBudget': {'class': 'text-nowrap', 'title': 'label.timeBudget'|trans}, 'budget': {'class': 'text-nowrap', 'title': 'label.budget'|trans}, - 'stateDuration': {'class': 'hidden-sm hidden-xs text-nowrap text-right', 'title': 'entryState.not_exported'|trans}, - 'stateMoney': {'class': 'hidden-sm hidden-xs text-nowrap text-right', 'title': 'entryState.not_exported'|trans}, - 'projectEnd': {'class': 'hidden-md hidden-sm hidden-xs hidden text-nowrap text-center', 'title': 'label.project_end'|trans}, + 'stateDuration': {'class': 'hidden-sm hidden-xs text-nowrap text-center w-min', 'title': 'label.not_exported'|trans}, + 'stateMoney': {'class': 'hidden-sm hidden-xs text-nowrap text-center w-min', 'title': 'label.not_invoiced'|trans}, + 'projectEnd': {'class': 'hidden-md hidden-sm hidden-xs hidden text-nowrap text-center w-min', 'title': 'label.project_end'|trans}, 'comment': {'class': 'hidden-md hidden-sm hidden-xs hidden', 'title': 'label.comment'|trans}, } %} {% set tableName = 'project_view_reporting' %} diff --git a/tests/EventSubscriber/UserPreferenceSubscriberTest.php b/tests/EventSubscriber/UserPreferenceSubscriberTest.php index 529b045d..e0cc2837 100644 --- a/tests/EventSubscriber/UserPreferenceSubscriberTest.php +++ b/tests/EventSubscriber/UserPreferenceSubscriberTest.php @@ -23,6 +23,23 @@ use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; */ class UserPreferenceSubscriberTest extends TestCase { + public const EXPECTED_PREFERENCES = [ + 'hourly_rate', + 'internal_rate', + 'timezone', + 'language', + 'first_weekday', + 'skin', + 'theme.layout', + 'theme.collapsed_sidebar', + 'theme.update_browser_title', + 'calendar.initial_view', + 'reporting.initial_view', + 'login.initial_view', + 'timesheet.daily_stats', + 'timesheet.export_decimal', + ]; + public function testGetSubscribedEvents() { $events = UserPreferenceSubscriber::getSubscribedEvents(); @@ -40,7 +57,11 @@ class UserPreferenceSubscriberTest extends TestCase self::assertSame($user, $event->getUser()); $prefs = $sut->getDefaultPreferences($user); - self::assertCount(13, $prefs); + foreach ($prefs as $pref) { + $this->assertTrue(\in_array($pref->getName(), self::EXPECTED_PREFERENCES), 'Unknown user preference: ' . $pref->getName()); + } + + self::assertCount(\count(self::EXPECTED_PREFERENCES), $prefs); foreach ($prefs as $pref) { switch ($pref->getName()) { @@ -70,7 +91,7 @@ class UserPreferenceSubscriberTest extends TestCase // TODO test merging values $sut->loadUserPreferences($event); $prefs = $event->getUser()->getPreferences(); - self::assertCount(13, $prefs); + self::assertCount(\count(self::EXPECTED_PREFERENCES), $prefs); foreach ($prefs as $pref) { switch ($pref->getName()) { diff --git a/tests/Reporting/ReportingServiceTest.php b/tests/Reporting/ReportingServiceTest.php index d051e59f..db17d313 100644 --- a/tests/Reporting/ReportingServiceTest.php +++ b/tests/Reporting/ReportingServiceTest.php @@ -24,7 +24,7 @@ class ReportingServiceTest extends TestCase protected function getSut(bool $isGranted = false): ReportingService { $dispatcher = $this->createMock(EventDispatcherInterface::class); - $dispatcher->expects($this->once())->method('dispatch')->willReturnCallback(function ($event) { + $dispatcher->expects($this->exactly($isGranted ? 1 : 0))->method('dispatch')->willReturnCallback(function ($event) { $this->assertInstanceOf(ReportingEvent::class, $event); }); diff --git a/translations/messages.de.xlf b/translations/messages.de.xlf index 938ac9b7..bb9826d8 100644 --- a/translations/messages.de.xlf +++ b/translations/messages.de.xlf @@ -1146,6 +1146,14 @@ label.includeNoBudget Einträge ohne Budget anzeigen + + label.not_exported + Nicht exportiert + + + label.not_invoiced + Nicht abgerechnet + diff --git a/translations/messages.en.xlf b/translations/messages.en.xlf index c1c57049..9a3d4bfb 100644 --- a/translations/messages.en.xlf +++ b/translations/messages.en.xlf @@ -1166,6 +1166,14 @@ label.includeNoBudget Show entries without budget + + label.not_exported + Not exported + + + label.not_invoiced + Not billed + diff --git a/translations/reporting.de.xlf b/translations/reporting.de.xlf index d7d8a5ff..32fec252 100644 --- a/translations/reporting.de.xlf +++ b/translations/reporting.de.xlf @@ -18,6 +18,10 @@ report_project_view Projektübersicht + + reporting.initial_view + Initialer Bericht + diff --git a/translations/reporting.en.xlf b/translations/reporting.en.xlf index c64d399a..f6812427 100644 --- a/translations/reporting.en.xlf +++ b/translations/reporting.en.xlf @@ -18,6 +18,10 @@ report_project_view Project overview + + reporting.initial_view + Initial report +