Skip to content

Commit

Permalink
Merge pull request #11703 from eileenmcnaughton/export
Browse files Browse the repository at this point in the history
Remove cloning hack from export and add unit tests
  • Loading branch information
monishdeb authored Mar 29, 2018
2 parents db5c67e + bab4f15 commit d902822
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 17 deletions.
17 changes: 2 additions & 15 deletions CRM/Export/BAO/Export.php
Original file line number Diff line number Diff line change
Expand Up @@ -770,27 +770,15 @@ 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;
$query->convertToPseudoNames($iterationDAO);

//first loop through output columns so that we return what is required, and in same order.
Expand Down Expand Up @@ -1070,7 +1058,6 @@ public static function exportComponents(
$componentDetails = array();
}
}
$dao->free();
if ($rowsThisIteration < self::EXPORT_ROW_COUNT) {
$limitReached = TRUE;
}
Expand Down
1 change: 1 addition & 0 deletions api/v3/Campaign.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
65 changes: 63 additions & 2 deletions tests/phpunit/CRM/Export/BAO/ExportTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {

public function tearDown() {
$this->quickCleanup(['civicrm_contact', 'civicrm_email', 'civicrm_address']);
$this->quickCleanUpFinancialEntities();
parent::tearDown();
}

Expand Down Expand Up @@ -203,7 +204,8 @@ 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[0], 'trxn_id' => 'null', 'invoice_id' => 'null'));
$this->contributionIDs[] = $this->contributionCreate(array('contact_id' => $this->contactIDs[1], 'trxn_id' => 'null', 'invoice_id' => 'null'));
}

/**
Expand All @@ -218,7 +220,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,
Expand Down Expand Up @@ -300,6 +302,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.
*/
Expand Down

0 comments on commit d902822

Please sign in to comment.