Do not fail scheduled events when publish date range is off (mautic#1…

…3995)

* Merge pull request mautic#1916 from acquia/MAUT-8480

Maut 8480 - do not fail scheduled events when publish date range is off

* CS, STAN fixes

app/bundles/CampaignBundle/Executioner/EventExecutioner.php

@@ -226,9 +226,13 @@ public function recordLogsAsFailedForEvent(Event $event, ArrayCollection $contac
     /**
      * @param ArrayCollection|LeadEventLog[] $logs
      * @param string                         $error
+     *
+     * @deprecated as not used
      */
     public function recordLogsWithError(ArrayCollection $logs, $error): void
     {
+        @trigger_error('EventExecutioner::recordLogsWithError() is deprecated in Mautic:4 and is removed from Mautic:5 as unused', E_USER_DEPRECATED);
+
         foreach ($logs as $log) {
             $log->appendToMetadata(
                 [

app/bundles/CampaignBundle/Executioner/ScheduledExecutioner.php

@@ -142,11 +142,6 @@ public function executeByIds(array $logIds, OutputInterface $output = null, ?\Da
                 } catch (NoContactsFoundException) {
                     // All of the events were rescheduled
                 }
-            } else {
-                $this->executioner->recordLogsWithError(
-                    $organizedLogs,
-                    $this->translator->trans('mautic.campaign.event.campaign_unpublished')
-                );
             }
 
             $this->progressBar->advance($organizedLogs->count());

app/bundles/CampaignBundle/Tests/Controller/AjaxControllerFunctionalTest.php

@@ -4,23 +4,23 @@
 
 namespace Mautic\CampaignBundle\Tests\Controller;
 
-use Mautic\CampaignBundle\Entity\Campaign;
-use Mautic\CampaignBundle\Entity\Event;
-use Mautic\CampaignBundle\Entity\Lead as CampaignLead;
 use Mautic\CampaignBundle\Entity\LeadEventLog;
 use Mautic\CampaignBundle\Entity\LeadEventLogRepository;
+use Mautic\CampaignBundle\Tests\Functional\Fixtures\FixtureHelper;
 use Mautic\CoreBundle\Test\MauticMysqlTestCase;
-use Mautic\LeadBundle\Entity\Lead;
 use PHPUnit\Framework\Assert;
 use Symfony\Component\HttpFoundation\Request;
 
 class AjaxControllerFunctionalTest extends MauticMysqlTestCase
 {
     public function testCancelScheduledCampaignEventAction(): void
     {
-        $contact  = $this->createContact();
-        $campaign = $this->createCampaign();
-        $this->addContactToCampaign($contact, $campaign);
+        $fixtureHelper = new FixtureHelper($this->em);
+        $contact       = $fixtureHelper->createContact('some@contact.email');
+        $campaign      = $fixtureHelper->createCampaign('Scheduled event test');
+        $fixtureHelper->addContactToCampaign($contact, $campaign);
+        $fixtureHelper->createCampaignWithScheduledEvent($campaign);
+        $this->em->flush();
 
         $commandResult = $this->testSymfonyCommand('mautic:campaigns:trigger', ['--campaign-id' => $campaign->getId()]);
 
@@ -48,101 +48,4 @@ public function testCancelScheduledCampaignEventAction(): void
         Assert::assertSame('{"success":1}', $this->client->getResponse()->getContent());
         Assert::assertFalse($log->getIsScheduled());
     }
-
-    private function createContact(): Lead
-    {
-        $contact = new Lead();
-
-        $this->em->persist($contact);
-        $this->em->flush();
-
-        return $contact;
-    }
-
-    private function addContactToCampaign(Lead $contact, Campaign $campaign): void
-    {
-        $ref = new CampaignLead();
-        $ref->setCampaign($campaign);
-        $ref->setLead($contact);
-        $ref->setDateAdded(new \DateTime());
-
-        $this->em->persist($ref);
-        $this->em->flush();
-    }
-
-    private function createCampaign(): Campaign
-    {
-        $campaign = new Campaign();
-        $campaign->setName('Campaign A');
-        $campaign->setIsPublished(true);
-
-        $this->em->persist($campaign);
-        $this->em->flush();
-
-        $event = new Event();
-        $event->setCampaign($campaign);
-        $event->setName('Adjust contact points');
-        $event->setType('lead.changepoints');
-        $event->setEventType('action');
-        $event->setTriggerInterval(1);
-        $event->setTriggerIntervalUnit('d');
-        $event->setTriggerMode('interval');
-        $event->setProperties(
-            [
-                'canvasSettings' => [
-                    'droppedX' => '1080',
-                    'droppedY' => '155',
-                ],
-                'name'                       => '',
-                'triggerMode'                => 'interval',
-                'triggerDate'                => null,
-                'triggerInterval'            => '1',
-                'triggerIntervalUnit'        => 'd',
-                'triggerHour'                => '',
-                'triggerRestrictedStartHour' => '',
-                'triggerRestrictedStopHour'  => '',
-                'anchor'                     => 'leadsource',
-                'properties'                 => ['points' => '5'],
-                'type'                       => 'lead.changepoints',
-                'eventType'                  => 'action',
-                'anchorEventType'            => 'source',
-                'campaignId'                 => $campaign->getId(),
-                'buttons'                    => ['save' => ''],
-                'points'                     => 5,
-            ]
-        );
-
-        $this->em->persist($event);
-        $this->em->flush();
-
-        $campaign->addEvent(0, $event);
-        $campaign->setCanvasSettings(
-            [
-                'nodes' => [
-                    [
-                        'id'        => $event->getId(),
-                        'positionX' => '1080',
-                        'positionY' => '155',
-                    ],
-                    [
-                        'id'        => 'lists',
-                        'positionX' => '1180',
-                        'positionY' => '50',
-                    ],
-                ],
-                'connections' => [
-                    [
-                        'sourceId' => 'lists',
-                        'targetId' => $event->getId(),
-                        'anchors'  => [
-                            'source' => 'leadsource',
-                            'target' => 'top',
-                        ],
-                    ],
-                ],
-            ]
-        );
-
-        return $campaign;
-    }
 }

app/bundles/CampaignBundle/Tests/Executioner/EventExecutionerTest.php

@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 namespace Mautic\CampaignBundle\Tests\Executioner;
 
 use Doctrine\Common\Collections\ArrayCollection;
@@ -26,65 +28,65 @@
 use Mautic\EmailBundle\EmailEvents;
 use Mautic\EmailBundle\Form\Type\EmailSendType;
 use Mautic\LeadBundle\Entity\Lead;
-use PHPUnit\Framework\MockObject\MockBuilder;
+use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
 
 class EventExecutionerTest extends \PHPUnit\Framework\TestCase
 {
     /**
-     * @var EventCollector|\PHPUnit\Framework\MockObject\MockObject
+     * @var EventCollector&MockObject
      */
-    private \PHPUnit\Framework\MockObject\MockObject $eventCollector;
+    private MockObject $eventCollector;
 
     /**
-     * @var EventLogger|\PHPUnit\Framework\MockObject\MockObject
+     * @var EventLogger&MockObject
      */
-    private \PHPUnit\Framework\MockObject\MockObject $eventLogger;
+    private MockObject $eventLogger;
 
     /**
-     * @var ActionExecutioner|\PHPUnit\Framework\MockObject\MockObject
+     * @var ActionExecutioner&MockObject
      */
-    private \PHPUnit\Framework\MockObject\MockObject $actionExecutioner;
+    private MockObject $actionExecutioner;
 
     /**
-     * @var ConditionExecutioner|\PHPUnit\Framework\MockObject\MockObject
+     * @var ConditionExecutioner&MockObject
      */
-    private \PHPUnit\Framework\MockObject\MockObject $conditionExecutioner;
+    private MockObject $conditionExecutioner;
 
     /**
-     * @var DecisionExecutioner|\PHPUnit\Framework\MockObject\MockObject
+     * @var DecisionExecutioner&MockObject
      */
-    private \PHPUnit\Framework\MockObject\MockObject $decisionExecutioner;
+    private MockObject $decisionExecutioner;
 
     /**
-     * @var LoggerInterface|\PHPUnit\Framework\MockObject\MockObject
+     * @var LoggerInterface&MockObject
      */
-    private \PHPUnit\Framework\MockObject\MockObject $logger;
+    private MockObject $logger;
 
     /**
-     * @var EventScheduler|\PHPUnit\Framework\MockObject\MockObject
+     * @var EventScheduler&MockObject
      */
-    private \PHPUnit\Framework\MockObject\MockObject $eventScheduler;
+    private MockObject $eventScheduler;
 
     /**
-     * @var RemovedContactTracker|\PHPUnit\Framework\MockObject\MockObject
+     * @var RemovedContactTracker&MockObject
      */
-    private \PHPUnit\Framework\MockObject\MockObject $removedContactTracker;
+    private MockObject $removedContactTracker;
 
     /**
-     * @var LeadRepository|\PHPUnit\Framework\MockObject\MockObject
+     * @var LeadRepository&MockObject
      */
-    private \PHPUnit\Framework\MockObject\MockObject $leadRepository;
+    private MockObject $leadRepository;
 
     /**
-     * @var EventRepository|MockBuilder
+     * @var EventRepository&MockObject
      */
-    private \PHPUnit\Framework\MockObject\MockObject $eventRepository;
+    private MockObject $eventRepository;
 
     /**
-     * @var Translator|MockBuilder
+     * @var Translator&MockObject
      */
-    private \PHPUnit\Framework\MockObject\MockObject $translator;
+    private MockObject $translator;
 
     protected function setUp(): void
     {
@@ -99,13 +101,32 @@ protected function setUp(): void
         $this->eventScheduler        = $this->createMock(EventScheduler::class);
         $this->removedContactTracker = $this->createMock(RemovedContactTracker::class);
         $this->leadRepository        = $this->createMock(LeadRepository::class);
-        $this->eventRepository       = $this->getMockBuilder(EventRepository::class)
-            ->disableOriginalConstructor()
-            ->getMock();
+        $this->eventRepository       = $this->createMock(EventRepository::class);
+        $this->translator            = $this->createMock(Translator::class);
+    }
 
-        $this->translator = $this->getMockBuilder(Translator::class)
-            ->disableOriginalConstructor()
-            ->getMock();
+    /**
+     * @group legacy
+     */
+    public function testDeprecatedMethodOtherwiseItLowersCodeCoverageAsItsNoLongerUsed(): void
+    {
+        $deprecationTriggered = false;
+        $errorHandler         = function (int $errorNumber, string $errorMessage) use (&$deprecationTriggered): bool {
+            if (E_USER_DEPRECATED === $errorNumber && 'EventExecutioner::recordLogsWithError() is deprecated in Mautic:4 and is removed from Mautic:5 as unused' === $errorMessage) {
+                $deprecationTriggered = true;
+            }
+
+            // returning false let the normal error handler continue
+            return false;
+        };
+
+        $this->eventLogger->expects($this->once())->method('persistCollection')->willReturn($this->eventLogger);
+
+        set_error_handler($errorHandler);
+        $this->getEventExecutioner()->recordLogsWithError(new ArrayCollection([]), 'some message'); // @phpstan-ignore-line as recordLogsWithError() is deprecated
+        restore_error_handler();
+
+        $this->assertTrue($deprecationTriggered, 'Deprecation should be triggered');
     }
 
     public function testJumpToEventsAreProcessedAfterOtherEvents(): void
@@ -206,10 +227,7 @@ function (Event $event, ActionAccessor $config, ArrayCollection $contacts, $isIn
         $this->getEventExecutioner()->executeEventsForContacts($events, $contacts);
     }
 
-    /**
-     * @return EventExecutioner
-     */
-    private function getEventExecutioner()
+    private function getEventExecutioner(): EventExecutioner
     {
         return new EventExecutioner(
             $this->eventCollector,

app/bundles/CampaignBundle/Tests/Executioner/ScheduledExecutionerTest.php

@@ -452,9 +452,6 @@ public function testSpecificEventsWithUnpublishedCamapign(): void
         $this->executioner->expects($this->never())
             ->method('executeLogs');
 
-        $this->executioner->expects($this->once())
-            ->method('recordLogsWithError');
-
         $this->contactFinder->expects($this->never())
             ->method('hydrateContacts');