lock exported timesheets (#798)

.scrutinizer.yml

@@ -35,6 +35,6 @@ filter:
         - 'vendor/'
 
 build_failure_conditions:
-    - 'project.metric("scrutinizer.quality", < 9.30)'
+    - 'project.metric("scrutinizer.quality", < 9.0)'
     - 'project.metric("scrutinizer.test_coverage", < 0.9)'
     - 'project.metric_change("scrutinizer.test_coverage", < -0.01)'

UPGRADING.md

@@ -22,11 +22,12 @@ And make sure to **create a backup before you start**.
 
 Follow the normal update and database migration process (see above).
 
-### Apply necessary changes to your `local.yaml`:
+### Apply necessary changes to your local.yaml:
 
-New permissions are available: 
+New permissions are available. If you configured custom permissions in `local.yaml`, you have to add those, otherwise you can't use the new features: 
 - `view_tag` - view all tags
 - `delete_tag` - delete tags
+- `edit_exported_timesheet` - allows to edit records which were exported
 
 ### BC BREAKS
 

assets/js/plugins/KimaiAjaxModalForm.js

@@ -52,7 +52,10 @@ export default class KimaiAjaxModalForm extends KimaiClickHandlerReducedInTableR
 
         // the modal that we use to render the form in
         let formIdentifier = '#remote_form_modal .modal-content form';
+        // if any of these is found in a response, the form will be re-displayed
         let flashErrorIdentifier = 'div.alert-error';
+        // messages to show above the form
+        let flashMessageIdentifier = 'div.alert';
         let form = jQuery(formIdentifier);
         let remoteModal = jQuery('#remote_form_modal');
 
@@ -88,10 +91,9 @@ export default class KimaiAjaxModalForm extends KimaiClickHandlerReducedInTableR
         }
 
         // show error flash messages
-        if (jQuery(html).find(flashErrorIdentifier).length > 0) {
-            jQuery('#remote_form_modal .modal-body').prepend(
-                jQuery(html).find(flashErrorIdentifier)
-            );
+        let flashMessages = jQuery(html).find(flashMessageIdentifier);
+        if (flashMessages.length > 0) {
+            jQuery('#remote_form_modal .modal-body').prepend(flashMessages);
         }
 
         // -----------------------------------------------------------------------

config/packages/kimai.yaml

@@ -101,8 +101,8 @@ kimai:
             # adding single permissions to user roles, extending the definition from "sets" ("role name" = [array of "permissions"])
             ROLE_USER: []
             ROLE_TEAMLEAD: [view_invoice_template,create_invoice_template,edit_invoice_template,view_rate_own_timesheet,view_rate_other_timesheet,hourly-rate_own_profile]
-            ROLE_ADMIN: [hourly-rate_own_profile]
-            ROLE_SUPER_ADMIN: [hourly-rate_own_profile,hourly-rate_other_profile,delete_own_profile,roles_own_profile,system_information,system_actions,system_configuration,plugins]
+            ROLE_ADMIN: [hourly-rate_own_profile,edit_exported_timesheet]
+            ROLE_SUPER_ADMIN: [hourly-rate_own_profile,hourly-rate_other_profile,delete_own_profile,roles_own_profile,system_information,system_actions,system_configuration,plugins,edit_exported_timesheet]
 # --------------------------------------------------------------------------------
 
 

public/build/manifest.json

@@ -1,5 +1,5 @@
 {
-  "build/app.js": "./app.js?dad0cc8443da0d5789c7",
+  "build/app.js": "./app.js?376d50667e3784222761",
   "build/app.css": "./app.css?954c34ace3717cfb9d9c80d2be5f8c68",
   "build/fonts/fa-solid-900.woff2": "./fonts/fa-solid-900.woff2?e8a92a29",
   "build/images/fa-solid-900.svg": "./images/fa-solid-900.svg?666a82cb",

src/API/TimesheetController.php

@@ -616,4 +616,52 @@ public function restartAction($id, ParamFetcherInterface $paramFetcher, Validato
 
         return $this->viewHandler->handle($view);
     }
+
+    /**
+     * Switch the export state of a timesheet record to (un-)lock it
+     *
+     * @SWG\Response(
+     *      response=200,
+     *      description="Switches the exported state on the record and therefor locks / unlocks it for further updates. Needs edit_export_*_timesheet permission.",
+     *      @SWG\Schema(ref="#/definitions/TimesheetEntity")
+     * )
+     * @SWG\Parameter(
+     *      name="id",
+     *      in="path",
+     *      type="integer",
+     *      description="Timesheet record ID to switch export state",
+     *      required=true,
+     * )
+     *
+     * @Security("is_granted('edit_export_own_timesheet') or is_granted('edit_export_other_timesheet')")
+     *
+     * @param int $id
+     * @return Response
+     */
+    public function exportAction($id)
+    {
+        /** @var Timesheet $timesheet */
+        $timesheet = $this->repository->find($id);
+
+        if (null === $timesheet) {
+            throw new NotFoundException();
+        }
+
+        if (!$this->isGranted('edit_export', $timesheet)) {
+            throw new AccessDeniedHttpException(
+                sprintf('You are not allowed to %s this timesheet', ($timesheet->isExported() ? 'unlock' : 'lock'))
+            );
+        }
+
+        $timesheet->setExported(!$timesheet->isExported());
+
+        $entityManager = $this->getDoctrine()->getManager();
+        $entityManager->persist($timesheet);
+        $entityManager->flush();
+
+        $view = new View($timesheet, 200);
+        $view->getContext()->setGroups(['Default', 'Entity', 'Timesheet']);
+
+        return $this->viewHandler->handle($view);
+    }
 }

src/Command/CreateReleaseCommand.php

@@ -18,6 +18,8 @@
 
 /**
  * Command used to create a release package with pre-installed composer, SQLite database and user.
+ *
+ * @codeCoverageIgnore
  */
 class CreateReleaseCommand extends Command
 {

src/Command/KimaiImporterCommand.php

@@ -37,6 +37,7 @@
  * Command used to import data from a Kimai v1 installation.
  * Getting help in improving this script would be fantastic, it currently only handles the most basic use-cases.
  *
+ * This command is way to messy and complex to be tested ... so we use something, which I actually don't like:
  * @codeCoverageIgnore
  */
 class KimaiImporterCommand extends Command

src/Command/ResetCommand.php

@@ -19,6 +19,11 @@
 
 /**
  * Command used to execute all the basic application bootstrapping AFTER "composer install" was executed.
+ *
+ * This command is NOT used during runtime and only meant for developers on their local machines.
+ * I am too lazy to think about how this could be tested ... and this is one of the rare edge cases where I don't
+ * feel like it is necessary, so I "cheat" with:
+ * @codeCoverageIgnore
  */
 class ResetCommand extends Command
 {

src/Twig/Extensions.php

@@ -93,6 +93,7 @@ class Extensions extends AbstractExtension
         'debug' => 'far fa-file-alt',
         'profile-stats' => 'far fa-chart-bar',
         'profile' => 'fas fa-user-edit',
+        'warning' => 'fas fa-exclamation-triangle',
     ];
 
     /**

src/Voter/TimesheetVoter.php

@@ -85,12 +85,24 @@ protected function voteOnAttribute($attribute, $subject, TokenInterface $token)
                 $permission .= $attribute;
                 break;
 
+            case self::EDIT:
+                if (!$this->canEdit($user, $subject)) {
+                    return false;
+                }
+                $permission .= $attribute;
+                break;
+
+            case self::DELETE:
+                if (!$this->canDelete($user, $subject)) {
+                    return false;
+                }
+                $permission .= $attribute;
+                break;
+
             case self::VIEW_RATE:
             case self::EDIT_RATE:
             case self::STOP:
-            case self::EDIT:
             case self::VIEW:
-            case self::DELETE:
             case self::EXPORT:
             case self::EDIT_EXPORT:
                 $permission .= $attribute;
@@ -114,11 +126,7 @@ protected function voteOnAttribute($attribute, $subject, TokenInterface $token)
         return $this->hasRolePermission($user, $permission);
     }
 
-    /**
-     * @param Timesheet $timesheet
-     * @return bool
-     */
-    protected function canStart(Timesheet $timesheet)
+    protected function canStart(Timesheet $timesheet): bool
     {
         // possible improvements for the future:
         // we could check the amount of active entries (maybe slow)
@@ -142,4 +150,22 @@ protected function canStart(Timesheet $timesheet)
 
         return true;
     }
+
+    protected function canEdit(User $user, Timesheet $timesheet): bool
+    {
+        if ($timesheet->isExported() && !$this->hasRolePermission($user, 'edit_exported_timesheet')) {
+            return false;
+        }
+
+        return true;
+    }
+
+    protected function canDelete(User $user, Timesheet $timesheet): bool
+    {
+        if ($timesheet->isExported() && !$this->hasRolePermission($user, 'edit_exported_timesheet')) {
+            return false;
+        }
+
+        return true;
+    }
 }

templates/export/index.html.twig

@@ -119,42 +119,56 @@
     {{ parent() }}
     <script type="text/javascript">
 
-        function confirmToggleState()
+        function confirmToggleState(button)
         {
-            if ($('#export-toggle-button').hasClass('export-off')) {
-                if (confirm('{{ 'export.clear_all'|trans }}')) {
-                    $('.exportBtn').each(function () {
-                        if ($(this).hasClass('active')) {
-                            $(this).click();
-                        }
-                    });
-                    return true;
+            var ALERT = kimai.getPlugin('alert');
+            var message = '{{ 'export.clear_all'|trans }}';
+            var hasActive = true;
+
+            if (!$('#export-toggle-button').hasClass('export-off')) {
+                message = '{{ 'export.mark_all'|trans }}';
+                hasActive = false;
+            }
+
+            ALERT.question(message, function(value) {
+                var btn = $(button);
+                var exportButtons = $('.exportBtn');
+                // disabling does not yet work...
+                if (exportButtons.length > 0) {
+                    btn.addClass('disabled');
                 }
-            } else {
-                if (confirm('{{ 'export.mark_all'|trans }}')) {
-                    $('.exportBtn').each(function () {
-                        if (!$(this).hasClass('active')) {
-                            $(this).click();
-                        }
-                    });
-                    return true;
+
+                // ... as the clicks are asynchronous and the each comes back too early ...
+                exportButtons.each(function () {
+                    if (hasActive === $(this).hasClass('active')) {
+                        $(this).click();
+                    }
+                });
+
+                // ... so the button is re-enabled immediately - that should be fixed in a future update
+                btn.removeClass('disabled');
+                if (btn.hasClass('export-off')) {
+                    btn.removeClass('export-off');
+                    btn.html('<i class="fas fa-toggle-off"></i>');
+                } else {
+                    btn.addClass('export-off');
+                    btn.html('<i class="fas fa-toggle-on"></i>');
                 }
-            }
-            return false;
+            });
         }
 
         function updateTimesheetExportState(button, id, exported)
         {
-            // FIXME use kimai API and ALERT
+            var ALERT = kimai.getPlugin('alert');
+
+            // FIXME use Kimai API plugin
             $.ajax({
-                url: '{{ path('patch_timesheet', {id: '-s-'}) }}'.replace('-s-', id),
+                url: '{{ path('export_timesheet', {id: '-s-'}) }}'.replace('-s-', id),
                 headers: {
                     'X-AUTH-SESSION': true,
                     'Content-Type':'application/json'
                 },
                 method: 'PATCH',
-                dataType: 'json',
-                data: JSON.stringify({'exported': exported}),
                 success: function(data) {
                     if (exported) {
                         button.button('exported');
@@ -181,13 +195,12 @@
                             }
                         }
                     }
-                    alert('{{ 'action.update.error'|trans({}, 'flashmessages') }}'.replace('%reason%', message));
+                    ALERT.error('{{ 'action.update.error'|trans({}, 'flashmessages') }}', message);
                     button.button('reset');
                     if (exported) {
                         button.removeClass('active');
                     } else {
                         button.addClass('active');
-
                     }
                 }
             });
@@ -197,25 +210,11 @@
             $('body').on('click', '.exportBtn', function() {
                 var button = $(this);
                 var id = button.attr('data-timesheet');
-
-                if (button.hasClass('active')) {
-                    updateTimesheetExportState(button, id, false);
-                } else {
-                    updateTimesheetExportState(button, id, true);
-                }
+                updateTimesheetExportState(button, id, !button.hasClass('active'));
             });
 
             $('#export-toggle-button').on('click', function () {
-                if (!confirmToggleState()) {
-                    return;
-                }
-                if ($(this).hasClass('export-off')) {
-                    $(this).removeClass('export-off');
-                    $(this).html('<i class="fas fa-toggle-off"></i>');
-                } else {
-                    $(this).addClass('export-off');
-                    $(this).html('<i class="fas fa-toggle-on"></i>');
-                }
+                confirmToggleState(this);
             });
 
             $('body').on('click', '#export-buttons .startExportBtn', function() {

templates/macros/widgets.html.twig

@@ -116,8 +116,14 @@
 {% macro alert(type, description, title, icon) %}
     <div class="alert alert-{{ type|default('danger') }} alert-dismissible">
         <button type="button" class="close" data-dismiss="alert" aria-hidden="true">×</button>
-        {% if title %}<h4><i class="icon {{ icon|icon(icon) }}"></i> {{ title|trans }}</h4>{% endif %}
-        {{ description|trans }}
+        {% if title and icon %}
+            <h4><i class="icon {{ icon|icon(icon) }}"></i> {{ title|trans }}</h4>
+            {{ description|trans }}
+        {% elseif icon %}
+            <h4><i class="icon {{ icon|icon(icon) }}"></i> {{ description|trans }}</h4>
+        {% else %}
+            {{ description|trans }}
+        {% endif %}
     </div>
 {% endmacro %}
 

templates/timesheet-team/edit.html.twig

@@ -7,6 +7,9 @@
 {% block page_actions %}{{ actions.timesheet_team(timesheet, 'edit') }}{% endblock %}
 
 {% block main %}
+    {% if timesheet.exported %}
+        {{ widgets.alert('warning', ('timesheet.locked.warning'|trans({}, 'flashmessages')), ('warning'|trans({}, 'flashmessages')), 'warning') }}
+    {% endif %}
     {{ include(app.request.xmlHttpRequest ? 'default/_form_modal.html.twig' : 'default/_form.html.twig', {
         'title': (timesheet.id ? 'timesheet.edit'|trans : 'create'|trans),
         'form': form,

templates/timesheet/edit.html.twig

@@ -7,6 +7,9 @@
 {% block page_actions %}{{ actions.timesheet(timesheet, 'edit') }}{% endblock %}
 
 {% block main %}
+    {% if timesheet.exported %}
+        {{ widgets.alert('warning', ('timesheet.locked.warning'|trans({}, 'flashmessages')), ('warning'|trans({}, 'flashmessages')), 'warning') }}
+    {% endif %}
     {{ include(app.request.xmlHttpRequest ? 'default/_form_modal.html.twig' : 'default/_form.html.twig', {
         'title': (timesheet.id ? 'timesheet.edit'|trans : 'create'|trans),
         'form': form,