From: Aidan Saunders <aidan.saunders@squiffle.uk>
Date: Wed, 27 Nov 2019 12:13:15 +0000 (+0000)
Subject: Fix dev/report#23
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=f8dcf33849222a773b4f7dada02017706a8a00a9;p=civicrm-core.git

Fix dev/report#23
When filtering by contact sub-type using 'is not one of' do not exclude where sub-type is null.

Add unit tests for contact sub-type 'is one of' and 'is not one of'
Tighten the matching on sub-type to avoid problems where one contact sub-type is a substring of another.
---

diff --git a/CRM/Report/Form.php b/CRM/Report/Form.php
index f4ba42cae7..c07f760b50 100644
--- a/CRM/Report/Form.php
+++ b/CRM/Report/Form.php
@@ -2114,32 +2114,34 @@ class CRM_Report_Form extends CRM_Core_Form {
    */
   public function whereSubtypeClause($field, $value, $op) {
     // Get the correct SQL operator.
+    $orNull = FALSE;
     switch ($op) {
       case 'notin':
         $op = 'nhas';
-        $clauseSeparator = 'AND';
+        $clauseSeparator = ' AND ';
+        $orNull = TRUE;
         break;
 
       case 'in':
         $op = 'has';
-        $clauseSeparator = 'OR';
+        $clauseSeparator = ' OR ';
         break;
     }
     $sqlOp = $this->getSQLOperator($op);
-    $clause = '( ';
-    $subtypeFilters = count($value);
     if ($sqlOp == 'IS NULL' || $sqlOp == 'IS NOT NULL') {
-      $clause .= "{$field['dbAlias']} $sqlOp";
+      $clause = "{$field['dbAlias']} $sqlOp";
     }
     else {
-      for ($i = 0; $i < $subtypeFilters; $i++) {
-        $clause .= "{$field['dbAlias']} $sqlOp '%$value[$i]%'";
-        if ($i !== ($subtypeFilters - 1)) {
-          $clause .= " $clauseSeparator ";
-        }
+      $subclauses = [];
+      foreach ($value as $item) {
+        $subclauses[] = "( {$field['dbAlias']} $sqlOp '%" . CRM_Core_DAO::VALUE_SEPARATOR . $item . CRM_Core_DAO::VALUE_SEPARATOR . "%' )";
       }
+      $clause = implode($clauseSeparator, $subclauses);
+    }
+    $clause = "( $clause )";
+    if ($orNull) {
+      $clause = "( ( {$field['dbAlias']} IS NULL ) OR $clause )";
     }
-    $clause .= ' )';
     return $clause;
   }
 
diff --git a/tests/phpunit/api/v3/ReportTemplateTest.php b/tests/phpunit/api/v3/ReportTemplateTest.php
index dbc0b63186..872d6cc588 100644
--- a/tests/phpunit/api/v3/ReportTemplateTest.php
+++ b/tests/phpunit/api/v3/ReportTemplateTest.php
@@ -1145,6 +1145,44 @@ class api_v3_ReportTemplateTest extends CiviUnitTestCase {
     $this->assertEquals(1, $rows['count']);
   }
 
+  /**
+   * Test contact subtype filter on summary report.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testContactSubtypeIn() {
+    $this->individualCreate(['contact_sub_type' => ['Student', 'Parent']]);
+    $this->individualCreate();
+
+    $rows = $this->callAPISuccess('report_template', 'getrows', [
+      'report_id' => 'contact/summary',
+      'contact_sub_type_op' => 'in',
+      'contact_sub_type_value' => ['Student'],
+      'contact_type_op' => 'in',
+      'contact_type_value' => 'Individual',
+    ]);
+    $this->assertEquals(1, $rows['count']);
+  }
+
+  /**
+   * Test contact subtype filter on summary report.
+   *
+   * @throws \CRM_Core_Exception
+   */
+  public function testContactSubtypeNotIn() {
+    $this->individualCreate(['contact_sub_type' => ['Student', 'Parent']]);
+    $this->individualCreate();
+
+    $rows = $this->callAPISuccess('report_template', 'getrows', [
+      'report_id' => 'contact/summary',
+      'contact_sub_type_op' => 'notin',
+      'contact_sub_type_value' => ['Student'],
+      'contact_type_op' => 'in',
+      'contact_type_value' => 'Individual',
+    ]);
+    $this->assertEquals(1, $rows['count']);
+  }
+
   /**
    * Test PCP report to ensure total donors and total committed is accurate.
    */