0010176: Dependent Records: Do not try to normalize id of existing records
authorAlexander Stintzing <a.stintzing@metaways.de>
Thu, 28 Aug 2014 11:31:48 +0000 (13:31 +0200)
committerPhilipp Schüle <p.schuele@metaways.de>
Wed, 10 Sep 2014 11:10:07 +0000 (13:10 +0200)
Do not try to normalize id of existing records if the dependent record has
been created already.

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

Change-Id: Ibf992e96fb093bc74e18e357472c5710e0ec0516
Reviewed-on: http://gerrit.tine20.com/customers/1058
Tested-by: Jenkins CI (http://ci.tine20.com/)
Reviewed-by: Philipp Schüle <p.schuele@metaways.de>
tests/tine20/HumanResources/JsonTests.php
tests/tine20/HumanResources/TestCase.php
tine20/Tinebase/Controller/Record/Abstract.php

index fc8eec2..5f0db12 100644 (file)
@@ -76,7 +76,6 @@ class HumanResources_JsonTests extends HumanResources_TestCase
         
         $savedEmployee['contracts'][]   = $newContract->toArray();
         $savedEmployee['costcenters'][] = $costCenter2->toArray();
-        
         $savedEmployee = $this->_json->saveEmployee($savedEmployee);
         
         $this->assertEquals(2, count($savedEmployee['contracts']),   'There should be 2 Contracts');
@@ -901,6 +900,7 @@ class HumanResources_JsonTests extends HumanResources_TestCase
     }
     
     /**
+<<<<<<< HEAD
      * @see: https://forge.tine20.org/mantisbt/view.php?id=10122
      */
     public function testAlternatingContracts()
@@ -1084,4 +1084,53 @@ class HumanResources_JsonTests extends HumanResources_TestCase
         $this->assertEquals(14, $account['excused_sickness']);
         $this->assertEquals(1, $account['unexcused_sickness']);
     }
+    
+    /**
+     * @see: https://forge.tine20.org/mantisbt/view.php?id=10176
+     */
+    public function testSavingRelatedRecord()
+    {
+        $date = new Tinebase_DateTime();
+        $e = $this->_getEmployee();
+        $c = $this->_getContract($date);
+        // in fe the record gets an id, to allow stores crud and sort actions. this must be set to a 40 length ssha key
+        $c->id = '1234567890';
+        
+        $employeeJson = $e->toArray();
+        $employeeJson['contracts'] = array($c->toArray());
+        
+        $employeeJson = $this->_json->saveEmployee($employeeJson);
+        
+        $id = $employeeJson['contracts'][0]['id'];
+        // the id should be set to a 40 length ssha key
+        $this->assertEquals(40, strlen($id));
+        
+        $employeeJson = $this->_json->saveEmployee($employeeJson);
+        $this->assertEquals($id, $employeeJson['contracts'][0]['id']);
+    }
+    
+    /**
+     * @see: https://forge.tine20.org/mantisbt/view.php?id=10176
+     */
+    public function testSavingRelatedRecordWithCorruptId()
+    {
+        $date = new Tinebase_DateTime();
+        $e = $this->_getEmployee();
+        $c = $this->_getContract($date);
+        $c->id = '1234567890';
+    
+        $employeeJson = $e->toArray();
+        $employeeJson = $this->_json->saveEmployee($employeeJson);
+    
+        $c->employee_id = $employeeJson['id'];
+        
+        $c = HumanResources_Controller_Contract::getInstance()->create($c);
+        $this->assertEquals('1234567890', $c->getId());
+
+        $employeeJson['contracts'] = array($c->toArray());
+        $employeeJson = $this->_json->saveEmployee($employeeJson);
+        
+        // if it has been corrupted before this change was committed, the corrupted id should stay
+        $this->assertEquals('1234567890', $employeeJson['contracts'][0]['id']);
+    }
 }
index ee36e53..2ee34c2 100644 (file)
@@ -58,14 +58,14 @@ class HumanResources_TestCase extends PHPUnit_Framework_TestCase
      */
     protected function setUp()
     {
-        Tinebase_TransactionManager::getInstance()->startTransaction(Tinebase_Core::getDb());
-        
         // remove employees and costcenters, if there are some already
         $filter = new HumanResources_Model_EmployeeFilter(array());
         HumanResources_Controller_Employee::getInstance()->deleteByFilter($filter);
         
         $filter = new Sales_Model_CostCenterFilter(array());
         Sales_Controller_CostCenter::getInstance()->deleteByFilter($filter);
+        
+        Tinebase_TransactionManager::getInstance()->startTransaction(Tinebase_Core::getDb());
     }
 
     /**
index e503ddd..02e3d60 100644 (file)
@@ -1842,6 +1842,9 @@ abstract class Tinebase_Controller_Record_Abstract
             
             foreach ($_record->{$_property} as $record) {
                 $record->{$_fieldConfig['refIdField']} = $_createdRecord->getId();
+                if (! $record->getId() || strlen($record->getId()) != 40) {
+                    $record->{$record->getIdProperty()} = NULL;
+                }
                 $new->add($controller->create($record));
             }
     
@@ -1863,7 +1866,11 @@ abstract class Tinebase_Controller_Record_Abstract
             return;
         }
         
-        // don't handle dependet records on property if it is set to null or doesn't exist.
+        if (! isset ($_fieldConfig['refIdField'])) {
+            throw new Tinebase_Exception_Record_DefinitionFailure('If a record is dependent, a refIdField has to be defined!');
+        }
+        
+        // don't handle dependent records on property if it is set to null or doesn't exist.
         if (($_record->{$_property} === NULL) || (! $_record->has($_property))) {
             if (Tinebase_Core::isLogLevel(Zend_Log::INFO)) {
                 Tinebase_Core::getLogger()->info(__METHOD__ . '::' . __LINE__. ' Skip updating dependent records (got NULL) on property ' . $_property . ' for ' . $this->_applicationName . ' ' . $this->_modelName . ' with id = "' . $_record->getId() . '"');
@@ -1900,7 +1907,9 @@ abstract class Tinebase_Controller_Record_Abstract
             $oldRecords = $controller->search($oldFilter);
             
             foreach ($_record->{$_property} as $record) {
-                $record->{$_fieldConfig['refIdField']} = $_oldRecord->getId();
+                
+                $record->{$_fieldConfig['refIdField']} = $_record->getId();
+                
                 // update record if ID exists and has a length of 40 (it has a length of 10 if it is a timestamp)
                 if ($record->getId() && strlen($record->getId()) == 40) {
                     
@@ -1915,13 +1924,37 @@ abstract class Tinebase_Controller_Record_Abstract
                     } else {
                         $existing->addRecord($record);
                     }
-                    // create if ID does not exist or has not a length of 40
+                    // create if is not existing already
                 } else {
-                    $record->{$record->getIdProperty()} = NULL;
-                    $crc = $controller->create($record);
-                    $existing->addRecord($crc);
-                    if (Tinebase_Core::isLogLevel(Zend_Log::INFO)) {
-                        Tinebase_Core::getLogger()->info(__METHOD__ . '::' . __LINE__. ' Creating dependent record with id = "' . $crc->getId() . '" on property ' . $_property . ' for ' . $this->_applicationName . ' ' . $this->_modelName);
+                    // try to find if it already exists (with corrupted id)
+                    if ($record->getId() == NULL) {
+                        $crc = $controller->create($record);
+                        $existing->addRecord($crc);
+                        
+                        if (Tinebase_Core::isLogLevel(Zend_Log::INFO)) {
+                            Tinebase_Core::getLogger()->info(__METHOD__ . '::' . __LINE__. ' Creating dependent record with id = "' . $crc->getId() . '" on property ' . $_property . ' for ' . $this->_applicationName . ' ' . $this->_modelName);
+                        }
+                    } else {
+                        
+                        try {
+                            
+                            $prevRecord = $controller->get($record->getId());
+    
+                            if (! empty($prevRecord->diff($record)->diff)) {
+                                $existing->addRecord($controller->update($record));
+                            } else {
+                                $existing->addRecord($record);
+                            }
+                            
+                        } catch (Tinebase_Exception_NotFound $e) {
+                            $record->id = NULL;
+                            $crc = $controller->create($record);
+                            $existing->addRecord($crc);
+                            
+                            if (Tinebase_Core::isLogLevel(Zend_Log::INFO)) {
+                                Tinebase_Core::getLogger()->info(__METHOD__ . '::' . __LINE__. ' Creating dependent record with id = "' . $crc->getId() . '" on property ' . $_property . ' for ' . $this->_applicationName . ' ' . $this->_modelName);
+                            }
+                        }
                     }
                 }
             }
@@ -1945,4 +1978,9 @@ abstract class Tinebase_Controller_Record_Abstract
         }
         $_record->{$_property} = $existing->toArray();
     }
+    
+    protected function _createDependentRecord($record, $_record, $_fieldConfig, $controller, $recordSet)
+    {
+        
+    }
 }