From 57623a4ceeb1f8f8b03a01b7c8b4954dc9defd09 Mon Sep 17 00:00:00 2001 From: Sunil Pawar Date: Mon, 2 Jul 2018 21:10:02 +0530 Subject: [PATCH 1/3] dev/report/#5 - Fix mailing report unique count issue --- CRM/Mailing/BAO/Mailing.php | 7 +++++++ CRM/Report/Form/Mailing/Opened.php | 18 ++++++++++++------ templates/CRM/Mailing/Page/Report.tpl | 2 +- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/CRM/Mailing/BAO/Mailing.php b/CRM/Mailing/BAO/Mailing.php index b0583b56eb8b..03f6afb4d0dc 100644 --- a/CRM/Mailing/BAO/Mailing.php +++ b/CRM/Mailing/BAO/Mailing.php @@ -2198,6 +2198,7 @@ public static function &report($id, $skipDetails = FALSE, $isSMS = FALSE) { 'forward', 'reply', 'opened', + 'opened_unique', 'optout', ) as $key) { $url = 'mailing/detail'; @@ -2235,6 +2236,12 @@ public static function &report($id, $skipDetails = FALSE, $isSMS = FALSE) { break; case 'opened': + $url = "mailing/opened"; + $searchFilter .= "&mailing_open_status=Y"; + $reportFilter .= "&distinct=0"; // do not use group by clause in report, because same report used for total and unique open + break; + + case 'opened_unique': $url = "mailing/opened"; $searchFilter .= "&mailing_open_status=Y"; break; diff --git a/CRM/Report/Form/Mailing/Opened.php b/CRM/Report/Form/Mailing/Opened.php index e893b75fc86a..98ba27413727 100644 --- a/CRM/Report/Form/Mailing/Opened.php +++ b/CRM/Report/Form/Mailing/Opened.php @@ -289,13 +289,19 @@ public function where() { } public function groupBy() { - $groupBys = empty($this->_params['charts']) ? array("civicrm_mailing_event_queue.email_id") : array("{$this->_aliases['civicrm_mailing']}.id"); - - if (!empty($this->_params['unique_opens_value'])) { - $groupBys[] = "civicrm_mailing_event_queue.id"; + $groupBys = array(); + // Do not use group by clause if distinct = 0 mentioned in url params. flag is used in mailing report screen, default value is TRUE + // this report is used to show total opened and unique opened + if (CRM_Utils_Request::retrieve('distinct', 'Boolean', CRM_Core_DAO::$_nullObject, FALSE, TRUE)) { + $groupBys = empty($this->_params['charts']) ? array("civicrm_mailing_event_queue.email_id") : array("{$this->_aliases['civicrm_mailing']}.id"); + if (!empty($this->_params['unique_opens_value'])) { + $groupBys[] = "civicrm_mailing_event_queue.id"; + } + } + if (!empty($groupBys)) { + $this->_select = CRM_Contact_BAO_Query::appendAnyValueToSelect($this->_selectClauses, $groupBys); + $this->_groupBy = "GROUP BY " . implode(', ', $groupBys); } - $this->_select = CRM_Contact_BAO_Query::appendAnyValueToSelect($this->_selectClauses, $groupBys); - $this->_groupBy = "GROUP BY " . implode(', ', $groupBys); } public function postProcess() { diff --git a/templates/CRM/Mailing/Page/Report.tpl b/templates/CRM/Mailing/Page/Report.tpl index 0767df8c743f..9c95b3604692 100644 --- a/templates/CRM/Mailing/Page/Report.tpl +++ b/templates/CRM/Mailing/Page/Report.tpl @@ -37,7 +37,7 @@ {if $report.mailing.open_tracking} {ts}Unique Opens{/ts} {$report.event_totals.opened} ({$report.event_totals.opened_rate|string_format:"%0.2f"}%) - {$report.event_totals.actionlinks.opened} + {$report.event_totals.actionlinks.opened_unique} {ts}Total Opens{/ts} {$report.event_totals.total_opened} {$report.event_totals.actionlinks.opened} From b736d2b6b26a831d605b1ad5a9518ef99743e59e Mon Sep 17 00:00:00 2001 From: Sunil Pawar Date: Wed, 11 Jul 2018 16:15:30 +0530 Subject: [PATCH 2/3] Test case for report --- tests/phpunit/api/v3/ReportTemplateTest.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php index 91dec1b95660..1f4ff7e1fcac 100644 --- a/tests/phpunit/api/v3/ReportTemplateTest.php +++ b/tests/phpunit/api/v3/ReportTemplateTest.php @@ -211,6 +211,19 @@ public function testReportTemplateGetRowsContactSummary() { */ } + /** + * Test getrows on Mailing Opened report. + */ + public function testReportTemplateGetRowsMailingUniqueOpened() { + $description = "Retrieve rows from a mailing opened report template."; + $result = $this->callAPIAndDocument('report_template', 'getrows', array( + 'report_id' => 'Mailing/opened', + 'options' => array('metadata' => array('labels', 'title')), + ), __FUNCTION__, __FILE__, $description, 'Getrows'); + + $this->assertEquals(4, $result['count']); + } + /** * Test api to get rows from reports. * From ead1a15e04a730ddd3634e75ea84c0d34974a9ac Mon Sep 17 00:00:00 2001 From: Monish Deb Date: Wed, 19 Dec 2018 20:11:58 +0530 Subject: [PATCH 3/3] Added test case --- CRM/Mailing/BAO/Mailing.php | 4 --- CRM/Report/Form/Mailing/Opened.php | 1 - tests/phpunit/api/v3/ReportTemplateTest.php | 29 +++++++++++++++++++-- 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/CRM/Mailing/BAO/Mailing.php b/CRM/Mailing/BAO/Mailing.php index 03f6afb4d0dc..434355d1e56e 100644 --- a/CRM/Mailing/BAO/Mailing.php +++ b/CRM/Mailing/BAO/Mailing.php @@ -2236,11 +2236,7 @@ public static function &report($id, $skipDetails = FALSE, $isSMS = FALSE) { break; case 'opened': - $url = "mailing/opened"; - $searchFilter .= "&mailing_open_status=Y"; $reportFilter .= "&distinct=0"; // do not use group by clause in report, because same report used for total and unique open - break; - case 'opened_unique': $url = "mailing/opened"; $searchFilter .= "&mailing_open_status=Y"; diff --git a/CRM/Report/Form/Mailing/Opened.php b/CRM/Report/Form/Mailing/Opened.php index 98ba27413727..62a076dc34f9 100644 --- a/CRM/Report/Form/Mailing/Opened.php +++ b/CRM/Report/Form/Mailing/Opened.php @@ -299,7 +299,6 @@ public function groupBy() { } } if (!empty($groupBys)) { - $this->_select = CRM_Contact_BAO_Query::appendAnyValueToSelect($this->_selectClauses, $groupBys); $this->_groupBy = "GROUP BY " . implode(', ', $groupBys); } } diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php index 1f4ff7e1fcac..c758370f08f0 100644 --- a/tests/phpunit/api/v3/ReportTemplateTest.php +++ b/tests/phpunit/api/v3/ReportTemplateTest.php @@ -192,7 +192,7 @@ public function hookSelectWhere($entity, &$clauses) { */ public function testReportTemplateGetRowsContactSummary() { $description = "Retrieve rows from a report template (optionally providing the instance_id)."; - $result = $this->callAPIAndDocument('report_template', 'getrows', array( + $result = $this->callApiSuccess('report_template', 'getrows', array( 'report_id' => 'contact/summary', 'options' => array('metadata' => array('labels', 'title')), ), __FUNCTION__, __FILE__, $description, 'Getrows'); @@ -216,12 +216,37 @@ public function testReportTemplateGetRowsContactSummary() { */ public function testReportTemplateGetRowsMailingUniqueOpened() { $description = "Retrieve rows from a mailing opened report template."; + $op = new PHPUnit_Extensions_Database_Operation_Insert(); + $op->execute($this->_dbconn, + $this->createFlatXMLDataSet( + dirname(__FILE__) . '/../../CRM/Mailing/BAO/queryDataset.xml' + ) + ); + + // Check total rows without distinct + global $_REQUEST; + $_REQUEST['distinct'] = 0; $result = $this->callAPIAndDocument('report_template', 'getrows', array( 'report_id' => 'Mailing/opened', 'options' => array('metadata' => array('labels', 'title')), ), __FUNCTION__, __FILE__, $description, 'Getrows'); + $this->assertEquals(14, $result['count']); - $this->assertEquals(4, $result['count']); + // Check total rows with distinct + $_REQUEST['distinct'] = 1; + $result = $this->callAPIAndDocument('report_template', 'getrows', array( + 'report_id' => 'Mailing/opened', + 'options' => array('metadata' => array('labels', 'title')), + ), __FUNCTION__, __FILE__, $description, 'Getrows'); + $this->assertEquals(5, $result['count']); + + // Check total rows with distinct by passing NULL value to distinct parameter + $_REQUEST['distinct'] = NULL; + $result = $this->callAPIAndDocument('report_template', 'getrows', array( + 'report_id' => 'Mailing/opened', + 'options' => array('metadata' => array('labels', 'title')), + ), __FUNCTION__, __FILE__, $description, 'Getrows'); + $this->assertEquals(5, $result['count']); } /**