diff --git a/lib/API/JSON/v1/SendingQueue.php b/lib/API/JSON/v1/SendingQueue.php index a6c4ba8902..e7833483c8 100644 --- a/lib/API/JSON/v1/SendingQueue.php +++ b/lib/API/JSON/v1/SendingQueue.php @@ -73,7 +73,7 @@ class SendingQueue extends APIEndpoint { $queue->status = SendingQueueModel::STATUS_SCHEDULED; $queue->scheduled_at = Scheduler::formatDatetimeString($newsletter->scheduledAt); } else { - $segments = $newsletter->segments()->findArray(); + $segments = $newsletter->segments()->findMany(); $finder = new SubscribersFinder(); $subscribers_count = $finder->addSubscribersToTaskFromSegments($queue->task(), $segments); if(!$subscribers_count) { diff --git a/lib/Cron/Workers/Scheduler.php b/lib/Cron/Workers/Scheduler.php index 826cf58b27..4c23098845 100644 --- a/lib/Cron/Workers/Scheduler.php +++ b/lib/Cron/Workers/Scheduler.php @@ -81,7 +81,7 @@ class Scheduler { ['newsletter_id' => $newsletter->id, 'task_id' => $queue->task_id] ); // ensure that segments exist - $segments = $newsletter->segments()->findArray(); + $segments = $newsletter->segments()->findMany(); if(empty($segments)) { Logger::getLogger('post-notifications')->addInfo( 'post notification no segments', @@ -122,9 +122,9 @@ class Scheduler { function processScheduledAutomaticEmail($newsletter, $queue) { if($newsletter->sendTo === 'segment') { - $segment = Segment::findOne($newsletter->segment)->asArray(); + $segment = Segment::findOne($newsletter->segment); $finder = new SubscribersFinder(); - $result = $finder->addSubscribersToTaskFromSegments($queue->task(), array($segment)); + $result = $finder->addSubscribersToTaskFromSegments($queue->task(), [$segment]); if(empty($result)) { $queue->delete(); return false; @@ -145,14 +145,14 @@ class Scheduler { return true; } - function processScheduledStandardNewsletter($newsletter, $queue) { - $segments = $newsletter->segments()->findArray(); + function processScheduledStandardNewsletter($newsletter, SendingTask $task) { + $segments = $newsletter->segments()->findMany(); $finder = new SubscribersFinder(); - $subscribers_count = $finder->addSubscribersToTaskFromSegments($queue->task(), $segments); + $finder->addSubscribersToTaskFromSegments($task->task(), $segments); // update current queue - $queue->updateCount(); - $queue->status = null; - $queue->save(); + $task->updateCount(); + $task->status = null; + $task->save(); // update newsletter status $newsletter->setStatus(Newsletter::STATUS_SENDING); return true; diff --git a/lib/Segments/SubscribersFinder.php b/lib/Segments/SubscribersFinder.php index ef2b1a178c..6d217a62d6 100644 --- a/lib/Segments/SubscribersFinder.php +++ b/lib/Segments/SubscribersFinder.php @@ -13,15 +13,15 @@ class SubscribersFinder { function findSubscribersInSegments($subscribers_to_process_ids, $newsletter_segments_ids) { $result = array(); foreach($newsletter_segments_ids as $segment_id) { - $segment = Segment::find_one($segment_id)->asArray(); + $segment = Segment::find_one($segment_id); $result = array_merge($result, $this->findSubscribersInSegment($segment, $subscribers_to_process_ids)); } return $this->unique($result); } - private function findSubscribersInSegment($segment, $subscribers_to_process_ids) { + private function findSubscribersInSegment(Segment $segment, $subscribers_to_process_ids) { if($this->isStaticSegment($segment)) { - $subscribers = Subscriber::findSubscribersInSegments($subscribers_to_process_ids, array($segment['id']))->findMany(); + $subscribers = Subscriber::findSubscribersInSegments($subscribers_to_process_ids, array($segment->id))->findMany(); return Subscriber::extractSubscribersIds($subscribers); } $finders = Hooks::applyFilters('mailpoet_get_subscribers_in_segment_finders', array()); @@ -34,9 +34,8 @@ class SubscribersFinder { return array(); } - - private function isStaticSegment($segment) { - return $segment['type'] === Segment::TYPE_DEFAULT || $segment['type'] === Segment::TYPE_WP_USERS; + private function isStaticSegment(Segment $segment) { + return $segment->type === Segment::TYPE_DEFAULT || $segment->type === Segment::TYPE_WP_USERS; } function addSubscribersToTaskFromSegments(ScheduledTask $task, array $segments) { @@ -61,7 +60,9 @@ class SubscribersFinder { } private function addSubscribersToTaskFromStaticSegments(ScheduledTask $task, array $segments) { - $segment_ids = array_column($segments, 'id'); + $segment_ids = array_map(function($segment) { + return $segment->id; + }, $segments); Subscriber::rawExecute( 'INSERT IGNORE INTO ' . MP_SCHEDULED_TASK_SUBSCRIBERS_TABLE . ' (task_id, subscriber_id, processed) @@ -90,7 +91,7 @@ class SubscribersFinder { return $count; } - private function addSubscribersToTaskFromDynamicSegment(ScheduledTask $task, $segment) { + private function addSubscribersToTaskFromDynamicSegment(ScheduledTask $task, Segment $segment) { $finders = Hooks::applyFilters('mailpoet_get_subscribers_in_segment_finders', array()); $count = 0; foreach($finders as $finder) { @@ -135,4 +136,4 @@ class SubscribersFinder { return $result; } -} \ No newline at end of file +} diff --git a/tests/unit/Segments/SubscribersFinderTest.php b/tests/unit/Segments/SubscribersFinderTest.php index 4a7de01259..3278728299 100644 --- a/tests/unit/Segments/SubscribersFinderTest.php +++ b/tests/unit/Segments/SubscribersFinderTest.php @@ -99,10 +99,10 @@ class SubscribersFinderTest extends \MailPoetTest { $finder = new SubscribersFinder(); $subscribers_count = $finder->addSubscribersToTaskFromSegments( $this->sending->task(), - array( - array('id' => $this->segment_1->id, 'type' => Segment::TYPE_DEFAULT), - array('id' => $this->segment_2->id, 'type' => Segment::TYPE_DEFAULT), - ) + [ + $this->getDummySegment($this->segment_1->id, Segment::TYPE_DEFAULT), + $this->getDummySegment($this->segment_2->id, Segment::TYPE_DEFAULT), + ] ); expect($subscribers_count)->equals(1); expect($this->sending->getSubscribers())->equals(array($this->subscriber_2->id)); @@ -112,9 +112,9 @@ class SubscribersFinderTest extends \MailPoetTest { $finder = new SubscribersFinder(); $subscribers_count = $finder->addSubscribersToTaskFromSegments( $this->sending->task(), - array( - array('id' => $this->segment_1->id, 'type' => 'UNKNOWN SEGMENT'), - ) + [ + $this->getDummySegment($this->segment_1->id, 'UNKNOWN SEGMENT'), + ] ); expect($subscribers_count)->equals(0); } @@ -134,9 +134,9 @@ class SubscribersFinderTest extends \MailPoetTest { $finder = new SubscribersFinder(); $subscribers_count = $finder->addSubscribersToTaskFromSegments( $this->sending->task(), - array( - array('id' => $this->segment_2->id, 'type' => ''), - ) + [ + $this->getDummySegment($this->segment_2->id, ''), + ] ); expect($subscribers_count)->equals(1); expect($this->sending->getSubscribers())->equals(array($this->subscriber_1->id)); @@ -157,15 +157,22 @@ class SubscribersFinderTest extends \MailPoetTest { $subscribers_count = $finder->addSubscribersToTaskFromSegments( $this->sending->task(), - array( - array('id' => $this->segment_1->id, 'type' => Segment::TYPE_DEFAULT), - array('id' => $this->segment_2->id, 'type' => Segment::TYPE_DEFAULT), - array('id' => $this->segment_3->id, 'type' => ''), - ) + [ + $this->getDummySegment($this->segment_1->id, Segment::TYPE_DEFAULT), + $this->getDummySegment($this->segment_2->id, Segment::TYPE_DEFAULT), + $this->getDummySegment($this->segment_3->id, ''), + ] ); expect($subscribers_count)->equals(1); expect($this->sending->getSubscribers())->equals(array($this->subscriber_2->id)); } + private function getDummySegment($id, $type) { + $segment = Segment::create(); + $segment->id = $id; + $segment->type = $type; + return $segment; + } + }