fix dolibarr sync execution

src/Service/Dolibarr/DolibarrSyncService.php

@@ -80,6 +80,8 @@ class DolibarrSyncService
         // Loop over the Opentalent organizations, and fill up the operations list
         $operations = [];
         foreach ($dolibarrClientsIndex as $organizationId => $dolibarrSociety) {
+            $dolibarrSociety = $this->sanitizeDolibarrData($dolibarrSociety);
+
             $organization = $this->organizationRepository->find($organizationId);
             if ($organization === null) {
                 $this->logger->error("Organization " . $organizationId . " not found in the Opentalent DB");
@@ -94,7 +96,7 @@ class DolibarrSyncService
             $newSocietyData = [];
 
             // Sync name
-            $newSocietyData['name'] = $organization->getName();
+            $newSocietyData['name'] = trim($organization->getName());
 
             // Sync contact data of the client
             $mainAddress = $this->getOrganizationPostalAddress($organization);
@@ -145,11 +147,7 @@ class DolibarrSyncService
             $newSocietyData['array_options']['options_2iopeninfoopentalent'] = implode('\n', $infos);
 
             // Only update the fields that are different
-            $newSocietyData = array_filter(
-                $newSocietyData,
-                function($v, $k) use ($dolibarrSociety) { return $v !== $dolibarrSociety[$k]; },
-                ARRAY_FILTER_USE_BOTH
-            );
+            $newSocietyData = $this->filterDiff($dolibarrSociety, $newSocietyData);
 
             // Add an update operation if some data has to be updated
             if (!empty($newSocietyData)) {
@@ -161,7 +159,6 @@ class DolibarrSyncService
                 );
             }
 
-
             // ===== Update Contacts =====
             $dolibarrContactsIndex = $this->getDolibarrContactsIndex((int)$dolibarrSociety['id']);
             $contactsProcessed = [];
@@ -186,23 +183,14 @@ class DolibarrSyncService
 
                         // Build parameters for the query (if a query is needed
                         $dolibarrContact = $dolibarrContactsIndex[$person->getId()] ?? null;
-
-                        // <-- special case: for phone numbers, dolibarr api returns empty strings even when field is null in DB
-                        // if left like this, it would lead to unnecessary updates
-                        if ($dolibarrContact !== null) {
-                            if ($dolibarrContact['phone_pro'] === '')
-                                $dolibarrContact['phone_pro'] = null;
-                            if ($dolibarrContact['phone_mobile'] === '')
-                                $dolibarrContact['phone_mobile'] = null;
-                        }
-                        // -->
+                        $dolibarrContact = $this->sanitizeDolibarrData($dolibarrContact);
 
                         $contact = $this->getPersonContact($person);
 
                         $newContactData = [
                             'civility_code' => $person->getGender() ? $this->translator->trans($person->getGender()) : null,
-                            'lastname' => $person->getName(),
-                            'firstname' => $person->getGivenName(),
+                            'lastname' => trim($person->getName()),
+                            'firstname' => trim($person->getGivenName()),
                             'email' => $contact?->getEmail(),
                             'phone_pro' => $contact?->getTelphone() ? $this->formatPhoneNumber($contact?->getTelphone()) : null,
                             'phone_mobile' => $contact?->getMobilPhone() ? $this->formatPhoneNumber($contact?->getMobilPhone()): null,
@@ -210,18 +198,20 @@ class DolibarrSyncService
                         ];
 
                         if ($dolibarrContact === null) {
+                            // New contact
+                            $newContactData['socid'] = (int)$dolibarrSociety['id'];
+                            $newContactData['array_options'] = [
+                                'options_2iopen_person_id' => $person->getId()
+                            ];
+
                             $operations[] = new CreateOperation(
                                 'New contact: ' . $person->getName() . ' ' . $person->getGivenName() . ' (' . $person->getId() . ')',
                                 'contacts',
                                 $newContactData
                             );
                         } else {
-                            // Only update the fields that are different from existing
-                            $newContactData = array_filter(
-                                $newContactData,
-                                function($v, $k) use ($dolibarrContact) { return $v !== $dolibarrContact[$k]; },
-                                ARRAY_FILTER_USE_BOTH
-                            );
+                            // Only update the fields that are different
+                            $newContactData = $this->filterDiff($dolibarrContact, $newContactData);
 
                             // add an update operation if some data has to be updated
                             if (!empty($newContactData)) {
@@ -263,7 +253,7 @@ class DolibarrSyncService
         foreach ($operations as $operation) {
             $this->logger->debug($operation->getLabel());
             foreach ($operation->getChangeLog() as $message) {
-                $this->logger->debug($message);
+                $this->logger->debug('   ' . $message);
             }
         }
 
@@ -397,6 +387,32 @@ class DolibarrSyncService
         return $index;
     }
 
+    /**
+     * Because for some fields the dolibarr api returns empty strings even when field is null in DB,
+     * we have to post-process it to avoid unnecessary and endless update operations
+     *
+     * As far as we know, there is no harm here to replace every empty string value by a null value
+     * (no loss of information)
+     *
+     * @param array|null $data
+     * @return array|null
+     */
+    private function sanitizeDolibarrData(?array $data): ?array {
+        if ($data === null)
+            return null;
+
+        foreach ($data as $field => $value) {
+            if (is_array($value)) {
+                $data[$field] = $this->sanitizeDolibarrData($value);
+            } else {
+                if ($value === '') {
+                    $data[$field] = null;
+                }
+            }
+        }
+        return $data;
+    }
+
     /**
      * Retrieve the postal address of the organization
      *
@@ -516,7 +532,7 @@ class DolibarrSyncService
      */
     private function formatContactPosition(array $missions, ?string $gender = 'X'): string {
         $to_exclude = [FunctionEnum::ADHERENT(), FunctionEnum::STUDENT(), FunctionEnum::OTHER()];
-        return implode(
+        $poste = implode(
             ', ',
             array_map(
                 function($m) use ($gender) {
@@ -536,6 +552,11 @@ class DolibarrSyncService
                 )
             )
         );
+
+        if (strlen($poste) > 80) {
+            $poste = substr($poste, 0, 77) . '...';
+        }
+        return $poste;
     }
 
     /**
@@ -548,4 +569,29 @@ class DolibarrSyncService
         $phoneUtil = PhoneNumberUtil::getInstance();
         return $phoneUtil->format($phoneNumber, PhoneNumberFormat::INTERNATIONAL);
     }
+
+    /**
+     * Returns an array containing the keys/values from the newData array
+     * which are absent or different from $initialData
+     *
+     * Because for some fields the dolibarr api returns empty strings even when field is null in DB,
+     * we have to consider null and empty-string as equals. As far as we know, this causes no loss of information.
+     *
+     * @param array $initialData
+     * @param array $newData
+     * @return array
+     */
+    private function filterDiff(array $initialData, array $newData) {
+        $result = [];
+        foreach ($newData as $field => $value) {
+            if (is_array($value)) {
+                $data[$field] = $this->filterDiff($initialData[$field] ?? [], $value);
+            } else {
+                if (($value ?? '') !== ($initialData[$field] ?? '') || !array_key_exists($field, $initialData)) {
+                    $result[$field] = $value;
+                }
+            }
+        }
+        return $result;
+    }
 }

src/Service/Rest/Operation/BaseRestOperation.php

@@ -28,18 +28,27 @@ abstract class BaseRestOperation
     protected string $method;
     protected string $path;
     protected array $parameters;
+    protected array $options;
     protected array $currentData;
     protected string $errorMessage = "";
 
     #[Required]
     public function setApiRequestService(ApiRequestService $apiService) { $this->apiService = $apiService; }
 
-    public function __construct(string $label, string $method, string $path, array $parameters = [], array $currentData = []) {
+    public function __construct(
+        string $label,
+        string $method,
+        string $path,
+        array $currentData = [],
+        array $parameters = [],
+        array $options = []
+    ) {
         $this->label = $label;
         $this->method = $method;
         $this->path = $path;
-        $this->parameters = $parameters;
         $this->currentData = $currentData;
+        $this->parameters = $parameters;
+        $this->options = $options;
     }
 
     /**
@@ -48,7 +57,12 @@ abstract class BaseRestOperation
     public function execute() {
         $this->status = self::STATUS_PENDING;
         try {
-            $this->apiService->request($this->method, $this->path, $this->parameters);
+            $response = $this->apiService->request($this->method, $this->path, $this->parameters, $this->options);
+            if ($response->getStatusCode() !== 200) {
+                $this->status = self::STATUS_ERROR;
+                $this->errorMessage = 'Error ' . $response->getStatusCode() . ' : ' . $response->getContent();
+                return;
+            }
             $this->status = self::STATUS_DONE;
         } catch (ClientExceptionInterface | TransportExceptionInterface | RedirectionExceptionInterface | ServerExceptionInterface $e) {
             $this->status = self::STATUS_ERROR;
@@ -96,6 +110,14 @@ abstract class BaseRestOperation
         return $this->parameters;
     }
 
+    /**
+     * @return array
+     */
+    public function getOptions(): array
+    {
+        return $this->options;
+    }
+
     /**
      * @return string
      */

src/Service/Rest/Operation/CreateOperation.php

@@ -11,14 +11,19 @@ use JetBrains\PhpStorm\Pure;
 class CreateOperation extends BaseRestOperation
 {
     protected string $entity;
+    protected array $data;
 
     #[Pure]
-    public function __construct(string $label, string $entity, array $parameters) {
+    public function __construct(string $label, string $entity, array $data, array $parameters = [], array $options = []) {
+        $this->data = $data;
+        $options['json'] = $this->data;
         parent::__construct(
             $label,
             'POST',
             $entity,
-            $parameters
+            [],
+            $parameters,
+            $options
         );
         $this->entity = $entity;
     }
@@ -32,8 +37,14 @@ class CreateOperation extends BaseRestOperation
         $messages = [
             '[POST ' . $this->entity . ']'
         ];
-        foreach ($this->parameters as $field => $newValue) {
-            $messages[] = $field . ' : `' . $newValue . '`';
+        foreach ($this->data as $field => $newValue) {
+            if (is_array($newValue)) {
+                foreach ($newValue as $subField => $newSubValue) {
+                    $messages[] = $field . '.' . $subField . ' : `' . $newSubValue . '`';
+                }
+            } else {
+                $messages[] = $field . ' : `' . $newValue . '`';
+            }
         }
         return $messages;
     }

src/Service/Rest/Operation/DeleteOperation.php

@@ -14,13 +14,16 @@ class DeleteOperation extends BaseRestOperation
     protected int $id;
 
     #[Pure]
-    public function __construct(string $label, string $entity, array $current) {
+    public function __construct(string $label, string $entity, array $current, array $options = []) {
         $id = (int)$current['id'];
 
         parent::__construct(
             $label,
             'DELETE',
-            $entity . '/' . $id
+            $entity . '/' . $id,
+            [],
+            [],
+            $options
         );
 
         $this->entity = $entity;

src/Service/Rest/Operation/UpdateOperation.php

@@ -12,17 +12,21 @@ class UpdateOperation extends BaseRestOperation
 {
     protected string $entity;
     protected int $id;
+    protected array $data;
 
     #[Pure]
-    public function __construct(string $label, string $entity, array $current, array $parameters) {
+    public function __construct(string $label, string $entity, array $current, array $data, array $parameters = [], array $options = []) {
         $id = (int)$current['id'];
+        $this->data = $data;
+        $options['json'] = $this->data;
 
         parent::__construct(
             $label,
             'PUT',
             $entity . '/' . $id,
+            $current,
             $parameters,
-            $current
+            $options
         );
 
         $this->entity = $entity;
@@ -40,7 +44,7 @@ class UpdateOperation extends BaseRestOperation
         $messages = [
             '[PUT ' . $this->entity . '/' . $this->id . ']'
         ];
-        foreach ($this->parameters as $field => $newValue) {
+        foreach ($this->data as $field => $newValue) {
             if (!array_key_exists($field, $this->currentData)) {
                 throw new \Exception('Field does not exists in the current object data : ' . $field);
             }