Remove cloning hack from export and add unit tests

civicrm · Feb 22, 2018 · ec4f44d · ec4f44d
1 parent b810074
commit ec4f44d
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 16 deletions.
diff --git a/CRM/Export/BAO/Export.php b/CRM/Export/BAO/Export.php
@@ -770,27 +770,16 @@ public static function exportComponents(
     $limitReached = FALSE;
     while (!$limitReached) {
       $limitQuery = "{$queryString} LIMIT {$offset}, {$rowCount}";
-      $dao = CRM_Core_DAO::executeQuery($limitQuery);
+      $iterationDAO = CRM_Core_DAO::executeQuery($limitQuery);
       // If this is less than our limit by the end of the iteration we do not need to run the query again to
       // check if some remain.
       $rowsThisIteration = 0;
 
-      while ($dao->fetch()) {
+      while ($iterationDAO->fetch()) {
         $count++;
         $rowsThisIteration++;
         $row = array();
-
-        //convert the pseudo constants
-        // CRM-14398 there is problem in this architecture that is not easily solved. For now we are using the cloned
-        // temporary iterationDAO object to get around it.
-        // the issue is that the convertToPseudoNames function is adding additional properties (e.g for campaign) to the DAO object
-        // these additional properties are NOT reset when the $dao cycles through the while loop
-        // nor are they overwritten as they are not in the loop
-        // the convertToPseudoNames will not adequately over-write them either as it doesn't 'kick-in' unless the
-        // relevant property is set.
-        // It may be that a long-term fix could be introduced there - however, it's probably necessary to figure out how to test the
-        // export class before tackling a better architectural fix
-        $iterationDAO = clone $dao;
+        $queryProperties = get_object_vars($iterationDAO);
         $query->convertToPseudoNames($iterationDAO);
 
         //first loop through output columns so that we return what is required, and in same order.
@@ -1070,7 +1059,6 @@ public static function exportComponents(
           $componentDetails = array();
         }
       }
-      $dao->free();
       if ($rowsThisIteration < self::EXPORT_ROW_COUNT) {
         $limitReached = TRUE;
       }

diff --git a/api/v3/Campaign.php b/api/v3/Campaign.php
@@ -58,6 +58,7 @@ function civicrm_api3_campaign_create($params) {
  */
 function _civicrm_api3_campaign_create_spec(&$params) {
   $params['title']['api.required'] = 1;
+  $params['is_active']['api.default'] = 1;
 }
 
 /**

diff --git a/tests/phpunit/CRM/Export/BAO/ExportTest.php b/tests/phpunit/CRM/Export/BAO/ExportTest.php
@@ -204,6 +204,7 @@ public function testGetExportStructureArrays() {
   public function setUpContributionExportData() {
     $this->setUpContactExportData();
     $this->contributionIDs[] = $this->contributionCreate(array('contact_id' => $this->contactIDs[0]));
+    $this->contributionIDs[] = $this->contributionCreate(array('contact_id' => $this->contactIDs[1], 'trxn_id' => 'null', 'invoice_id' => 'null'));
   }
 
   /**
@@ -218,7 +219,7 @@ public function setUpActivityExportData() {
    * Set up some data for us to do testing on.
    */
   public function setUpContactExportData() {
-    $this->contactIDs[] = $contactA = $this->individualCreate();
+    $this->contactIDs[] = $contactA = $this->individualCreate(['gender_id' => 'Female']);
     // Create address for contact A.
     $params = array(
       'contact_id' => $contactA,
@@ -300,6 +301,65 @@ public function getPrimarySearchOptions() {
     return [[TRUE], [FALSE]];
   }
 
+  /**
+   * Test that when exporting a pseudoField it is reset for NULL entries.
+   *
+   * ie. we have a contact WITH a gender & one without - make sure the latter one
+   * does NOT retain the gender of the former.
+   */
+  public function testExportPseudoField() {
+    $this->setUpContactExportData();
+    $selectedFields = [['Individual', 'gender_id']];
+    list($tableName, $sqlColumns) = CRM_Export_BAO_Export::exportComponents(
+      TRUE,
+      $this->contactIDs[1],
+      array(),
+      NULL,
+      $selectedFields,
+      NULL,
+      CRM_Export_Form_Select::CONTACT_EXPORT,
+      "contact_a.id IN (" . implode(",", $this->contactIDs) . ")",
+      NULL,
+      FALSE,
+      FALSE,
+      array(
+        'exportOption' => CRM_Export_Form_Select::CONTACT_EXPORT,
+        'suppress_csv_for_testing' => TRUE,
+      )
+    );
+    $this->assertEquals('Female,', CRM_Core_DAO::singleValueQuery("SELECT GROUP_CONCAT(gender_id) FROM {$tableName}"));
+  }
+
+  /**
+   * Test that when exporting a pseudoField it is reset for NULL entries.
+   *
+   * This is specific to the example in CRM-14398
+   */
+  public function testExportPseudoFieldCampaign() {
+    $this->setUpContributionExportData();
+    $campaign = $this->callAPISuccess('Campaign', 'create', ['title' => 'Big campaign']);
+    $this->callAPISuccess('Contribution', 'create', ['campaign_id' => 'Big_campaign', 'id' => $this->contributionIDs[0]]);
+    $selectedFields = [['Individual', 'gender_id'], ['Contribution', 'contribution_campaign_title']];
+    list($tableName, $sqlColumns) = CRM_Export_BAO_Export::exportComponents(
+      TRUE,
+      $this->contactIDs[1],
+      array(),
+      NULL,
+      $selectedFields,
+      NULL,
+      CRM_Export_Form_Select::CONTRIBUTE_EXPORT,
+      "contact_a.id IN (" . implode(",", $this->contactIDs) . ")",
+      NULL,
+      FALSE,
+      FALSE,
+      array(
+        'exportOption' => CRM_Export_Form_Select::CONTACT_EXPORT,
+        'suppress_csv_for_testing' => TRUE,
+      )
+    );
+    $this->assertEquals('Big campaign,', CRM_Core_DAO::singleValueQuery("SELECT GROUP_CONCAT(contribution_campaign_title) FROM {$tableName}"));
+  }
+
   /**
    * Test master_address_id field.
    */