Merge pull request mautic#1060 from mautic-inc/MAUT-4362

MAUT-4362: Scoring - SFDC clearing out new data from ACS
escopecz · Feb 23, 2024 · 55b5ab7 · 55b5ab7
1 parent 6ac2e69
commit 55b5ab7
Show file tree

Hide file tree

Showing 9 changed files with 200 additions and 9 deletions.
diff --git a/app/bundles/IntegrationsBundle/Config/config.php b/app/bundles/IntegrationsBundle/Config/config.php
@@ -212,6 +212,7 @@
                     'mautic.integrations.sync.data_exchange.mautic.full_object_report_builder',
                     'mautic.integrations.sync.data_exchange.mautic.partial_object_report_builder',
                     'mautic.integrations.sync.data_exchange.mautic.order_executioner',
+                    'mautic.integrations.helper.sync_date',
                 ],
             ],
             'mautic.integrations.sync.integration_process.object_change_generator' => [

diff --git a/app/bundles/IntegrationsBundle/Entity/FieldChangeRepository.php b/app/bundles/IntegrationsBundle/Entity/FieldChangeRepository.php
@@ -39,7 +39,7 @@ public function deleteEntitiesForObjectByColumnName(int $objectId, string $objec
     /**
      * Takes an object id & type and deletes all entities that match.
      */
-    public function deleteEntitiesForObject(int $objectId, string $objectType, ?string $integration = null): void
+    public function deleteEntitiesForObject(int $objectId, string $objectType, ?string $integration = null, \DateTimeInterface $toDateTime = null): void
     {
         $qb = $this->getEntityManager()->getConnection()->createQueryBuilder();
 
@@ -51,8 +51,13 @@ public function deleteEntitiesForObject(int $objectId, string $objectType, ?stri
             $qb->setParameter('integration', $integration);
         }
 
+        if (null !== $toDateTime) {
+            $expr = $expr->with($qb->expr()->lte('modified_at', ':toDateTime'));
+            $qb->setParameter('toDateTime', $toDateTime->format('Y-m-d H:i:s'));
+        }
+
         $qb->setParameter('objectType', $objectType)
-            ->setParameter('objectId', (int) $objectId);
+            ->setParameter('objectId', $objectId);
 
         $qb
             ->delete(MAUTIC_TABLE_PREFIX.'sync_object_field_change_report')

diff --git a/app/bundles/IntegrationsBundle/Sync/Helper/SyncDateHelper.php b/app/bundles/IntegrationsBundle/Sync/Helper/SyncDateHelper.php
@@ -20,6 +20,8 @@ class SyncDateHelper
      */
     private array $lastObjectSyncDates = [];
 
+    private ?\DateTimeInterface $internalSyncStartDateTime;
+
     public function __construct(
         private Connection $connection
     ) {
@@ -110,4 +112,37 @@ public function getLastSyncDateForObject(string $integration, string $object): ?
 
         return $lastSync;
     }
+
+    public function getInternalSyncStartDateTime(): ?\DateTimeInterface
+    {
+        return $this->internalSyncStartDateTime;
+    }
+
+    public function setInternalSyncStartDateTime(): void
+    {
+        if ($this->internalSyncStartDateTime) {
+            return;
+        }
+
+        $this->internalSyncStartDateTime = $this->calculateInternalSyncStartDateTime();
+    }
+
+    private function calculateInternalSyncStartDateTime(): \DateTimeInterface
+    {
+        $now = new \DateTimeImmutable('now', new \DateTimeZone('UTC'));
+        // If there is no syncToDateTime value use "now"
+        if (!$this->getSyncToDateTime()) {
+            return $now;
+        }
+
+        // Clone it so that we don't modify the initial object
+        $syncToDateTime = clone $this->getSyncToDateTime();
+
+        // We should compare in UTC timezone
+        $syncToDateTime->setTimezone(new \DateTimeZone('UTC'));
+
+        // If syncToDate is less than now then use syncToDate, because otherwise we may delete
+        // changes that aren't supposed to be deleted from the sync_object_field_change_report table
+        return min($now, $syncToDateTime);
+    }
 }
diff --git a/app/bundles/IntegrationsBundle/Sync/SyncDataExchange/MauticSyncDataExchange.php b/app/bundles/IntegrationsBundle/Sync/SyncDataExchange/MauticSyncDataExchange.php
@@ -16,6 +16,7 @@
 use Mautic\IntegrationsBundle\Sync\Exception\ObjectNotFoundException;
 use Mautic\IntegrationsBundle\Sync\Exception\ObjectNotSupportedException;
 use Mautic\IntegrationsBundle\Sync\Helper\MappingHelper;
+use Mautic\IntegrationsBundle\Sync\Helper\SyncDateHelper;
 use Mautic\IntegrationsBundle\Sync\Logger\DebugLogger;
 use Mautic\IntegrationsBundle\Sync\SyncDataExchange\Helper\FieldHelper;
 use Mautic\IntegrationsBundle\Sync\SyncDataExchange\Internal\Executioner\OrderExecutioner;
@@ -36,7 +37,8 @@ public function __construct(
         private MappingHelper $mappingHelper,
         private FullObjectReportBuilder $fullObjectReportBuilder,
         private PartialObjectReportBuilder $partialObjectReportBuilder,
-        private OrderExecutioner $orderExecutioner
+        private OrderExecutioner $orderExecutioner,
+        private SyncDateHelper $syncDateHelper
     ) {
     }
 
@@ -95,7 +97,12 @@ public function cleanupProcessedObjects(array $objectChanges): void
                 $object   = $this->fieldHelper->getFieldObjectName($changedObjectDAO->getMappedObject());
                 $objectId = $changedObjectDAO->getMappedObjectId();
 
-                $this->fieldChangeRepository->deleteEntitiesForObject((int) $objectId, $object, $changedObjectDAO->getIntegration());
+                $this->fieldChangeRepository->deleteEntitiesForObject(
+                    (int) $objectId,
+                    $object,
+                    $changedObjectDAO->getIntegration(),
+                    $this->syncDateHelper->getInternalSyncStartDateTime()
+                );
             } catch (ObjectNotSupportedException $exception) {
                 DebugLogger::log(
                     self::NAME,

diff --git a/app/bundles/IntegrationsBundle/Sync/SyncProcess/SyncProcess.php b/app/bundles/IntegrationsBundle/Sync/SyncProcess/SyncProcess.php
@@ -65,6 +65,7 @@ public function execute(): void
         }
 
         if ($this->inputOptionsDAO->pushIsEnabled()) {
+            $this->syncDateHelper->setInternalSyncStartDateTime();
             $this->executeInternalSync();
         }
 

diff --git a/app/bundles/IntegrationsBundle/Tests/Functional/Entity/FieldChangeRepositoryTest.php b/app/bundles/IntegrationsBundle/Tests/Functional/Entity/FieldChangeRepositoryTest.php
@@ -0,0 +1,104 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Mautic\IntegrationsBundle\Tests\Functional\Entity;
+
+use Mautic\CoreBundle\Test\MauticMysqlTestCase;
+use Mautic\IntegrationsBundle\Entity\FieldChange;
+use Mautic\IntegrationsBundle\Entity\FieldChangeRepository;
+use Mautic\LeadBundle\Entity\Lead;
+use PHPUnit\Framework\Assert;
+
+final class FieldChangeRepositoryTest extends MauticMysqlTestCase
+{
+    const INTEGRATION = 'someIntegration';
+    const COLUMN_NAME = 'some_column';
+    const OBJECT_ID   = 100;
+
+    private FieldChangeRepository $repository;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->repository = $this->em->getRepository(FieldChange::class);
+    }
+
+    public function testThatFindChangesBeforeMethodReturnsChangesInCorrectOrder(): void
+    {
+        $fieldChanges = $this->generateFieldChanges(3);
+        $fieldChanges[0]->setObjectId(3);
+        $fieldChanges[1]->setObjectId(1);
+        $fieldChanges[2]->setObjectId(2);
+
+        foreach ($fieldChanges as $fieldChange) {
+            $this->em->persist($fieldChange);
+        }
+
+        $this->em->flush();
+
+        $changes = $this->repository->findChangesBefore(
+            static::INTEGRATION,
+            Lead::class,
+            $this->getNow()->modify('+30 seconds'),
+            null,
+            2
+        );
+
+        Assert::assertSame(1, (int) $changes[0]['object_id']);
+        Assert::assertSame(2, (int) $changes[1]['object_id']);
+    }
+
+    public function testThatItDoesntDeleteObjectsThatCameDuringInternalSynchronization(): void
+    {
+        $now          = $this->getNow();
+        $fieldChanges = $this->generateFieldChanges(2);
+        $aDayLater    = (clone $now)->modify('+1 day'); // Don't use \DateTimeImmutable because entity expects \DateTime
+        $fieldChanges[1]->setModifiedAt($aDayLater);
+
+        foreach ($fieldChanges as $fieldChange) {
+            $this->em->persist($fieldChange);
+        }
+
+        $this->em->flush();
+
+        $this->repository->deleteEntitiesForObject(
+            static::OBJECT_ID,
+            Lead::class,
+            static::INTEGRATION,
+            $now->modify('+30 seconds')
+        );
+
+        $remainingChanges = $this->repository->findAll();
+        Assert::assertCount(1, $remainingChanges);
+        Assert::assertSame($fieldChanges[1]->getId(), $remainingChanges[0]->getId());
+    }
+
+    /**
+     * @return FieldChange[]
+     */
+    private function generateFieldChanges(int $quantity): array
+    {
+        $fieldChanges = [];
+        $now          = $this->getNow();
+        for ($i = 1; $i <= $quantity; ++$i) {
+            $fieldChange = new FieldChange();
+            $fieldChange->setIntegration(static::INTEGRATION);
+            $fieldChange->setObjectId(static::OBJECT_ID);
+            $fieldChange->setObjectType(Lead::class);
+            $fieldChange->setModifiedAt($now);
+            $fieldChange->setColumnName(static::COLUMN_NAME);
+            $fieldChange->setColumnType('string');
+            $fieldChange->setColumnValue((string) $i);
+
+            $fieldChanges[] = $fieldChange;
+        }
+
+        return $fieldChanges;
+    }
+
+    private function getNow(): \DateTime
+    {
+        return new \DateTime('now', new \DateTimeZone('UTC'));
+    }
+}
diff --git a/app/bundles/IntegrationsBundle/Tests/Unit/Sync/Helper/SyncDateHelperTest.php b/app/bundles/IntegrationsBundle/Tests/Unit/Sync/Helper/SyncDateHelperTest.php
@@ -5,6 +5,7 @@
 namespace Mautic\IntegrationsBundle\Tests\Unit\Sync\Helper;
 
 use Mautic\IntegrationsBundle\Sync\Helper\SyncDateHelper;
+use PHPUnit\Framework\Assert;
 use PHPUnit\Framework\TestCase;
 
 class SyncDateHelperTest extends TestCase
@@ -28,7 +29,7 @@ public function testSpecifiedFromDateTimeIsReturned(): void
 
         $this->syncDateHelper->setSyncDateTimes($syncFromDateTime);
 
-        $this->assertEquals($syncFromDateTime, $this->syncDateHelper->getSyncFromDateTime('Test', 'Object'));
+        Assert::assertEquals($syncFromDateTime, $this->syncDateHelper->getSyncFromDateTime('Test', 'Object'));
     }
 
     public function testLastSyncDateForIntegrationSyncObjectIsReturned(): void
@@ -38,7 +39,7 @@ public function testLastSyncDateForIntegrationSyncObjectIsReturned(): void
         $this->syncDateHelper->method('getLastSyncDateForObject')
             ->willReturn($objectLastSyncDate);
 
-        $this->assertEquals($objectLastSyncDate, $this->syncDateHelper->getSyncFromDateTime('Test', 'Object'));
+        Assert::assertEquals($objectLastSyncDate, $this->syncDateHelper->getSyncFromDateTime('Test', 'Object'));
     }
 
     public function testSyncToDateTimeIsReturnedIfSpecified(): void
@@ -47,13 +48,39 @@ public function testSyncToDateTimeIsReturnedIfSpecified(): void
 
         $this->syncDateHelper->setSyncDateTimes(null, $syncToDateTime);
 
-        $this->assertEquals($syncToDateTime, $this->syncDateHelper->getSyncToDateTime());
+        Assert::assertEquals($syncToDateTime, $this->syncDateHelper->getSyncToDateTime());
     }
 
     public function testSyncDateTimeIsReturnedForSyncToDateTimeIfNotSpecified(): void
     {
         $this->syncDateHelper->setSyncDateTimes();
 
-        $this->assertInstanceOf(\DateTimeImmutable::class, $this->syncDateHelper->getSyncToDateTime());
+        Assert::assertInstanceOf(\DateTimeImmutable::class, $this->syncDateHelper->getSyncToDateTime());
+    }
+
+    public function testThatSetInternalSyncStartDateTimeMethodUsesSyncToDateValueIfItIsEarlier(): void
+    {
+        // Although $fiveSecondsBefore value is expected to be in UTC timezone let's use another timezone
+        // to check how the method handles such cases.
+        $fiveSecondsBefore = new \DateTime('-5 seconds', new \DateTimeZone('Etc/GMT-5'));
+        $this->syncDateHelper->setSyncDateTimes(null, $fiveSecondsBefore);
+        $this->syncDateHelper->setInternalSyncStartDateTime();
+        $internalSyncStartDateTime = $this->syncDateHelper->getInternalSyncStartDateTime();
+        Assert::assertSame($fiveSecondsBefore->getTimestamp(), $internalSyncStartDateTime->getTimestamp());
+    }
+
+    public function testThatSetInternalSyncStartDateTimeMethodUsesNowIfItIsEarlier(): void
+    {
+        // Although $fiveSecondsAfter value is expected to be in UTC timezone let's use another timezone
+        // to check how the method handles such cases.
+        $fiveSecondsAfter = new \DateTime('+5 seconds', new \DateTimeZone('Etc/GMT+5'));
+        $this->syncDateHelper->setSyncDateTimes(null, $fiveSecondsAfter);
+        $this->syncDateHelper->setInternalSyncStartDateTime();
+        $now                       = new \DateTime('now', new \DateTimeZone('UTC'));
+        $internalSyncStartDateTime = $this->syncDateHelper->getInternalSyncStartDateTime();
+        $difference                = $internalSyncStartDateTime->getTimestamp() - $now->getTimestamp();
+
+        // Add a 1 second buffer in case there is some delay
+        Assert::assertTrue((1 >= $difference) && (-1 < $difference));
     }
 }
diff --git a/...undles/IntegrationsBundle/Tests/Unit/Sync/SyncDataExchange/MauticSyncDataExchangeTest.php b/...undles/IntegrationsBundle/Tests/Unit/Sync/SyncDataExchange/MauticSyncDataExchangeTest.php
@@ -12,6 +12,7 @@
 use Mautic\IntegrationsBundle\Sync\DAO\Sync\Request\RequestDAO;
 use Mautic\IntegrationsBundle\Sync\DAO\Value\NormalizedValueDAO;
 use Mautic\IntegrationsBundle\Sync\Helper\MappingHelper;
+use Mautic\IntegrationsBundle\Sync\Helper\SyncDateHelper;
 use Mautic\IntegrationsBundle\Sync\SyncDataExchange\Helper\FieldHelper;
 use Mautic\IntegrationsBundle\Sync\SyncDataExchange\Internal\Executioner\OrderExecutioner;
 use Mautic\IntegrationsBundle\Sync\SyncDataExchange\Internal\ReportBuilder\FullObjectReportBuilder;
@@ -56,6 +57,11 @@ class MauticSyncDataExchangeTest extends TestCase
 
     private \Mautic\IntegrationsBundle\Sync\SyncDataExchange\MauticSyncDataExchange $mauticSyncDataExchange;
 
+    /**
+     * @var SyncDateHelper&MockObject
+     */
+    private MockObject $syncDateHelper;
+
     protected function setUp(): void
     {
         $this->fieldChangeRepository      = $this->createMock(FieldChangeRepository::class);
@@ -64,14 +70,16 @@ protected function setUp(): void
         $this->fullObjectReportBuilder    = $this->createMock(FullObjectReportBuilder::class);
         $this->partialObjectReportBuilder = $this->createMock(PartialObjectReportBuilder::class);
         $this->orderExecutioner           = $this->createMock(OrderExecutioner::class);
+        $this->syncDateHelper             = $this->createMock(SyncDateHelper::class);
 
         $this->mauticSyncDataExchange = new MauticSyncDataExchange(
             $this->fieldChangeRepository,
             $this->fieldHelper,
             $this->mappingHelper,
             $this->fullObjectReportBuilder,
             $this->partialObjectReportBuilder,
-            $this->orderExecutioner
+            $this->orderExecutioner,
+            $this->syncDateHelper
         );
     }
 

diff --git a/app/bundles/IntegrationsBundle/Tests/Unit/Sync/SyncProcess/SyncProcessTest.php b/app/bundles/IntegrationsBundle/Tests/Unit/Sync/SyncProcess/SyncProcessTest.php
@@ -135,6 +135,9 @@ public function testBatchSyncEventsAreDispatched(): void
             ->method('pushIsEnabled')
             ->willReturn(true);
 
+        $this->syncDateHelper->expects($this->once())
+            ->method('setInternalSyncStartDateTime');
+
         // Integration to Mautic
 
         // fetch the report from the integration