From 2f2240dc5f8a9e29ae93bef5dac0129ed4ea5aaf Mon Sep 17 00:00:00 2001 From: Lars SG Date: Sat, 17 Apr 2021 10:47:15 -0600 Subject: [PATCH] Change to unique clicks and opens, clarify unsubs Show 0 instead of n/a for stats that don't yet have any count Before: Bounces, Open and Unsubs show n/a if there are 0. After: All stats show 0 if api returns null. Add intended recipients to API and A/B mailing report page Remove in_distinct parameter from Bounce and Delivered in API and BAO First commit removes is_distinct code from getTotalCount() for Bounces and Delivered in BAO, so we no longer need the is_distinct parameter for these functions, as it doesn't do anything. Changed API to match. These functions do not appear to be used elsewhere. Fix comments for removed param for getTotalCount Removing is_distrinct from getTotalCount Forgot this! Update mailing stats test Update label for Tracked opens to Unique Opens, Click-throughs to Unique Clicks Thanks @sluc23 for pointing this out --- CRM/Mailing/Event/BAO/Bounce.php | 8 +------- CRM/Mailing/Event/BAO/Delivered.php | 12 +----------- ang/crmMailing/services.js | 20 ++++++++++---------- ang/crmMailingAB/EditCtrl/report.html | 2 +- api/v3/Mailing.php | 12 +++++++++--- tests/phpunit/api/v3/MailingTest.php | 1 + 6 files changed, 23 insertions(+), 32 deletions(-) diff --git a/CRM/Mailing/Event/BAO/Bounce.php b/CRM/Mailing/Event/BAO/Bounce.php index 57c8c3a99b..cefde07fb4 100644 --- a/CRM/Mailing/Event/BAO/Bounce.php +++ b/CRM/Mailing/Event/BAO/Bounce.php @@ -91,15 +91,13 @@ class CRM_Mailing_Event_BAO_Bounce extends CRM_Mailing_Event_DAO_Bounce { * ID of the mailing. * @param int $job_id * Optional ID of a job to filter on. - * @param bool $is_distinct - * Group by queue ID?. * * @param string|null $toDate * * @return int * Number of rows in result set */ - public static function getTotalCount($mailing_id, $job_id = NULL, $is_distinct = FALSE, $toDate = NULL) { + public static function getTotalCount($mailing_id, $job_id = NULL, $toDate = NULL) { $dao = new CRM_Core_DAO(); $bounce = self::getTableName(); @@ -126,10 +124,6 @@ class CRM_Mailing_Event_BAO_Bounce extends CRM_Mailing_Event_DAO_Bounce { $query .= " AND $job.id = " . CRM_Utils_Type::escape($job_id, 'Integer'); } - if ($is_distinct) { - $query .= " GROUP BY $queue.id "; - } - // query was missing $dao->query($query); diff --git a/CRM/Mailing/Event/BAO/Delivered.php b/CRM/Mailing/Event/BAO/Delivered.php index e236767c2e..10f5235195 100644 --- a/CRM/Mailing/Event/BAO/Delivered.php +++ b/CRM/Mailing/Event/BAO/Delivered.php @@ -68,14 +68,12 @@ class CRM_Mailing_Event_BAO_Delivered extends CRM_Mailing_Event_DAO_Delivered { * ID of the mailing. * @param int $job_id * Optional ID of a job to filter on. - * @param bool $is_distinct - * Group by queue ID?. * @param string $toDate * * @return int * Number of rows in result set */ - public static function getTotalCount($mailing_id, $job_id = NULL, $is_distinct = FALSE, $toDate = NULL) { + public static function getTotalCount($mailing_id, $job_id = NULL, $toDate = NULL) { $dao = new CRM_Core_DAO(); $delivered = self::getTableName(); @@ -107,10 +105,6 @@ class CRM_Mailing_Event_BAO_Delivered extends CRM_Mailing_Event_DAO_Delivered { $query .= " AND $job.id = " . CRM_Utils_Type::escape($job_id, 'Integer'); } - if ($is_distinct) { - $query .= " GROUP BY $queue.id "; - } - // query was missing $dao->query($query); @@ -184,10 +178,6 @@ class CRM_Mailing_Event_BAO_Delivered extends CRM_Mailing_Event_DAO_Delivered { $query .= " AND $job.id = " . CRM_Utils_Type::escape($job_id, 'Integer'); } - if ($is_distinct) { - $query .= " GROUP BY $queue.id, $delivered.id"; - } - $orderBy = "sort_name ASC, {$delivered}.time_stamp DESC"; if ($sort) { if (is_string($sort)) { diff --git a/ang/crmMailing/services.js b/ang/crmMailing/services.js index fc5ab146c7..abbdb9b4af 100644 --- a/ang/crmMailing/services.js +++ b/ang/crmMailing/services.js @@ -481,15 +481,15 @@ angular.module('crmMailing').factory('crmMailingStats', function (crmApi, crmLegacy) { var statTypes = [ - // {name: 'Recipients', title: ts('Intended Recipients'), searchFilter: '', eventsFilter: '&event=queue', reportType: 'detail', reportFilter: ''}, - {name: 'Delivered', title: ts('Successful Deliveries'), searchFilter: '&mailing_delivery_status=Y', eventsFilter: '&event=delivered', reportType: 'detail', reportFilter: '&delivery_status_value=successful'}, - {name: 'Opened', title: ts('Tracked Opens'), searchFilter: '&mailing_open_status=Y', eventsFilter: '&event=opened', reportType: 'opened', reportFilter: ''}, - {name: 'Unique Clicks', title: ts('Click-throughs'), searchFilter: '&mailing_click_status=Y', eventsFilter: '&event=click&distinct=1', reportType: 'clicks', reportFilter: ''}, - // {name: 'Forward', title: ts('Forwards'), searchFilter: '&mailing_forward=1', eventsFilter: '&event=forward', reportType: 'detail', reportFilter: '&is_forwarded_value=1'}, - // {name: 'Replies', title: ts('Replies'), searchFilter: '&mailing_reply_status=Y', eventsFilter: '&event=reply', reportType: 'detail', reportFilter: '&is_replied_value=1'}, - {name: 'Bounces', title: ts('Bounces'), searchFilter: '&mailing_delivery_status=N', eventsFilter: '&event=bounce', reportType: 'bounce', reportFilter: ''}, - {name: 'Unsubscribers', title: ts('Unsubscribes'), searchFilter: '&mailing_unsubscribe=1', eventsFilter: '&event=unsubscribe', reportType: 'detail', reportFilter: '&is_unsubscribed_value=1'}, - // {name: 'OptOuts', title: ts('Opt-Outs'), searchFilter: '&mailing_optout=1', eventsFilter: '&event=optout', reportType: 'detail', reportFilter: ''} + {name: 'Recipients', title: ts('Intended Recipients'), searchFilter: '', eventsFilter: '&event=queue', reportType: 'detail', reportFilter: ''}, + {name: 'Delivered', title: ts('Successful Deliveries'), searchFilter: '&mailing_delivery_status=Y', eventsFilter: '&event=delivered', reportType: 'detail', reportFilter: '&delivery_status_value=successful'}, + {name: 'Opened', title: ts('Unique Opens'), searchFilter: '&mailing_open_status=Y', eventsFilter: '&event=opened', reportType: 'opened', reportFilter: ''}, + {name: 'Unique Clicks', title: ts('Unique Clicks'), searchFilter: '&mailing_click_status=Y', eventsFilter: '&event=click&distinct=1', reportType: 'clicks', reportFilter: ''}, + // {name: 'Forward', title: ts('Forwards'), searchFilter: '&mailing_forward=1', eventsFilter: '&event=forward', reportType: 'detail', reportFilter: '&is_forwarded_value=1'}, + // {name: 'Replies', title: ts('Replies'), searchFilter: '&mailing_reply_status=Y', eventsFilter: '&event=reply', reportType: 'detail', reportFilter: '&is_replied_value=1'}, + {name: 'Bounces', title: ts('Bounces'), searchFilter: '&mailing_delivery_status=N', eventsFilter: '&event=bounce', reportType: 'bounce', reportFilter: ''}, + {name: 'Unsubscribers', title: ts('Unsubscribes & Opt-outs'), searchFilter: '&mailing_unsubscribe=1', eventsFilter: '&event=unsubscribe', reportType: 'detail', reportFilter: '&is_unsubscribed_value=1'}, + // {name: 'OptOuts', title: ts('Opt-Outs'), searchFilter: '&mailing_optout=1', eventsFilter: '&event=optout', reportType: 'detail', reportFilter: ''} ]; return { @@ -507,7 +507,7 @@ getStats: function(mailingIds) { var params = {}; angular.forEach(mailingIds, function(mailingId, name) { - params[name] = ['Mailing', 'stats', {mailing_id: mailingId, is_distinct: 0}]; + params[name] = ['Mailing', 'stats', {mailing_id: mailingId, is_distinct: 1}]; }); return crmApi(params).then(function(result) { var stats = {}; diff --git a/ang/crmMailingAB/EditCtrl/report.html b/ang/crmMailingAB/EditCtrl/report.html index 5e97ae964a..8595172d5e 100644 --- a/ang/crmMailingAB/EditCtrl/report.html +++ b/ang/crmMailingAB/EditCtrl/report.html @@ -78,7 +78,7 @@ class="crm-hover-button action-item" ng-href="{{statUrl(am.mailing, statType, 'events')}}" title="{{ts('Browse events of type \'%1\'', {1: statType.title})}}" - >{{stats[am.name][statType.name] || ts('n/a')}} {{stats[am.name][rateStats[statType.name]] || ' '}} + >{{stats[am.name][statType.name] || 0}} {{stats[am.name][rateStats[statType.name]] || ' '}} CRM_Mailing_Event_BAO_Queue::getTotalCount($params['mailing_id'], $params['job_id']), + ]; + break; + case 'Delivered': $stats[$params['mailing_id']] += [ - $detail => CRM_Mailing_Event_BAO_Delivered::getTotalCount($params['mailing_id'], $params['job_id'], (bool) $params['is_distinct'], $params['date']), + $detail => CRM_Mailing_Event_BAO_Delivered::getTotalCount($params['mailing_id'], $params['job_id'], $params['date']), ]; break; case 'Bounces': $stats[$params['mailing_id']] += [ - $detail => CRM_Mailing_Event_BAO_Bounce::getTotalCount($params['mailing_id'], $params['job_id'], (bool) $params['is_distinct'], $params['date']), + $detail => CRM_Mailing_Event_BAO_Bounce::getTotalCount($params['mailing_id'], $params['job_id'], $params['date']), ]; break; diff --git a/tests/phpunit/api/v3/MailingTest.php b/tests/phpunit/api/v3/MailingTest.php index f2e325fe80..e385bfc2d7 100644 --- a/tests/phpunit/api/v3/MailingTest.php +++ b/tests/phpunit/api/v3/MailingTest.php @@ -776,6 +776,7 @@ SELECT event_queue_id, time_stamp FROM {$temporaryTableName}"; $result = $this->callAPISuccess('mailing', 'stats', ['mailing_id' => $mail['id']]); $expectedResult = [ //since among 100 mails 20 has been bounced + 'Recipients' => 100, 'Delivered' => 80, 'Bounces' => 20, 'Opened' => 20, -- 2.25.1