fix(Timetracker/Timesheet): update timesheets with invalid relations …

…needs confirmation

tests/tine20/Admin/Frontend/Json/UserTest.php

@@ -355,11 +355,11 @@ public function testDeleteAccountsWithConfirmation()
             $userArray = $this->_createTestUser();
             Admin_Controller_User::getInstance()->delete($userArray['accountId']);
             self::fail('should throw Tinebase_Exception_Confirmation');
-        } catch (Tinebase_Exception_Confirmation $e) {
+        } catch (Tinebase_Exception_Confirmation $tec) {
             $translate = Tinebase_Translation::getTranslation('Admin');
             $translation = $translate->_('Delete user will trigger the [V] events, do you still want to execute this action?');
 
-            self::assertEquals($translation, $e->getMessage());
+            self::assertEquals($translation, $tec->getMessage());
         }
 
         // user deletion need the confirmation header

tests/tine20/Sales/InvoiceJsonTests.php

@@ -295,8 +295,8 @@ public function testClearing()
             $timesheets[0]->start_time = '10:00:00';
             $tsController->update($timesheets[0]);
             self::fail('should throw Tinebase_Exception_Confirmation!');
-        } catch (Tinebase_Exception_Confirmation $seiace) {
-            self::assertEquals(Tinebase_Translation::getTranslation(Timetracker_Config::APP_NAME)->_('The Invoice you tried to edit is cleared already, change date will rebill the invoice, do you still want to execute this action?'), $seiace->getMessage());
+        } catch (Tinebase_Exception_Confirmation $tec) {
+            self::assertEquals(Tinebase_Translation::getTranslation(Timetracker_Config::APP_NAME)->_('The Invoice you tried to edit is cleared already, change date will rebill the invoice, do you still want to execute this action?'), $tec->getMessage());
         }
     }
 

tests/tine20/Timetracker/JsonTest.php

@@ -1153,9 +1153,9 @@ public function testDeleteInUseTimeaccount()
             Timetracker_Controller_Timeaccount::getInstance()->setRequestContext([]);
             $this->_json->deleteTimeaccounts($timesheetData['timeaccount_id']['id']);
             self::fail('should throw Tinebase_Exception_Confirmation');
-        } catch (Tinebase_Exception_Confirmation $e) {
+        } catch (Tinebase_Exception_Confirmation $tec) {
             $translate = Tinebase_Translation::getTranslation('Timetracker');
-            self::assertEquals($translate->_('Timeaccounts are still in use! Are you sure you want to delete them?'), $e->getMessage());
+            self::assertEquals($translate->_('Timeaccounts are still in use! Are you sure you want to delete them?'), $tec->getMessage());
         }
     }
 
@@ -1459,11 +1459,13 @@ public function testTimesheetInvoiceId()
      */
     public function testUpdateClosedTimeaccount()
     {
+        $this->_testNeedsTransaction();
         Timetracker_Controller_Timesheet::getInstance()->doContainerACLChecks(true);
 
         $timeaccountData = $this->_saveTimeaccountWithGrants();
         $timeaccountData['is_open'] = 0;
         $timeaccount = $this->_json->saveTimeaccount($timeaccountData);
+        $this->_deleteTimeAccounts = array($timeaccountData['id']);
 
         $timesheet = $this->_getTimesheet(array(
              'timeaccount_id'    => $timeaccount['id'],
@@ -1476,10 +1478,15 @@ public function testUpdateClosedTimeaccount()
         $timesheetData['timeaccount_id'] = $timesheetData['timeaccount_id']['id'];
         try {
             $timesheetUpdated = $this->_json->saveTimesheet($timesheetData, array('skipClosedCheck' => false));
-            $this->fail('Failed asserting that exception of type "Timetracker_Exception_ClosedTimeaccount" is thrown.');
-        } catch (Timetracker_Exception_ClosedTimeaccount $tect) {
-            $this->assertEquals('This Timeaccount is already closed!', $tect->getMessage());
+            $this->fail('Failed asserting that exception of type "Tinebase_Exception_Confirmation" is thrown.');
+        } catch (Tinebase_Exception_Confirmation $tec) {
+            $translate = Tinebase_Translation::getTranslation('Timetracker');
+            $this->assertEquals($translate->_('The related Timeaccount is already closed, do you still want to execute this action?'), $tec->getMessage());
         }
+
+        // update with the confirmation header
+        $timesheetUpdated = $this->_json->saveTimesheet($timesheetData, ['confirm' => true]);
+        $this->assertEquals('blubbblubb', $timesheetUpdated['description']);
     }
 
     public function testUnitField()

tine20/Sales/Controller/Invoice.php

@@ -1688,7 +1688,7 @@ protected function _inspectDelete(array $_ids)
 
                     // TODO move this to Timetracker (as on delete hook/fn)
                     if ($model == 'Timetracker_Model_Timeaccount') {
-                        // prevent throwing of Timetracker_Exception_ClosedTimeaccount for closed accounts (see \Timetracker_Controller_Timesheet::_checkGrant)
+                        // prevent throwing of Tinebase_Exception_Confirmation for closed accounts (see \Timetracker_Controller_Timesheet::_checkGrant)
                         $billableControllerName::getInstance()->setRequestContext([
                             'skipClosedCheck' => true,
                         ]);

tine20/Timetracker/Controller/Timesheet.php

@@ -374,27 +374,11 @@ protected function _inspectBeforeUpdate($_record, $_oldRecord)
         $this->_calculateTimes($_record);
         $this->_calcClearedAmount($_record, $_oldRecord);
 
-        if ($this->_isTSDateChanged($_record, $_oldRecord)) {
-            if ($_record->is_cleared && !empty($_record->invoice_id)) {
-                $context = $this->getRequestContext();
-
-                if ($context && is_array($context) &&
-                    (array_key_exists('clientData', $context) && array_key_exists('confirm', $context['clientData'])
-                        || array_key_exists('confirm', $context))) {
-
-                    $relation = Tinebase_Relations::getInstance()->getRelations('Sales_Model_Invoice', 'Sql', $_record->invoice_id, 'sibling', array('CONTRACT'), 'Sales_Model_Contract')->getFirstRecord();
-                    $contract = Sales_Controller_Contract::getInstance()->get($relation->related_id);
-
-                    //Sales_Controller_Invoice::getInstance()->delete(array($_record->invoice_id));
-                    Sales_Controller_Invoice::getInstance()->createAutoInvoices(null, $contract, true);
-                } else {
-                    $translation = Tinebase_Translation::getTranslation($this->_applicationName);
-                    $exception = new Tinebase_Exception_Confirmation(
-                        $translation->_('The Invoice you tried to edit is cleared already, change date will rebill the invoice, do you still want to execute this action?')
-                    );
-                    throw $exception;
-                }
-            }
+        if ($this->_isTSDateChanged($_record, $_oldRecord) && $_record->is_cleared && !empty($_record->invoice_id)) {
+            $relation = Tinebase_Relations::getInstance()->getRelations('Sales_Model_Invoice', 'Sql', $_record->invoice_id, 'sibling', ['CONTRACT'], 'Sales_Model_Contract')
+                ->getFirstRecord();
+            $contract = Sales_Controller_Contract::getInstance()->get($relation->related_id);
+            Sales_Controller_Invoice::getInstance()->createAutoInvoices(null, $contract, true);
         }
     }
 
@@ -502,9 +486,10 @@ public function doContainerACLChecks($setTo = NULL)
      * @param Timetracker_Model_Timesheet $_oldRecord
      * @return boolean
      * @throws Tinebase_Exception_AccessDenied
-     * 
-     * @todo think about just setting the default values when user 
-     *       hasn't the required grant to change the field (instead of throwing exception) 
+     * @throws Tinebase_Exception_Confirmation
+     *
+     * @todo think about just setting the default values when user
+     *       hasn't the required grant to change the field (instead of throwing exception)
      */
     protected function _checkGrant($_record, $_action, $_throw = TRUE, $_errorMessage = 'No Permission.', $_oldRecord = NULL)
     {
@@ -521,25 +506,16 @@ protected function _checkGrant($_record, $_action, $_throw = TRUE, $_errorMessag
 
         // only TA managers are allowed to alter TS of closed TAs, but they have to confirm first that they really want to do it
         if ($_action != 'get') {
-            $timeaccount = Timetracker_Controller_Timeaccount::getInstance()->get($_record->timeaccount_id);
-            if (! $timeaccount->is_open) {
-                if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug(__METHOD__ . '::' . __LINE__
-                        . ' This Timeaccount is already closed!');
+            if ($isAdmin && ($this->_requestContext['skipClosedCheck'] ?? false)) {
+               return true;
+            }
 
-                if ($isAdmin === true) {
-                    if (is_array($this->_requestContext) && isset($this->_requestContext['skipClosedCheck']) && $this->_requestContext['skipClosedCheck']) {
-                        return true;
-                    }
-                }
+            $this->_validateRelations($_record, $_oldRecord);
 
-                if ($_throw) {
-                    throw new Timetracker_Exception_ClosedTimeaccount();
-                }
-                return FALSE;
-            }
-
             // check if timeaccount->is_billable is false => set default in fieldGrants to 0 and allow only managers to change it
             // if old record is billable, everybody can make it not billable
+            $timeaccount = Timetracker_Controller_Timeaccount::getInstance()->get($_record->timeaccount_id);
+
             if (!$timeaccount->is_billable && (!$_oldRecord || !$_oldRecord->is_billable)) {
                 $this->_fieldGrants['is_billable']['default'] = 0;
                 $this->_fieldGrants['is_billable']['requiredGrant'] = Tinebase_Model_Grants::GRANT_ADMIN;
@@ -662,4 +638,78 @@ public function checkFilterACL(Tinebase_Model_Filter_FilterGroup $_filter, $_act
                 throw new Timetracker_Exception_UnexpectedValue('Unknown action: ' . $_action);
         }
     }
+
+    /**
+     * attach tags hook
+     *
+     * @throws Tinebase_Exception_Record_NotAllowed
+     */
+    public function attachTagsHook($recordIds, $tagId)
+    {
+        $records = $this->getMultiple($recordIds);
+        $tag = Tinebase_Tags::getInstance()->getTagById($tagId);
+
+        foreach ($records as $record) {
+            $timeaccount = Timetracker_Controller_Timeaccount::getInstance()->get($record->timeaccount_id);
+            if ($timeaccount && !$timeaccount->is_open && $tag['name'] === Sales_Export_TimesheetTimeaccount::TAG_SUM) {
+                throw new Tinebase_Exception_SystemGeneric(
+                    'Is is not allowed to update tag : ' . $tag['name'] . ', when the related timeaccount is closed'
+                );
+            }
+        }
+    }
+
+    /**
+     * detach tags hook
+     *
+     */
+    public function detachTagsHook($recordIds, $tagId)
+    {
+        $records = $this->getMultiple($recordIds);
+        $tag = Tinebase_Tags::getInstance()->getTagById($tagId);
+
+        foreach ($records as $record) {
+            $timeaccount = Timetracker_Controller_Timeaccount::getInstance()->get($record->timeaccount_id);
+            if ($timeaccount && !$timeaccount->is_open && $tag['name'] === Sales_Export_TimesheetTimeaccount::TAG_SUM) {
+                throw new Tinebase_Exception_SystemGeneric(
+                    'Is is not allowed to update tag : ' . $tag['name'] . ', when the related timeaccount is closed'
+                );
+            }
+        }
+    }
+
+    protected function _validateRelations($_record, $_oldRecord)
+    {
+        $context = $this->getRequestContext();
+
+        if ($context && is_array($context) &&
+            (array_key_exists('clientData', $context) && array_key_exists('confirm', $context['clientData'])
+                || array_key_exists('confirm', $context))) {
+            if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug(__METHOD__ . '::' . __LINE__
+                . ' Force updating the timesheet');
+            return true;
+        }
+
+        $timeaccount = Timetracker_Controller_Timeaccount::getInstance()->get($_record->timeaccount_id);
+        $translation = Tinebase_Translation::getTranslation($this->_applicationName);
+        $exception = null;
+
+        if ($_oldRecord && $this->_isTSDateChanged($_record, $_oldRecord) && $_record->is_cleared && !empty($_record->invoice_id)) {
+            $exception = new Tinebase_Exception_Confirmation(
+                $translation->_('The Invoice you tried to edit is cleared already, change date will rebill the invoice, do you still want to execute this action?')
+            );
+        }
+
+        if (!$timeaccount->is_open) {
+            $exception = new Tinebase_Exception_Confirmation(
+                $translation->_('The related Timeaccount is already closed, do you still want to execute this action?')
+            );
+        }
+
+        if ($exception) {
+            throw $exception;
+        }
+
+        return true;
+    }
 }

tine20/Timetracker/translations/de.po

@@ -89,6 +89,9 @@ msgstr "Zeitkonten sind noch in Benutzung!, Sind Sie sicher, dass Sie sie lösch
 msgid "The Invoice you tried to edit is cleared already, change date will rebill the invoice, do you still want to execute this action?"
 msgstr "Die Rechnung die sie versucht haben zu bearbeiten ist bereits abgerechnet. Datumsänderungen führen zu erneuter Abrechnung. Wollen sie diese Aktion wirklich ausführen?"
 
+msgid "The related Timeaccount is already closed, do you still want to execute this action?"
+msgstr "Das verknüpfte Zeitkonto ist bereits geschlossen, Wollen sie diese Aktion wirklich ausführen?"
+
 msgid "Add Timesheet"
 msgstr "Stundenzettel hinzufügen"
 

tine20/Timetracker/translations/en.po

@@ -76,6 +76,9 @@ msgstr "Timeaccounts are still in use! Are you sure you want to delete them?"
 msgid "The Invoice you tried to edit is cleared already, change date will rebill the invoice, do you still want to execute this action?"
 msgstr "The Invoice you tried to edit is cleared already, change date will rebill the invoice, do you still want to execute this action?"
 
+msgid "The related Timeaccount is already closed, do you still want to execute this action?"
+msgstr "The related Timeaccount is already closed, do you still want to execute this action?"
+
 msgid "Add Timesheet"
 msgstr "Add Timesheet"
 

tine20/Timetracker/translations/template.pot

@@ -76,6 +76,9 @@ msgstr ""
 msgid "The Invoice you tried to edit is cleared already, change date will rebill the invoice, do you still want to execute this action?"
 msgstr ""
 
+msgid "The related Timeaccount is already closed, do you still want to execute this action?"
+msgstr ""
+
 msgid "Add Timesheet"
 msgstr ""
 

tine20/Tinebase/js/ExceptionHandler.js

@@ -302,7 +302,7 @@ Tine.Tinebase.ExceptionHandler = function() {
                     Ext.Msg.confirm(
                         exception.title,
                         exception.message,
-                        async function(button) {
+                    async function(button) {
                             if (button === 'yes') {
                                 Ext.Ajax.request({
                                     scope: callbackScope,
@@ -311,17 +311,8 @@ Tine.Tinebase.ExceptionHandler = function() {
                                     },
                                     params: params,
                                     success : function(_result, _request) {
-                                        Ext.callback(callback, callbackScope);
+                                        Ext.callback(callback, callbackScope, [_result, _request]);
                                     },
-                                    failure: function(exception) {
-                                        if (! exception.code && exception.responseText) {
-                                            // we need to decode the exception first
-                                            const response = Ext.util.JSON.decode(exception.responseText);
-                                            exception = response.data;
-                                            exception.request = Ext.util.JSON.encode(request);
-                                        }
-                                        Tine.Tinebase.ExceptionHandler.handleRequestException(exception);
-                                    }
                                 });
                             }
                         }