Merge branch 'feature/V8-4013-revoir-synchronisation-clients-o' into develop

src/Service/Dolibarr/DolibarrSyncService.php

@@ -33,12 +33,6 @@ use libphonenumber\PhoneNumberFormat;
 use libphonenumber\PhoneNumberUtil;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\HttpKernel\Exception\HttpException;
-use Symfony\Contracts\HttpClient\Exception\ClientExceptionInterface;
-use Symfony\Contracts\HttpClient\Exception\DecodingExceptionInterface;
-use Symfony\Contracts\HttpClient\Exception\RedirectionExceptionInterface;
-use Symfony\Contracts\HttpClient\Exception\ServerExceptionInterface;
-use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
-use Symfony\Contracts\HttpClient\ResponseInterface;
 use Symfony\Contracts\Service\Attribute\Required;
 use Symfony\Contracts\Translation\TranslatorInterface;
 
@@ -309,6 +303,7 @@ class DolibarrSyncService
                 }
             }
 
+            // Disable non existing contacts
             foreach ($dolibarrSocietyContacts as $contactData) {
                 if (empty($contactData['array_options']['options_2iopen_person_id'])) {
                     continue;
@@ -336,6 +331,49 @@ class DolibarrSyncService
                 }
             }
 
+            // Update Billing service contact (if existing)
+            foreach ($dolibarrSocietyContacts as $contactData) {
+                if (
+                    'service facturation' === strtolower($contactData['lastname'])
+                    && empty($contactData['name'])
+                    && empty($contactData['array_options']['options_2iopen_person_id'])
+                ) {
+                    $billingAddress = $this->getOrganizationBillingPostalAddress($organization);
+                    $newContactData = $contactData;
+
+                    if ($billingAddress) {
+                        $newContactData['address'] = $this->addressPostalUtils->getFullStreetAddress($billingAddress);
+                        $newContactData['zip'] = $mainAddress->getPostalCode();
+                        $newContactData['town'] = $mainAddress->getAddressCity();
+                    } else {
+                        $newContactData['address'] = null;
+                        $newContactData['zip'] = null;
+                        $newContactData['town'] = null;
+                    }
+
+                    // Only update the fields that are different (it's important to let it non-recursive, the subarray have to be passed entirely)
+                    $newContactData = $this->arrayUtils->getChanges(
+                        $contactData,
+                        $newContactData,
+                        false,
+                        static function ($v1, $v2) { return ($v1 ?? '') === ($v2 ?? ''); }
+                    );
+
+                    // add an update operation if some data has to be updated
+                    if (!empty($newContactData)) {
+                        $operations[] = new UpdateOperation(
+                            'Update contact: Service facturation'.
+                            ' in '.$organization->getName().' ('.$organization->getId().')',
+                            'contacts',
+                            (int) $contactData['id'],
+                            $newContactData,
+                            $contactData
+                        );
+                    }
+                    break;
+                }
+            }
+
             // ===== Update Tags =====
             $currentTags = $this->dolibarrApiService->getSocietyTagsIds($dolibarrSocietyId);
             $expectedTags = $this->getExpectedTagsFor($organization);
@@ -429,15 +467,6 @@ class DolibarrSyncService
                     throw new \RuntimeException('Operation has an inconsistent status : '.$operation->getStatus());
                 }
 
-                // If this is an update operation, validate the result
-                if ($operation instanceof UpdateOperation) {
-                    try {
-                        $this->validateResponse($response, $operation);
-                    } catch (\RuntimeException $e) {
-                        $this->logger->warning($e->getMessage());
-                    }
-                }
-
                 ++$done;
             } catch (\RuntimeException $e) {
                 $this->logger->error('Error while executing operation : '.$operation);
@@ -607,9 +636,9 @@ class DolibarrSyncService
     protected function getOrganizationPostalAddress(Organization $organization): ?AddressPostal
     {
         $addressPriorities = [
+            AddressPostalOrganizationTypeEnum::ADDRESS_HEAD_OFFICE,
             AddressPostalOrganizationTypeEnum::ADDRESS_BILL,
             AddressPostalOrganizationTypeEnum::ADDRESS_CONTACT,
-            AddressPostalOrganizationTypeEnum::ADDRESS_HEAD_OFFICE,
             AddressPostalOrganizationTypeEnum::ADDRESS_PRACTICE,
             AddressPostalOrganizationTypeEnum::ADDRESS_OTHER,
         ];
@@ -627,6 +656,31 @@ class DolibarrSyncService
         return null;
     }
 
+    /**
+     * Retrieve the postal address of the organization.
+     */
+    protected function getOrganizationBillingPostalAddress(Organization $organization): ?AddressPostal
+    {
+        $addressPriorities = [
+            AddressPostalOrganizationTypeEnum::ADDRESS_BILL,
+            AddressPostalOrganizationTypeEnum::ADDRESS_CONTACT,
+            AddressPostalOrganizationTypeEnum::ADDRESS_HEAD_OFFICE,
+            AddressPostalOrganizationTypeEnum::ADDRESS_OTHER,
+        ];
+
+        $organizationAddressPostal = $organization->getOrganizationAddressPostals();
+
+        foreach ($addressPriorities as $addressType) {
+            foreach ($organizationAddressPostal as $postalAddress) {
+                if ($postalAddress->getType() === $addressType) {
+                    return $postalAddress->getAddressPostal();
+                }
+            }
+        }
+
+        return null;
+    }
+
     /**
      * Retrieve the phone for the organization.
      */
@@ -824,34 +878,4 @@ class DolibarrSyncService
 
         return $expectedTags;
     }
-
-    /**
-     * Post-validation of the execution of the operation.
-     * Compare the actual result to the expected one to ensure that the data was correctly updated.
-     *
-     * In the case of a validation error, throw an HttpException
-     *
-     * @throws \RuntimeException
-     */
-    protected function validateResponse(ResponseInterface $response, UpdateOperation|CreateOperation $operation): void
-    {
-        $updated = $operation->getData();
-
-        try {
-            $responseData = $response->toArray();
-        } catch (ClientExceptionInterface|DecodingExceptionInterface|RedirectionExceptionInterface|ServerExceptionInterface|TransportExceptionInterface $e) {
-            throw new \RuntimeException("Couldn't read the content of the response : ".$e);
-        }
-
-        // Sanitize to get rid of the null / empty strings transformations of the API
-        $updated = $this->sanitizeDolibarrData($updated);
-        $responseData = $this->sanitizeDolibarrData($responseData);
-
-        $diffs = $this->arrayUtils->getChanges($responseData, $updated, true);
-
-        if (!empty($diffs)) {
-            /* @noinspection JsonEncodingApiUsageInspection */
-            throw new \RuntimeException('The '.$operation->getMethod()." request had an unexpected result.\n".'Expected content: '.json_encode($updated)."\n".'Actual content  : '.json_encode($responseData));
-        }
-    }
 }

tests/Unit/Service/Dolibarr/DolibarrSyncServiceTest.php

@@ -41,7 +41,6 @@ use libphonenumber\PhoneNumberUtil;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Psr\Log\LoggerInterface;
-use Symfony\Component\HttpClient\Exception\ServerException;
 use Symfony\Contracts\HttpClient\ResponseInterface;
 use Symfony\Contracts\Translation\TranslatorInterface;
 
@@ -72,6 +71,11 @@ class TestableDolibarrSyncService extends DolibarrSyncService
         return parent::getOrganizationPostalAddress($organization);
     }
 
+    public function getOrganizationBillingPostalAddress(Organization $organization): ?AddressPostal
+    {
+        return parent::getOrganizationBillingPostalAddress($organization);
+    }
+
     public function getOrganizationPhone(Organization $organization): ?string
     {
         return parent::getOrganizationPhone($organization);
@@ -200,6 +204,7 @@ class DolibarrSyncServiceTest extends TestCase
 
         $organizationData1 = [
             'fullAddress' => '1 Rue Azerty',
+            'fullBillingAddress' => '2 Rue Ascii',
             'addressOwner' => 'Mr Keyboard',
             'postalCode' => '01110',
             'city' => 'ByteCity',
@@ -344,7 +349,20 @@ class DolibarrSyncServiceTest extends TestCase
             ],
         ];
 
-        $dolibarrSocietyContacts1 = [$contactData1, $obsoleteContactData, $obsoleteContactData2];
+        $billingContactData = [
+            'id' => '5',
+            'lastname' => 'Service facturation',
+            'firstname' => null,
+            'address' => '',
+            'zip' => '',
+            'town' => '',
+            'statut' => '1',
+            'array_options' => [
+                'options_2iopen_person_id' => null,
+            ],
+        ];
+
+        $dolibarrSocietyContacts1 = [$contactData1, $obsoleteContactData, $obsoleteContactData2, $billingContactData];
         $dolibarrSocietyContacts2 = [];
 
         // ----- Setup Mocks -----
@@ -408,24 +426,37 @@ class DolibarrSyncServiceTest extends TestCase
             ->method('sanitizeDolibarrData')
             ->willReturnCallback(function ($args) { return $args; });
 
-        $addressPostal = $this->getMockBuilder(AddressPostal::class)->getMock();
+        $mainPostalAddress = $this->getMockBuilder(AddressPostal::class)->getMock();
+        $mainPostalAddress->method('getStreetAddress')->willReturn($organizationData1['fullAddress']);
+        $mainPostalAddress->method('getAddressOwner')->willReturn($organizationData1['addressOwner']);
+        $mainPostalAddress->method('getPostalCode')->willReturn($organizationData1['postalCode']);
+        $mainPostalAddress->method('getAddressCity')->willReturn($organizationData1['city']);
+
+        $billingPostalAddress = $this->getMockBuilder(AddressPostal::class)->getMock();
+        $billingPostalAddress->method('getStreetAddress')->willReturn($organizationData1['fullBillingAddress']);
+        $billingPostalAddress->method('getAddressOwner')->willReturn($organizationData1['addressOwner']);
+        $billingPostalAddress->method('getPostalCode')->willReturn($organizationData1['postalCode']);
+        $billingPostalAddress->method('getAddressCity')->willReturn($organizationData1['city']);
 
         $dolibarrSyncService
             ->expects(self::exactly(2))
             ->method('getOrganizationPostalAddress')
             ->willReturnMap([
-                [$organization1, $addressPostal],
+                [$organization1, $mainPostalAddress],
+                [$organization2, null],
+            ]);
+
+        $dolibarrSyncService
+            ->expects(self::exactly(1))
+            ->method('getOrganizationBillingPostalAddress')
+            ->willReturnMap([
+                [$organization1, $billingPostalAddress],
                 [$organization2, null],
             ]);
 
         $this->addressPostalUtils
             ->method('getFullStreetAddress')
-            ->with($addressPostal)
-            ->willReturn($organizationData1['fullAddress']);
-
-        $addressPostal->method('getAddressOwner')->willReturn($organizationData1['addressOwner']);
-        $addressPostal->method('getPostalCode')->willReturn($organizationData1['postalCode']);
-        $addressPostal->method('getAddressCity')->willReturn($organizationData1['city']);
+            ->willReturnCallback(function ($a) { return $a->getStreetAddress(); });
 
         $dolibarrSyncService
             ->expects(self::exactly(2))
@@ -477,7 +508,7 @@ class DolibarrSyncServiceTest extends TestCase
         ]);
 
         $this->arrayUtils
-            ->expects(self::exactly(3))
+            ->expects(self::exactly(4))
             ->method('getChanges')
             ->willReturnCallback(
                 function (array $initialArray, array $newArray, bool $recursive = false, ?callable $callback = null) {
@@ -591,7 +622,7 @@ class DolibarrSyncServiceTest extends TestCase
 
         $operations = $dolibarrSyncService->scan($progressionCallback);
 
-        $this->assertCount(8, $operations);
+        $this->assertCount(9, $operations);
 
         $this->assertEqualsCanonicalizing(
             [
@@ -637,18 +668,28 @@ class DolibarrSyncServiceTest extends TestCase
 
         $this->assertEqualsCanonicalizing(
             [
-                '[DELETE /thirdparties/1/categories/68]',
+                '[PUT contacts/5]',
+                'address : `` => `2 Rue Ascii`',
+                'town : `` => `ByteCity`',
+                'zip : `` => `01110`',
             ],
             $operations[3]->getChangeLog()
         );
 
         $this->assertEqualsCanonicalizing(
             [
-                '[POST /thirdparties/1/categories/67]',
+                '[DELETE /thirdparties/1/categories/68]',
             ],
             $operations[4]->getChangeLog()
         );
 
+        $this->assertEqualsCanonicalizing(
+            [
+                '[POST /thirdparties/1/categories/67]',
+            ],
+            $operations[5]->getChangeLog()
+        );
+
         $this->assertEqualsCanonicalizing(
             [
                 '[PUT thirdparties/2]',
@@ -664,7 +705,7 @@ class DolibarrSyncServiceTest extends TestCase
                 "array_options.options_2iopeninfoopentalent : (new) => `Nombre d'adhérents : 0\nNombre d'accès admin : 2`",
                 'array_options.options_2iopen_last_sync_date : (new) => `2024-01-01T00:00:00+00:00`',
             ],
-            $operations[5]->getChangeLog()
+            $operations[6]->getChangeLog()
         );
 
         $this->assertEqualsCanonicalizing(
@@ -682,14 +723,14 @@ class DolibarrSyncServiceTest extends TestCase
                 'array_options.options_2iopen_last_sync_date : (new) => `2024-01-01T00:00:00+00:00`',
                 'socid : (new) => `2`',
             ],
-            $operations[6]->getChangeLog()
+            $operations[7]->getChangeLog()
         );
 
         $this->assertEqualsCanonicalizing(
             [
                 '[DELETE /thirdparties/2/categories/67]',
             ],
-            $operations[7]->getChangeLog()
+            $operations[8]->getChangeLog()
         );
 
         $this->assertCount(0, $progressionCallbackExpectedCalls);
@@ -721,8 +762,6 @@ class DolibarrSyncServiceTest extends TestCase
         $operation1->method('getChangeLog')->willReturn(['foo']);
         $operation1->expects(self::once())->method('execute')->willReturn($response);
 
-        $dolibarrSyncService->method('validateResponse')->with($response, $operation1)->willThrowException(new \RuntimeException());
-
         $operation2 = $this->getMockBuilder(CreateOperation::class)->disableOriginalConstructor()->getMock();
         $operation2->method('getStatus')->willReturn(
             BaseRestOperation::STATUS_READY, BaseRestOperation::STATUS_ERROR, BaseRestOperation::STATUS_ERROR // An error happened
@@ -733,7 +772,7 @@ class DolibarrSyncServiceTest extends TestCase
         $operation3->method('getStatus')->willReturn(BaseRestOperation::STATUS_DONE); // Invalid status, should log a warning and not execute
         $operation3->expects(self::never())->method('execute');
 
-        $this->logger->expects(self::exactly(3))->method('warning'); // 1 warning from validateResponse on the Update Op, and 2 because of the bad status of the Create Op
+        $this->logger->expects(self::exactly(2))->method('warning'); // 2 warnings because of the bad status of the Create Op
         $this->logger->expects(self::exactly(3))->method('error'); // The exception thrown during the execution of the Delete op will log 3 errors
 
         $dolibarrSyncService->execute([$operation1, $operation2, $operation3], $progressionCallback);
@@ -1412,76 +1451,4 @@ class DolibarrSyncServiceTest extends TestCase
             []
         );
     }
-
-    /**
-     * @see DolibarrSyncService::validateResponse()
-     */
-    public function testValidateResponse(): void
-    {
-        $dolibarrSyncService = $this->getMockForMethod('validateResponse');
-
-        $response = $this->getMockBuilder(ResponseInterface::class)->getMock();
-        $response->method('toArray')->willReturn(['a' => 1]);
-
-        $operation = $this->getMockBuilder(CreateOperation::class)->disableOriginalConstructor()->getMock();
-        $operation->method('getData')->willReturn(['a' => 1]);
-
-        $dolibarrSyncService->expects(self::exactly(2))->method('sanitizeDolibarrData')->with(['a' => 1])->willReturn(['a' => 1]);
-
-        $this->arrayUtils->expects(self::once())->method('getChanges')->with(['a' => 1], ['a' => 1], true)->willReturn([]);
-
-        $dolibarrSyncService->validateResponse($response, $operation);
-    }
-
-    /**
-     * @see DolibarrSyncService::validateResponse()
-     */
-    public function testValidateResponseInvalid(): void
-    {
-        $dolibarrSyncService = $this->getMockForMethod('validateResponse');
-
-        $response = $this->getMockBuilder(ResponseInterface::class)->getMock();
-        $response->method('toArray')->willReturn(['a' => 1]);
-
-        $operation = $this->getMockBuilder(CreateOperation::class)->disableOriginalConstructor()->getMock();
-        $operation->method('getData')->willReturn(['a' => 0]);
-
-        $dolibarrSyncService->expects(self::exactly(2))
-            ->method('sanitizeDolibarrData')
-            ->willReturnMap([
-                [['a' => 1], ['a' => 1]],
-                [['a' => 0], ['a' => 0]],
-            ]);
-
-        $this->arrayUtils->expects(self::once())->method('getChanges')->with(['a' => 1], ['a' => 0], true)->willReturn(['a' => 0]);
-
-        $this->expectException(\RuntimeException::class);
-
-        $dolibarrSyncService->validateResponse($response, $operation);
-    }
-
-    /**
-     * @see DolibarrSyncService::validateResponse()
-     */
-    public function testValidateResponseRequestError(): void
-    {
-        $dolibarrSyncService = $this->getMockForMethod('validateResponse');
-
-        $response = $this->getMockBuilder(ResponseInterface::class)->getMock();
-        $response->method('getInfo')->willReturnMap([
-            ['http_code', '200'], ['url', 'http://url.com'], ['response_headers', []],
-        ]);
-        $response->method('getContent')->willReturn('');
-        $response->method('toArray')->willThrowException(new ServerException($response));
-
-        $operation = $this->getMockBuilder(CreateOperation::class)->disableOriginalConstructor()->getMock();
-        $operation->method('getData')->willReturn(['a' => 0]);
-
-        $dolibarrSyncService->expects(self::never())->method('sanitizeDolibarrData');
-        $this->arrayUtils->expects(self::never())->method('getChanges');
-
-        $this->expectException(\RuntimeException::class);
-
-        $dolibarrSyncService->validateResponse($response, $operation);
-    }
 }