0011962: contract end_date can't be changed if vacation has been added

* adds workaround for json encoded field "workingtime_json"
* adds another test for updating contract end_date
* fixes "undefined index" if employee->contracts structure is not correct
 in json frontend

https://forge.tine20.org/view.php?id=11962

Change-Id: Ia18b9ce3c50b4fa60a36ab15d650a2bff3d63041
Reviewed-on: http://gerrit.tine20.com/customers/3271
Tested-by: Jenkins CI (http://ci.tine20.com/)
Reviewed-by: Philipp Schüle <p.schuele@metaways.de>

diff --git a/tests/tine20/HumanResources/JsonTests.php b/tests/tine20/HumanResources/JsonTests.php
index 38bf6df..12136d4 100644 (file)
--- a/tests/tine20/HumanResources/JsonTests.php
+++ b/tests/tine20/HumanResources/JsonTests.php
@@ -815,7 +815,6 @@ class HumanResources_JsonTests extends HumanResources_TestCase
      * test contract dates on update dependent
      * must not throw the HumanResources_Exception_ContractNotEditable exception
      */
-    
     public function testContractDates()
     {
         $employmentBegin = Tinebase_DateTime::now()->setDate(2014, 1, 2)->setTimezone(Tinebase_Core::getUserTimezone())->setTime(0,0,0);
@@ -846,6 +845,8 @@ class HumanResources_JsonTests extends HumanResources_TestCase
     
     /**
      * test adding a contract with manually setting the end_date of the contract before
+     *
+     * @see 0011962: contract end_date can't be changed if vacation has been added
      */
     public function testAddContract()
     {
@@ -897,16 +898,23 @@ class HumanResources_JsonTests extends HumanResources_TestCase
         // no exception should be thrown
         $employeeJson = $this->_json->saveEmployee($employeeJson);
         $this->assertEquals(2, count($employeeJson['contracts']));
+
+        $endDate = '2013-05-30 00:00:00';
+        $employeeJson['contracts'][0]['end_date'] = $endDate;
+        $employeeJson['contracts'][0]['workingtime_json'] = '{"days": [8,8,8,8,8,0,0]}';
+        $recordData = $this->_json->saveEmployee($employeeJson);
+        $this->assertEquals($endDate, $recordData['contracts'][0]['end_date']);
         
-        // an exception should be thrown
         $employeeJson['contracts'][0]['vacation_days'] = 31;
-        $this->setExpectedException('HumanResources_Exception_ContractNotEditable');
-        
-        $employeeJson = $this->_json->saveEmployee($employeeJson);
+        try {
+            $employeeJson = $this->_json->saveEmployee($employeeJson);
+            $this->fail('an exception should be thrown');
+        } catch (Exception $e) {
+            $this->assertTrue($e instanceof HumanResources_Exception_ContractNotEditable);
+        }
     }
     
     /**
-<<<<<<< HEAD
      * @see: https://forge.tine20.org/mantisbt/view.php?id=10122
      */
     public function testAlternatingContracts()

diff --git a/tine20/HumanResources/Controller/Contract.php b/tine20/HumanResources/Controller/Contract.php
index ca96901..67ea465 100644 (file)
--- a/tine20/HumanResources/Controller/Contract.php
+++ b/tine20/HumanResources/Controller/Contract.php
@@ -77,10 +77,17 @@ class HumanResources_Controller_Contract extends Tinebase_Controller_Record_Abst
         }
         
         $diff = $_record->diff($_oldRecord, array(
-            'created_by', 'creation_time', 'last_modified_by', 'last_modified_time', 'notes', 'end_date', 'seq', 'tags'
+            'created_by', 'creation_time', 'last_modified_by', 'last_modified_time', 'notes', 'end_date', 'seq', 'tags',
+            // see 0011962: contract end_date can't be changed if vacation has been added
+            // TODO fix json encoded field diff - this is only a workaround
+            // sadly, there is currently no test that breaks without this hotfix :(
+            'workingtime_json'
         ))->diff;
 
         if (! empty($diff)) {
+            if (Tinebase_Core::isLogLevel(Zend_Log::DEBUG)) Tinebase_Core::getLogger()->debug(
+                __METHOD__ . '::' . __LINE__ . " Contract diff:" . print_r($diff, true));
+
             if ($this->getFreeTimes($_record)->filter('type', 'vacation')->count() > 0) {
                 throw new HumanResources_Exception_ContractNotEditable();
             }
@@ -110,14 +117,16 @@ class HumanResources_Controller_Contract extends Tinebase_Controller_Record_Abst
         $freeTimeFilter->addFilter(new Tinebase_Model_Filter_Text(
             array('field' => 'employee_id', 'operator' => 'equals', 'value' => $contract->employee_id)
         ));
+
+        if (Tinebase_Core::isLogLevel(Zend_Log::TRACE)) Tinebase_Core::getLogger()->trace(
+            __METHOD__ . '::' . __LINE__ . " FreeTime filter:" . print_r($freeTimeFilter->toArray(), true));
         
         $freeTimeController = HumanResources_Controller_FreeTime::getInstance();
         $results = $freeTimeController->search($freeTimeFilter);
         
         return $results;
     }
-    
-    
+
     /**
      * checks the start_date and end_date
      * 
@@ -134,6 +143,7 @@ class HumanResources_Controller_Contract extends Tinebase_Controller_Record_Abst
             throw new HumanResources_Exception_ContractDates();
         }
     }
+
     /**
      * resolves the container array to the corresponding id
      * 

diff --git a/tine20/HumanResources/Frontend/Json.php b/tine20/HumanResources/Frontend/Json.php
index f8fdc87..416ee51 100644 (file)
--- a/tine20/HumanResources/Frontend/Json.php
+++ b/tine20/HumanResources/Frontend/Json.php
@@ -212,7 +212,7 @@ class HumanResources_Frontend_Json extends Tinebase_Frontend_Json_Abstract
         if (! empty($recordData['contracts'])) {
             
             for ($i = 0; $i < count($recordData['contracts']); $i++) {
-                if (is_array($recordData['contracts'][$i]['feast_calendar_id'])) {
+                if (isset($recordData['contracts'][$i]) && is_array($recordData['contracts'][$i]['feast_calendar_id'])) {
                     $recordData['contracts'][$i]['feast_calendar_id'] = $recordData['contracts'][$i]['feast_calendar_id']['id'];
                 }
             }