fix(Addressbook): save duplicate contact with image failed

... because the jpegphoto could not be converted to json we now just skip the photo when adding the record to the duplicate exception. it is also possible to define more fields that should be unset in the TED client record Change-Id: I38a6153304081c5add19b8735a232365cab92b7c Reviewed-on: http://gerrit.tine20.com/customers/15995 Tested-by: Jenkins CI (http://ci.tine20.com/) <tine20-jenkins@metaways.de> Reviewed-by: Philipp Schüle <p.schuele@metaways.de>
tine20 · Mar 5, 2020 · 2925198 · 2925198
1 parent 957ee6b
commit 2925198
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 11 deletions.
diff --git a/tests/tine20/Addressbook/JsonTest.php b/tests/tine20/Addressbook/JsonTest.php
@@ -4,7 +4,7 @@
  *
  * @package     Addressbook
  * @license     http://www.gnu.org/licenses/agpl.html
- * @copyright   Copyright (c) 2008-2019 Metaways Infosystems GmbH (http://www.metaways.de)
+ * @copyright   Copyright (c) 2008-2020 Metaways Infosystems GmbH (http://www.metaways.de)
  * @author      Lars Kneschke <l.kneschke@metaways.de>
  *
  */
@@ -2542,6 +2542,24 @@ public function testSaveContactWithAreaLockedRelation()
     }
 
     public function testSetImage()
+    {
+        $contact = $this->_getContactWithImage();
+        $savedContactWithImage = $this->_uit->saveContact($contact);
+
+        // save contact again
+        $savedContactWithImageAgain = $this->_uit->saveContact($savedContactWithImage);;
+
+        // check if image is still there
+        self::assertTrue(isset($savedContactWithImageAgain['jpegphoto']));
+        self::assertEquals($savedContactWithImage['jpegphoto'], $savedContactWithImageAgain['jpegphoto'],
+            'image should not change!');
+        return $savedContactWithImageAgain;
+    }
+
+    /**
+     * @return array
+     */
+    protected function _getContactWithImage()
     {
         // create tempfile
         $tempFileBackend = new Tinebase_TempFile();
@@ -2552,16 +2570,27 @@ public function testSetImage()
         $contact = $this->_getContactData();
         $contact['jpegphoto'] = 'index.php?method=Tinebase.getImage&application=Tinebase&location=tempFile&id='
             . $tempFile->getId() . '&width=88&height=118&ratiomode=0&mtime=1546880445806';
-        $savedContactWithImage = $this->_uit->saveContact($contact);
+        return $contact;
+    }
 
-        // save contact again
-        $savedContactWithImageAgain = $this->_uit->saveContact($savedContactWithImage);;
+    public function testSetImageCreateDuplicateContact()
+    {
+        $contact = $this->_getContactWithImage();
+        // let's throw a duplicate exception
+        $contact['email'] = Tinebase_Core::getUser()->accountEmailAddress;
+        try {
+            $result = $this->_uit->saveContact($contact);
+            self::fail('duplicate exception expected');
+        } catch (Tinebase_Exception_Duplicate $ted) {
+            try {
+                $jsonResponse = json_encode($ted->toArray());
+            } catch (Throwable $e) {
+                // pre php 7.2
+                $jsonResponse = false;
+            }
+            self::assertNotFalse($jsonResponse);
+        }
 
-        // check if image is still there
-        self::assertTrue(isset($savedContactWithImageAgain['jpegphoto']));
-        self::assertEquals($savedContactWithImage['jpegphoto'], $savedContactWithImageAgain['jpegphoto'],
-            'image should not change!');
-        return $savedContactWithImageAgain;
     }
 
     public function testRemoveImage()

diff --git a/tine20/Addressbook/Model/Contact.php b/tine20/Addressbook/Model/Contact.php
@@ -462,7 +462,7 @@ class Addressbook_Model_Contact extends Tinebase_Record_NewAbstract
                     Zend_Filter_Input::ALLOW_EMPTY      => true,
                 ],
                 self::OMIT_MOD_LOG              => true,
-                self::SYSTEM                    => true
+                self::SYSTEM                    => true,
             ],
             'note'                          => [
                 self::TYPE                      => self::TYPE_FULLTEXT,
@@ -1205,4 +1205,11 @@ public function resolveAttenderCleanUp()
             'account_id'  => true,
         ]);
     }
+
+    public function unsetFieldsBeforeConvertingToJson()
+    {
+        parent::unsetFieldsBeforeConvertingToJson();
+
+        unset($this->jpegphoto);
+    }
 }
diff --git a/tine20/Tinebase/Exception/Duplicate.php b/tine20/Tinebase/Exception/Duplicate.php
@@ -44,6 +44,11 @@ public function __construct($_message = 'data exception', $_code = 629)
      */
     public function setClientRecord(Tinebase_Record_Interface $_record)
     {
+        if (method_exists($_record, 'unsetFieldsBeforeConvertingToJson')) {
+            // may need to unset some fields because record is converted to json
+            $_record->unsetFieldsBeforeConvertingToJson();
+        }
+
         $this->_clientRecord = $_record;
     }
 

diff --git a/tine20/Tinebase/Record/NewAbstract.php b/tine20/Tinebase/Record/NewAbstract.php
@@ -1585,4 +1585,14 @@ public function getNotesTranslatedText()
     {
         return $this->getTitle();
     }
-}
+
+    /**
+     * can be used to remove fields that can't be converted to json
+     *
+     * @todo add this to model config (field config) and just loop the fields here?
+     * @todo move this to TRInterface + TRAbstract?
+     */
+    public function unsetFieldsBeforeConvertingToJson()
+    {
+    }
+}