Add logging, code clean up, PHPStan hints

putyourlightson · Jun 1, 2023 · 58597d6 · 58597d6
1 parent eb5b1aa
commit 58597d6
Show file tree

Hide file tree

Showing 17 changed files with 69 additions and 45 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,8 +1,9 @@
 # Release Notes for Campaign
 
-## 2.8.0 - Unreleased
+## 2.8.0 - 2023-06-01
 ### Added
 - Added the ability to import campaigns, contacts and mailing lists using Feed Me ([#395](https://github.com/putyourlightson/craft-campaign/issues/395)).
+- Added logging of sendout batches and when a sendout completes ([#385](https://github.com/putyourlightson/craft-campaign/issues/385)).
 
 ### Changed
 - Contact field names now include info modals with instructions in the CSV file import field ([#392](https://github.com/putyourlightson/craft-campaign/issues/392)).

diff --git a/src/controllers/ExportsController.php b/src/controllers/ExportsController.php
@@ -67,21 +67,14 @@ public function actionExportFile(): ?Response
             'fields' => $this->request->getBodyParam('fields'),
         ]);
 
-        // Get storage directory path
         $path = Craft::$app->getPath()->getStoragePath() . '/campaign/exports/' . gmdate('YmdHis') . '/';
-
-        // Create directory
         FileHelper::createDirectory($path);
-
-        // Set file path
         $export->filePath = $path . 'export.csv';
 
-        // Export it
         if (!Campaign::$plugin->exports->exportFile($export)) {
             return $this->asModelFailure($export, Craft::t('campaign', 'Couldn’t export file.'), 'export');
         }
 
-        // Log it
         Campaign::$plugin->log('File exported by "{username}".');
 
         return $this->response->sendFile($export->filePath);

diff --git a/src/controllers/ImportsController.php b/src/controllers/ImportsController.php
@@ -118,7 +118,9 @@ public function actionImportFile(): ?Response
 
         if (!$import->validate()) {
             $errors = implode('. ', $import->getErrorSummary(true));
-            Campaign::$plugin->log('Couldn’t import file. {errors}', ['errors' => $errors]);
+            Campaign::$plugin->log('Couldn’t import file. {errors}', [
+                'errors' => $errors,
+            ]);
 
             Craft::$app->getSession()->setError(Craft::t('campaign', 'Couldn’t import file.'));
 
@@ -136,7 +138,9 @@ public function actionImportFile(): ?Response
             }
         }
 
-        Campaign::$plugin->log('CSV file "{fileName}" imported by "{username}".', ['fileName' => $import->fileName]);
+        Campaign::$plugin->log('CSV file "{fileName}" imported by "{username}".', [
+            'fileName' => $import->fileName,
+        ]);
 
         return $this->asModelSuccess($import, Craft::t('campaign', 'CSV file successfully queued for importing.'), 'import');
     }
@@ -186,7 +190,9 @@ public function actionImportUserGroup(): ?Response
 
         if (!$import->validate()) {
             $errors = implode('. ', $import->getErrorSummary(true));
-            Campaign::$plugin->log('Couldn’t import user group. {errors}', ['errors' => $errors]);
+            Campaign::$plugin->log('Couldn’t import user group. {errors}', [
+                'errors' => $errors,
+            ]);
 
             Craft::$app->getSession()->setError(Craft::t('campaign', 'Couldn’t import user group.'));
 
@@ -204,8 +210,9 @@ public function actionImportUserGroup(): ?Response
             }
         }
 
-        // Log it
-        Campaign::$plugin->log('User group "{userGroup}" imported by "{username}".', ['userGroup' => $import->getUserGroup()->name]);
+        Campaign::$plugin->log('User group "{userGroup}" imported by "{username}".', [
+            'userGroup' => $import->getUserGroup()->name,
+        ]);
 
         return $this->asModelSuccess($import, Craft::t('campaign', 'User group successfully queued for importing.'), 'import');
     }

diff --git a/src/controllers/SendoutsController.php b/src/controllers/SendoutsController.php
@@ -255,28 +255,21 @@ public function actionPreview(int $sendoutId): Response
      */
     public function actionSend(): ?Response
     {
-        // Require permission to send
         $this->requirePermission('campaign:sendSendouts');
-
         $this->requirePostRequest();
 
         $sendout = $this->_getSendoutFromParamId();
-
-        // Store current user ID
         $sendout->senderId = Craft::$app->getUser()->getId();
-
-        // Set status to pending
         $sendout->sendStatus = SendoutElement::STATUS_PENDING;
 
-        // Save it
         if (!Craft::$app->getElements()->saveElement($sendout)) {
             return $this->asModelFailure($sendout, Craft::t('campaign', 'Couldn’t save sendout.'), 'sendout');
         }
 
-        // Log it
-        Campaign::$plugin->log('Sendout "{title}" initiated by "{username}".', ['title' => $sendout->title]);
+        Campaign::$plugin->log('Sendout "{title}" initiated by "{username}".', [
+            'title' => $sendout->title,
+        ]);
 
-        // Queue pending sendouts
         Campaign::$plugin->sendouts->queuePendingSendouts();
 
         return $this->asModelSuccess($sendout, Craft::t('campaign', 'Sendout saved.'), 'sendout');
@@ -322,8 +315,9 @@ public function actionPause(): ?Response
             return $this->asFailure(Craft::t('campaign', 'Sendout could not be paused.'));
         }
 
-        // Log it
-        Campaign::$plugin->log('Sendout "{title}" paused by "{username}".', ['title' => $sendout->title]);
+        Campaign::$plugin->log('Sendout "{title}" paused by "{username}".', [
+            'title' => $sendout->title,
+        ]);
 
         return $this->asSuccess(Craft::t('campaign', 'Sendout paused.'));
     }
@@ -341,8 +335,9 @@ public function actionCancel(): ?Response
             return $this->asFailure(Craft::t('campaign', 'Sendout could not be cancelled.'));
         }
 
-        // Log it
-        Campaign::$plugin->log('Sendout "{title}" cancelled by "{username}".', ['title' => $sendout->title]);
+        Campaign::$plugin->log('Sendout "{title}" cancelled by "{username}".', [
+            'title' => $sendout->title,
+        ]);
 
         return $this->asSuccess(Craft::t('campaign', 'Sendout cancelled.'));
     }

diff --git a/src/controllers/WebhookController.php b/src/controllers/WebhookController.php
@@ -288,7 +288,6 @@ public function actionSendgrid(): ?Response
      */
     private function _callWebhook(string $event, string $email = null): Response
     {
-        // Log request
         Campaign::$plugin->log('Webhook request: ' . $this->request->getRawBody(), [], Logger::LEVEL_WARNING);
 
         if ($email === null) {

diff --git a/src/elements/actions/CancelSendouts.php b/src/elements/actions/CancelSendouts.php
@@ -84,8 +84,9 @@ public function performAction(ElementQueryInterface $query): bool
         foreach ($sendouts as $sendout) {
             Campaign::$plugin->sendouts->cancelSendout($sendout);
 
-            // Log it
-            Campaign::$plugin->log('Sendout "{title}" cancelled by "{username}".', ['title' => $sendout->title]);
+            Campaign::$plugin->log('Sendout "{title}" cancelled by "{username}".', [
+                'title' => $sendout->title,
+            ]);
         }
 
         $this->setMessage(Craft::t('campaign', 'Sendouts cancelled.'));

diff --git a/src/elements/actions/PauseSendouts.php b/src/elements/actions/PauseSendouts.php
@@ -76,8 +76,9 @@ public function performAction(ElementQueryInterface $query): bool
         foreach ($sendouts as $sendout) {
             Campaign::$plugin->sendouts->pauseSendout($sendout);
 
-            // Log it
-            Campaign::$plugin->log('Sendout "{title}" paused by "{username}".', ['title' => $sendout->title]);
+            Campaign::$plugin->log('Sendout "{title}" paused by "{username}".', [
+                'title' => $sendout->title,
+            ]);
         }
 
         $this->setMessage(Craft::t('campaign', 'Sendouts paused.'));

diff --git a/src/jobs/SendoutJob.php b/src/jobs/SendoutJob.php
@@ -92,7 +92,7 @@ public function execute($queue): void
         $timeLimit = ($timeLimit == 0) ? null : round($timeLimit * $settings->timeThreshold);
 
         // Prepare sending
-        Campaign::$plugin->sendouts->prepareSending($sendout);
+        Campaign::$plugin->sendouts->prepareSending($sendout, $this->batch);
 
         // Get pending recipients
         $pendingRecipients = $sendout->getPendingRecipients();

diff --git a/src/services/CampaignTypesService.php b/src/services/CampaignTypesService.php
@@ -395,6 +395,8 @@ private function _campaignTypes(): MemoizableArray
     {
         if (!isset($this->_campaignTypes)) {
             $campaignTypes = [];
+
+            /** @var CampaignTypeRecord[] $campaignTypeRecords */
             $campaignTypeRecords = CampaignTypeRecord::find()
                 ->innerJoinWith('site')
                 ->orderBy(['name' => SORT_ASC])

diff --git a/src/services/ImportsService.php b/src/services/ImportsService.php
@@ -60,6 +60,8 @@ class ImportsService extends Component
     public function getAllImports(): array
     {
         $imports = [];
+
+        /** @var ImportRecord[] $importRecords */
         $importRecords = ImportRecord::find()->all();
 
         foreach ($importRecords as $importRecord) {

diff --git a/src/services/MailingListTypesService.php b/src/services/MailingListTypesService.php
@@ -105,6 +105,7 @@ function(MailingListTypeModel $mailingListType) use ($user) {
      */
     public function getMailingListTypeById(int $mailingListTypeId): ?MailingListTypeModel
     {
+        /** @var MailingListTypeRecord|null $mailingListTypeRecord */
         $mailingListTypeRecord = MailingListTypeRecord::find()
             ->innerJoinWith('site')
             ->where([MailingListTypeRecord::tableName() . '.id' => $mailingListTypeId])
@@ -125,6 +126,7 @@ public function getMailingListTypeById(int $mailingListTypeId): ?MailingListType
      */
     public function getMailingListTypeByHandle(string $mailingListTypeHandle): ?MailingListTypeModel
     {
+        /** @var MailingListTypeRecord|null $mailingListTypeRecord */
         $mailingListTypeRecord = MailingListTypeRecord::find()
             ->innerJoinWith('site')
             ->where([MailingListTypeRecord::tableName() . '.handle' => $mailingListTypeHandle])
@@ -420,6 +422,8 @@ private function _mailingListTypes(): MemoizableArray
     {
         if (!isset($this->_mailingListTypes)) {
             $mailingListTypes = [];
+
+            /** @var MailingListTypeRecord[] $mailingListTypeRecords */
             $mailingListTypeRecords = MailingListTypeRecord::find()
                 ->innerJoinWith('site')
                 ->orderBy(['name' => SORT_ASC])

diff --git a/src/services/PendingContactsService.php b/src/services/PendingContactsService.php
@@ -27,7 +27,7 @@ class PendingContactsService extends Component
      */
     public function getPendingContactByPid(string $pid): ?PendingContactModel
     {
-        // Get pending contact
+        /** @var PendingContactRecord|null $pendingContactRecord */
         $pendingContactRecord = PendingContactRecord::find()
             ->andWhere(['pid' => $pid])
             ->one();
@@ -173,7 +173,9 @@ public function purgeExpiredPendingContacts(): void
             $pendingContactRecord->delete();
 
             if (!$softDeleted) {
-                Campaign::$plugin->log('Deleted pending contact "{email}" because they took too long to verify their email.', ['email' => $pendingContactRecord->email]);
+                Campaign::$plugin->log('Deleted pending contact "{email}" because they took too long to verify their email.', [
+                    'email' => $pendingContactRecord->email,
+                ]);
             }
         }
     }

diff --git a/src/services/ReportsService.php b/src/services/ReportsService.php
@@ -229,7 +229,7 @@ public function getCampaignContactActivity(int $campaignId, string $interaction
      */
     public function getCampaignLinks(int $campaignId, int $limit = null): array
     {
-        // Get campaign links
+        /** @var LinkRecord[] $linkRecords */
         $linkRecords = LinkRecord::find()
             ->where(['campaignId' => $campaignId])
             ->orderBy(['clicked' => SORT_DESC, 'clicks' => SORT_DESC])

diff --git a/src/services/SendoutsService.php b/src/services/SendoutsService.php
@@ -435,26 +435,31 @@ public function sendNotification(SendoutElement $sendout): void
     /**
      * Prepares sending.
      */
-    public function prepareSending(SendoutElement $sendout): void
+    public function prepareSending(SendoutElement $sendout, ?int $batch = null): void
     {
         if ($sendout->sendStatus !== SendoutElement::STATUS_SENDING) {
             $sendout->sendStatus = SendoutElement::STATUS_SENDING;
 
             $this->_updateSendoutRecord($sendout, ['sendStatus']);
         }
 
-        // Set the current site from the sendout's site ID
+        // Set the current site from the sendout’s site ID
         Craft::$app->getSites()->setCurrentSite($sendout->siteId);
+
+        if ($batch !== null) {
+            Campaign::$plugin->log('Sending batch {batch} of sendout "{title}".', [
+                'batch' => $batch,
+                'title' => $sendout->title,
+            ]);
+        }
     }
 
     /**
      * Finalises sending.
      */
     public function finaliseSending(SendoutElement $sendout): void
     {
-        // If not failed
         if ($sendout->sendStatus != SendoutElement::STATUS_FAILED) {
-            // Change sending status to sent
             if ($sendout->sendStatus == SendoutElement::STATUS_SENDING) {
                 $sendout->sendStatus = SendoutElement::STATUS_SENT;
             }
@@ -466,9 +471,12 @@ public function finaliseSending(SendoutElement $sendout): void
             ) {
                 $sendout->sendStatus = SendoutElement::STATUS_PENDING;
             }
+
+            Campaign::$plugin->log('Sending of sendout "{title}" completed.', [
+                'title' => $sendout->title,
+            ]);
         }
 
-        // Get campaign
         $campaign = $sendout->getCampaign();
 
         if ($campaign !== null) {

diff --git a/src/services/SyncService.php b/src/services/SyncService.php
@@ -165,7 +165,9 @@ public function syncUserMailingList(User $user, MailingListElement $mailingList)
 
         if (!Craft::$app->getElements()->saveElement($contact)) {
             $errors = implode('. ', $contact->getErrorSummary(true));
-            Campaign::$plugin->log('Couldn’t sync user. {errors}', ['errors' => $errors]);
+            Campaign::$plugin->log('Couldn’t sync user. {errors}', [
+                'errors' => $errors,
+            ]);
 
             return;
         }

diff --git a/src/services/TrackerService.php b/src/services/TrackerService.php
@@ -57,6 +57,7 @@ public function click(ContactElement $contact, SendoutElement $sendout, LinkReco
      */
     public function unsubscribe(ContactElement $contact, SendoutElement $sendout): ?MailingListElement
     {
+        /** @var ContactCampaignRecord|null $contactCampaignRecord */
         $contactCampaignRecord = ContactCampaignRecord::find()
             ->where([
                 'contactId' => $contact->id,

diff --git a/src/services/WebhookService.php b/src/services/WebhookService.php
@@ -19,7 +19,9 @@ class WebhookService extends Component
      */
     public function complain(ContactElement $contact): void
     {
-        Campaign::$plugin->log('Contact {email} marked as "complained".', ['email' => $contact->email]);
+        Campaign::$plugin->log('Contact {email} marked as "complained".', [
+            'email' => $contact->email,
+        ]);
 
         $this->_addInteraction($contact, 'complained');
     }
@@ -29,7 +31,9 @@ public function complain(ContactElement $contact): void
      */
     public function bounce(ContactElement $contact): void
     {
-        Campaign::$plugin->log('Contact {email} marked as "bounced".', ['email' => $contact->email]);
+        Campaign::$plugin->log('Contact {email} marked as "bounced".', [
+            'email' => $contact->email,
+        ]);
 
         $this->_addInteraction($contact, 'bounced');
     }
@@ -39,7 +43,9 @@ public function bounce(ContactElement $contact): void
      */
     public function unsubscribe(ContactElement $contact): void
     {
-        Campaign::$plugin->log('Contact {email} marked as "unsubscribed".', ['email' => $contact->email]);
+        Campaign::$plugin->log('Contact {email} marked as "unsubscribed".', [
+            'email' => $contact->email,
+        ]);
 
         $this->_addInteraction($contact, 'unsubscribed');
     }