Capaign events with 0 delay ignore day of week restrictions (#11332)

* Add functional test for campaign interval event * Refactor to use dataproviders and include more test scenarios * update test to reflect community behaviour * include error message * extract condition to method for readability * Extract condition check to method * extract condition check to method * Allow DOW restrictions for all interval types Co-authored-by: John Linhart <admin@escope.cz>
mautic · Aug 4, 2022 · 0f0736d · 0f0736d
1 parent 52f221e
commit 0f0736d
Show file tree

Hide file tree

Showing 4 changed files with 335 additions and 518 deletions.
diff --git a/app/bundles/CampaignBundle/Assets/js/campaign.js b/app/bundles/CampaignBundle/Assets/js/campaign.js
@@ -170,12 +170,10 @@ Mautic.campaignOnUnload = function(container) {
  */
 Mautic.campaignEventOnLoad = function (container, response) {
     if (mQuery('#campaignevent_triggerHour').length) {
-        Mautic.campaignEventShowHideIntervalSettings();
         Mautic.campaignEventUpdateIntervalHours();
         mQuery('#campaignevent_triggerHour').on('change', Mautic.campaignEventUpdateIntervalHours);
         mQuery('#campaignevent_triggerRestrictedStartHour').on('change', Mautic.campaignEventUpdateIntervalHours);
         mQuery('#campaignevent_triggerRestrictedStopHour').on('change', Mautic.campaignEventUpdateIntervalHours);
-        mQuery('#campaignevent_triggerIntervalUnit').on('change', Mautic.campaignEventShowHideIntervalSettings);
         mQuery('#campaignevent_triggerRestrictedDaysOfWeek_0').on('change', Mautic.campaignEventSelectDOW);
         mQuery('#campaignevent_triggerRestrictedDaysOfWeek_1').on('change', Mautic.campaignEventSelectDOW);
         mQuery('#campaignevent_triggerRestrictedDaysOfWeek_2').on('change', Mautic.campaignEventSelectDOW);
@@ -294,18 +292,6 @@ Mautic.campaignEventUpdateIntervalHours = function () {
     }
 };
 
-/**
- * Show/hide interval settings
- */
-Mautic.campaignEventShowHideIntervalSettings = function() {
-    var unit = mQuery('#campaignevent_triggerIntervalUnit').val();
-    if (unit === 'i' || unit === 'h') {
-        mQuery('#interval_settings').addClass('hide');
-    } else {
-        mQuery('#interval_settings').removeClass('hide');
-    }
-};
-
 /**
  * Update DOW for weekday selection
  */

diff --git a/app/bundles/CampaignBundle/Executioner/Scheduler/Mode/Interval.php b/app/bundles/CampaignBundle/Executioner/Scheduler/Mode/Interval.php
@@ -56,7 +56,7 @@ public function getExecutionDateTime(Event $event, \DateTime $compareFromDateTim
         } catch (\Exception $exception) {
             $this->logger->error('CAMPAIGN: Determining interval scheduled failed with "'.$exception->getMessage().'"');
 
-            throw new NotSchedulableException();
+            throw new NotSchedulableException($exception->getMessage());
         }
 
         if ($comparedToDateTime > $compareFromDateTime) {
@@ -146,24 +146,28 @@ public function groupContactsByDate(Event $event, ArrayCollection $contacts, \Da
      */
     public function isContactSpecificExecutionDateRequired(Event $event)
     {
-        if (Event::TRIGGER_MODE_INTERVAL !== $event->getTriggerMode()) {
+        if (!$this->isTriggerModeInterval($event) || $this->isRestrictedToDailyScheduling($event) || $this->hasTimeRelatedRestrictions($event)) {
             return false;
         }
 
-        // Restrict just for daily scheduling
-        if (!in_array($event->getTriggerIntervalUnit(), ['d', 'm', 'y'])) {
-            return false;
-        }
+        return true;
+    }
 
-        if (
-            null === $event->getTriggerHour() &&
-            (null === $event->getTriggerRestrictedStartHour() || null === $event->getTriggerRestrictedStopHour()) &&
-            empty($event->getTriggerRestrictedDaysOfWeek())
-        ) {
-            return false;
-        }
+    private function isTriggerModeInterval(Event $event): bool
+    {
+        return Event::TRIGGER_MODE_INTERVAL === $event->getTriggerMode();
+    }
 
-        return true;
+    private function isRestrictedToDailyScheduling(Event $event): bool
+    {
+        return !in_array($event->getTriggerIntervalUnit(), ['d', 'm', 'y']);
+    }
+
+    private function hasTimeRelatedRestrictions(Event $event): bool
+    {
+        return null === $event->getTriggerHour() &&
+            (null === $event->getTriggerRestrictedStartHour() || null === $event->getTriggerRestrictedStopHour()) &&
+            empty($event->getTriggerRestrictedDaysOfWeek());
     }
 
     /**