Improve weekly hours form (#5528)

phpstan.neon

@@ -2010,7 +2010,7 @@ parameters:
 
         -
             message: "#^Cannot clone mixed\\.$#"
-            count: 2
+            count: 1
             path: src/Form/QuickEntryForm.php
 
         -
@@ -2838,21 +2838,6 @@ parameters:
             count: 1
             path: src/Form/Type/QuickEntryTimesheetType.php
 
-        -
-            message: "#^Cannot access offset 'activity' on mixed\\.$#"
-            count: 1
-            path: src/Form/Type/QuickEntryWeekType.php
-
-        -
-            message: "#^Cannot access offset 'project' on mixed\\.$#"
-            count: 1
-            path: src/Form/Type/QuickEntryWeekType.php
-
-        -
-            message: "#^Cannot clone mixed\\.$#"
-            count: 1
-            path: src/Form/Type/QuickEntryWeekType.php
-
         -
             message: "#^Method App\\\\Form\\\\Type\\\\SearchTermType\\:\\:buildForm\\(\\) has parameter \\$options with no value type specified in iterable type array\\.$#"
             count: 1

src/Controller/QuickEntryController.php

@@ -20,6 +20,7 @@ use App\Repository\TimesheetRepository;
 use App\Timesheet\FavoriteRecordService;
 use App\Timesheet\TimesheetService;
 use App\Utils\PageSetup;
+use App\WorkingTime\WorkingTimeService;
 use Psr\EventDispatcher\EventDispatcherInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
@@ -38,6 +39,7 @@ final class QuickEntryController extends AbstractController
         private readonly TimesheetRepository $repository,
         private readonly FavoriteRecordService $favoriteRecordService,
         private readonly EventDispatcherInterface $dispatcher,
+        private readonly WorkingTimeService $workingTimeService,
     )
     {
     }
@@ -114,37 +116,46 @@ final class QuickEntryController extends AbstractController
 
         ksort($rows);
 
-        // attach recent activities
+        // this should also check via lock service
-        $amount = $this->configuration->getQuickEntriesRecentAmount();
+        $locked = $this->workingTimeService->isApproved($user, $endWeek);
-        if ($amount > 0) {
-            $takeOverWeeks = $this->configuration->find('quick_entry.recent_activity_weeks');
-            $startFrom = null;
-            if ($takeOverWeeks !== null && \intval($takeOverWeeks) > 0) {
-                $startFrom = clone $startWeek;
-                $startFrom->modify(\sprintf('-%s weeks', $takeOverWeeks));
-            }
 
-            $favorites = $this->favoriteRecordService->favoriteEntries($user, $amount);
+        if (!$locked) {
-            foreach ($favorites as $favorite) {
+            // attach recent activities
-                $timesheet = $favorite->getTimesheet();
+            $amount = $this->configuration->getQuickEntriesRecentAmount();
-                if ($startFrom !== null && !$favorite->isFavorite() && $startFrom > $timesheet->getBegin()) {
+            if ($amount > 0) {
-                    continue;
+                $takeOverWeeks = $this->configuration->find('quick_entry.recent_activity_weeks');
+                $startFrom = null;
+                if ($takeOverWeeks !== null && \intval($takeOverWeeks) > 0) {
+                    $startFrom = clone $startWeek;
+                    $startFrom->modify(\sprintf('-%s weeks', $takeOverWeeks));
                 }
 
-                $id = $timesheet->getProject()->getId() . '_' . $timesheet->getActivity()->getId();
+                $favorites = $this->favoriteRecordService->favoriteEntries($user, $amount);
-                if (\array_key_exists($id, $rows)) {
+                foreach ($favorites as $favorite) {
-                    continue;
+                    $timesheet = $favorite->getTimesheet();
+                    if ($startFrom !== null && !$favorite->isFavorite() && $startFrom > $timesheet->getBegin()) {
+                        continue;
+                    }
+
+                    $id = $timesheet->getProject()->getId() . '_' . $timesheet->getActivity()->getId();
+                    if (\array_key_exists($id, $rows)) {
+                        continue;
+                    }
+                    // edge case: a project that starts and ends between the start and end date allows to select it from the dropdown,
+                    // but it is better to hide a row than displaying already ended projects
+                    if ($timesheet->getProject() !== null && (!$timesheet->getProject()->isVisibleAtDate($startWeek) && !$timesheet->getProject()->isVisibleAtDate($endWeek))) {
+                        continue;
+                    }
+                    // make sure no invisible entries are included
+                    if (!$this->isGranted('start', $timesheet)) {
+                        continue;
+                    }
+                    $rows[$id] = [
+                        'days' => $week,
+                        'project' => $timesheet->getProject(),
+                        'activity' => $timesheet->getActivity()
+                    ];
                 }
-                // there is an edge case possible with a project that starts and ends between the start and end date
-                // user could still select it from the dropdown, but it is better to hide a row than displaying already ended projects
-                if ($timesheet->getProject() !== null && (!$timesheet->getProject()->isVisibleAtDate($startWeek) && !$timesheet->getProject()->isVisibleAtDate($endWeek))) {
-                    continue;
-                }
-                $rows[$id] = [
-                    'days' => $week,
-                    'project' => $timesheet->getProject(),
-                    'activity' => $timesheet->getActivity()
-                ];
             }
         }
 
@@ -165,6 +176,7 @@ final class QuickEntryController extends AbstractController
                 if (!\array_key_exists('entry', $day)) {
                     // fill all rows and columns to make sure we do not have missing records
                     $tmp = $this->timesheetService->createNewTimesheet($user);
+                    $tmp->setDuration(null);
                     $tmp->setProject($row['project']);
                     $tmp->setActivity($row['activity']);
                     $newTime = \DateTime::createFromInterface($day['day']);
@@ -185,6 +197,7 @@ final class QuickEntryController extends AbstractController
         $empty->markAsPrototype();
         foreach ($week as $dayId => $day) {
             $tmp = $this->timesheetService->createNewTimesheet($user);
+            $tmp->setDuration(null);
             $newTime = \DateTime::createFromInterface($day['day']);
             $newTime = $newTime->setTime($defaultHour, $defaultMinute, 0, 0);
             $tmp->setBegin($newTime);
@@ -194,18 +207,20 @@ final class QuickEntryController extends AbstractController
 
         // add empty rows for simpler starting
         $minRows = \intval($this->configuration->find('quick_entry.minimum_rows'));
-        if ($formModel->countRows() < $minRows) {
+        if (!$locked && $formModel->countRows() < $minRows) {
             $newRows = $minRows - $formModel->countRows();
             for ($a = 0; $a < $newRows; $a++) {
                 $model = $formModel->addRow($user);
                 foreach ($week as $dayId => $day) {
                     $tmp = $this->timesheetService->createNewTimesheet($user);
+                    $tmp->setDuration(null);
                     $newTime = \DateTime::createFromInterface($day['day']);
                     $newTime = $newTime->setTime($defaultHour, $defaultMinute, 0, 0);
                     $tmp->setBegin($newTime);
                     $this->timesheetService->prepareNewTimesheet($tmp);
                     $model->addTimesheet($tmp);
                 }
+                $model->setMetaFields($metaFields);
             }
         }
 
@@ -229,12 +244,14 @@ final class QuickEntryController extends AbstractController
                 foreach ($tmpModel->getTimesheets() as $timesheet) {
                     if ($timesheet->getId() !== null) {
                         $duration = $timesheet->getDuration(false);
-                        // previously running timesheets were deleted, which was wrong
+                        // running timesheets also have a empty duration.
-                        // so now we distinguish between running timesheets and null duration
+                        // we distinguish them from temporary ones, to make sure they will not be deleted
                         if ($timesheet->isRunning()) {
                             $saveTimesheets[] = $timesheet;
                         } elseif ($duration === null) {
-                            $deleteTimesheets[] = $timesheet;
+                            if ($this->isGranted('delete', $timesheet)) {
+                                $deleteTimesheets[] = $timesheet;
+                            }
                         } else {
                             $saveTimesheets[] = $timesheet;
                         }
@@ -248,14 +265,22 @@ final class QuickEntryController extends AbstractController
 
             try {
                 $saved = false;
-                if (\count($deleteTimesheets) > 0 && $this->isGranted('delete_own_timesheet')) {
+                if (\count($deleteTimesheets) > 0) {
                     $this->timesheetService->deleteMultipleTimesheets($deleteTimesheets);
                     $saved = true;
                 }
 
                 if (\count($saveTimesheets) > 0) {
-                    $this->timesheetService->updateMultipleTimesheets($saveTimesheets);
+                    $saveMe = [];
-                    $saved = true;
+                    foreach ($saveTimesheets as $timesheet) {
+                        if ($timesheet->getId() === null || $this->isGranted('edit', $timesheet)) {
+                            $saveMe[] = $timesheet;
+                        }
+                    }
+                    if (\count($saveMe) > 0) {
+                        $this->timesheetService->updateMultipleTimesheets($saveMe);
+                        $saved = true;
+                    }
                 }
 
                 if ($saved) {
@@ -278,6 +303,7 @@ final class QuickEntryController extends AbstractController
             'days' => $week,
             'form' => $form->createView(),
             'metaColumns' => $metaFields,
+            'locked' => $locked,
         ]);
     }
 }

src/Form/QuickEntryForm.php

@@ -18,6 +18,7 @@ use Symfony\Component\Form\CallbackTransformer;
 use Symfony\Component\Form\Extension\Core\Type\CollectionType;
 use Symfony\Component\Form\FormBuilderInterface;
 use Symfony\Component\Form\FormInterface;
+use Symfony\Component\Form\FormView;
 use Symfony\Component\OptionsResolver\OptionsResolver;
 use Symfony\Component\Validator\Constraints\All;
 use Symfony\Component\Validator\Constraints\Valid;
@@ -92,13 +93,16 @@ final class QuickEntryForm extends AbstractType
             'entry_options' => [
                 'label' => false,
                 'duration_minutes' => $this->configuration->getTimesheetIncrementDuration(),
-                // this is NOT the start_date, because it would prevent projects from appearing
+                // this is NOT the start_date, because it would prevent projects from appearing in the
-                // in the first days of the week, if the projects ends at the end of the week
+                // first days of the week if the project ends at the end of the week.
-                // the validation still triggers if the user selects days outside the project range
+                // the validation still triggers if the user selects days outside the project range.
                 'start_date' => $options['end_date'],
                 'end_date' => $options['end_date'],
                 'empty_data' => function (FormInterface $form) use ($options) {
-                    return clone $options['prototype_data'];
+                    if ($options['prototype_data'] instanceof QuickEntryModel) {
+                        return clone $options['prototype_data'];
+                    }
+                    throw new \Exception('Invalid Prototype given');
                 },
                 'prototype_data' => clone $options['prototype_data'],
             ],
@@ -111,6 +115,25 @@ final class QuickEntryForm extends AbstractType
         ]);
     }
 
+    public function finishView(FormView $view, FormInterface $form, array $options): void
+    {
+        usort($view['rows']->children, function (FormView $a, FormView $b) {
+            /** @var \App\Model\QuickEntryModel $objectA */
+            $objectA = $a->vars['data'];
+            /** @var \App\Model\QuickEntryModel $objectB */
+            $objectB = $b->vars['data'];
+
+            $existingA = $objectA->hasExistingTimesheet();
+            $existingB = $objectB->hasExistingTimesheet();
+
+            if ($existingA === $existingB) {
+                return 0;
+            }
+
+            return ($existingA && !$existingB) ? -1 : 1;
+        });
+    }
+
     public function configureOptions(OptionsResolver $resolver): void
     {
         $start = new \DateTime();

src/Form/Type/QuickEntryTimesheetType.php

@@ -20,7 +20,7 @@ use Symfony\Component\OptionsResolver\OptionsResolver;
 
 final class QuickEntryTimesheetType extends AbstractType
 {
-    public function __construct(private Security $security)
+    public function __construct(private readonly Security $security)
     {
     }
 
@@ -60,8 +60,28 @@ final class QuickEntryTimesheetType extends AbstractType
                     $event->getForm()->get('duration')->setData(null);
                 }
 
-                if (null !== $data && !$this->security->isGranted('edit', $data)) {
+                if ($data instanceof Timesheet && !$this->security->isGranted('edit', $data)) {
+                    $event->getForm()->remove('duration');
                     $event->getForm()->add('duration', DurationType::class, array_merge(['disabled' => true], $durationOptions));
+
+                    $mainForm = $event->getForm()->getParent()?->getParent();
+                    if ($mainForm === null) {
+                        return;
+                    }
+
+                    $isNew = $data->getId() === null;
+
+                    foreach($mainForm->all() as $key => $child) {
+                        if ($key === 'timesheets') {
+                            continue;
+                        }
+                        if ($child->isDisabled() || $isNew) {
+                            continue;
+                        }
+                        $type = \get_class($child->getConfig()->getType()->getInnerType());
+                        $mainForm->remove($key);
+                        $mainForm->add($key, $type, array_merge($child->getConfig()->getOptions(), ['disabled' => true]));
+                    }
                 }
             }
         );

src/Form/Type/QuickEntryWeekType.php

@@ -9,7 +9,6 @@
 
 namespace App\Form\Type;
 
-use App\Entity\User;
 use App\Model\QuickEntryModel;
 use App\Validator\Constraints\QuickEntryTimesheet;
 use DateTime;
@@ -77,21 +76,6 @@ final class QuickEntryWeekType extends AbstractType
         };
         $builder->addEventListener(FormEvents::PRE_SET_DATA, $activityFunction);
 
-        $activityPreSubmitFunction = function (FormEvent $event) use ($activityOptions) {
-            $data = $event->getData();
-
-            if (isset($data['project']) && !empty($data['project'])) {
-                $activityOptions['projects'] = [$data['project']];
-            }
-
-            if (isset($data['activity']) && !empty($data['activity'])) {
-                $activityOptions['activities'] = [$data['activity']];
-            }
-
-            $event->getForm()->add('activity', ActivityType::class, $activityOptions);
-        };
-        $builder->addEventListener(FormEvents::PRE_SUBMIT, $activityPreSubmitFunction);
-
         $builder->add('metaFields', MetaFieldsCollectionType::class);
 
         $builder->add('timesheets', CollectionType::class, [
@@ -112,13 +96,13 @@ final class QuickEntryWeekType extends AbstractType
         ]);
 
         $builder->addEventListener(FormEvents::PRE_SET_DATA, function (FormEvent $event) use ($options) {
-            if ($event->getData() === null) {
+            if ($event->getData() === null && $options['prototype_data'] instanceof QuickEntryModel) {
                 $event->setData(clone $options['prototype_data']);
             }
         });
 
         $builder->addModelTransformer(new CallbackTransformer(
-            function ($transformValue) use ($options) {
+            function ($transformValue) {
                 /** @var QuickEntryModel|null $transformValue */
                 if ($transformValue === null || $transformValue->isPrototype()) {
                     return $transformValue;
@@ -133,9 +117,6 @@ final class QuickEntryWeekType extends AbstractType
                 }
 
                 $user = $transformValue->getUser();
-                if ($user === null && $options['user'] instanceof User) {
-                    $user = $options['user'];
-                }
                 foreach ($transformValue->getTimesheets() as $timesheet) {
                     $timesheet->setUser($user);
                     $timesheet->setProject($project);
@@ -162,9 +143,7 @@ final class QuickEntryWeekType extends AbstractType
                 $activity = $data->getActivity();
 
                 foreach ($newRecords as $record) {
-                    if ($user !== null) {
+                    $record->setUser($user);
-                        $record->setUser($user);
-                    }
                     if ($project !== null) {
                         $record->setProject($project);
                     }

src/Timesheet/LockdownService.php

@@ -17,7 +17,7 @@ final class LockdownService
 {
     private ?bool $isActive = null;
 
-    public function __construct(private SystemConfiguration $configuration)
+    public function __construct(private readonly SystemConfiguration $configuration)
     {
     }
 
@@ -225,7 +225,7 @@ final class LockdownService
             return true;
         }
 
-        // further validate entries inside of the most recent lockdown
+        // further validate entries inside the most recent lockdown
         if ($timesheetStart >= $lockdownStart) {
             // if grace period is still in effect, validation succeeds
             if ($now <= $lockdownGrace) {

templates/quick-entry/index.html.twig

@@ -13,11 +13,15 @@
             {{ form_errors(form) }}
         {% endblock %}
         {% block box_after %}
+            {% if locked %}
+                {{ widgets.alert('warning', 'The chosen date is already locked.'|trans({}, 'validators')) }}
+            {% else %}
             <input type="submit" value="{{ 'action.save'|trans }}" class="btn btn-primary" />
             <button type="button" class="btn btn-success add-item-link" data-collection-prototype="{{ form.rows.vars.id }}" data-collection-holder="ts-collection">
                 {{ icon('create', true) }}
                 {{ 'action.add'|trans }}
             </button>
+            {% endif %}
             {{ form_end(form) }}
         {% endblock %}
         {# "table-responsive" does not work, because that would render dropdowns at the bottom behind the container #}