From: eileen <emcnaughton@wikimedia.org>
Date: Tue, 9 Jul 2019 04:43:26 +0000 (+1200)
Subject: [REF] export code simplification
X-Git-Url: https://vcs.fsf.org/?a=commitdiff_plain;h=12a6fdef6678def37e39e076951ce0c7336b28c7;p=civicrm-core.git
[REF] export code simplification
On review I find the value ['exportOption'] is a parameter used at the form later to determine whether to
export 'Primary fields only' (ahem) or the user should select an array of fields. By the time the form is submitted we
either have an array of selected fields or NULL denoting that there is no selection (& hence the defaults should be used)
and the selection at the form layer is redundant. However it is still being referenced in one place (and hence the tests
are passing it in to avoid an enotice). This fixes that one place & removes it from the tests. It also extracts part of
a test to move towards making it easier to alter the signature of exportComponents without messing with so many places in
the code
---
diff --git a/CRM/Export/BAO/Export.php b/CRM/Export/BAO/Export.php
index b4a3c76319..3b5c5c4c03 100644
--- a/CRM/Export/BAO/Export.php
+++ b/CRM/Export/BAO/Export.php
@@ -224,12 +224,11 @@ INSERT INTO {$componentTable} SELECT distinct gc.contact_id FROM civicrm_group_c
$returnProperties = array_merge($returnProperties, $moreReturnProperties);
}
- $exportParams['postal_mailing_export']['temp_columns'] = [];
- if ($exportParams['exportOption'] == 2 &&
- isset($exportParams['postal_mailing_export']) &&
- CRM_Utils_Array::value('postal_mailing_export', $exportParams['postal_mailing_export']) == 1
+ if ($processor->getRequestedFields() &&
+ $processor->isPostalableOnly()
) {
$postalColumns = ['is_deceased', 'do_not_mail', 'street_address', 'supplemental_address_1'];
+ $exportParams['postal_mailing_export']['temp_columns'] = [];
foreach ($postalColumns as $column) {
if (!array_key_exists($column, $returnProperties)) {
$returnProperties[$column] = 1;
diff --git a/tests/phpunit/CRM/Export/BAO/ExportTest.php b/tests/phpunit/CRM/Export/BAO/ExportTest.php
index fc0fe1037b..c45e61056f 100644
--- a/tests/phpunit/CRM/Export/BAO/ExportTest.php
+++ b/tests/phpunit/CRM/Export/BAO/ExportTest.php
@@ -87,29 +87,7 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
* @throws \League\Csv\Exception
*/
public function testExportComponentsNull() {
- $this->startCapturingOutput();
- try {
- list($tableName) = CRM_Export_BAO_Export::exportComponents(
- TRUE,
- [],
- [],
- NULL,
- NULL,
- NULL,
- CRM_Export_Form_Select::CONTACT_EXPORT,
- NULL,
- NULL,
- FALSE,
- FALSE,
- [
- 'exportOption' => 1,
- ]
- );
- }
- catch (CRM_Core_Exception_PrematureExitException $e) {
- }
- $this->processor = $e->errorData['processor'];
- ob_end_clean();
+ $this->doExportTest([]);
}
/**
@@ -471,7 +449,6 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
FALSE,
FALSE,
[
- 'exportOption' => CRM_Export_Form_Select::CONTACT_EXPORT,
'suppress_csv_for_testing' => TRUE,
]
);
@@ -523,7 +500,6 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
FALSE,
FALSE,
[
- 'exportOption' => CRM_Export_Form_Select::CONTACT_EXPORT,
'suppress_csv_for_testing' => TRUE,
]
);
@@ -558,7 +534,6 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
FALSE,
FALSE,
[
- 'exportOption' => CRM_Export_Form_Select::CONTACT_EXPORT,
'suppress_csv_for_testing' => TRUE,
]
);
@@ -619,7 +594,6 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
FALSE,
TRUE,
[
- 'exportOption' => CRM_Export_Form_Select::CONTACT_EXPORT,
'suppress_csv_for_testing' => TRUE,
]
);
@@ -666,7 +640,6 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
FALSE,
TRUE,
[
- 'exportOption' => CRM_Export_Form_Select::CONTACT_EXPORT,
'suppress_csv_for_testing' => TRUE,
]
);
@@ -1133,7 +1106,6 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
FALSE,
FALSE,
[
- 'exportOption' => CRM_Export_Form_Select::CONTACT_EXPORT,
'suppress_csv_for_testing' => TRUE,
]
);
@@ -1208,7 +1180,6 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
TRUE,
FALSE,
[
- 'exportOption' => CRM_Export_Form_Select::CONTACT_EXPORT,
'mergeOption' => TRUE,
'suppress_csv_for_testing' => TRUE,
'postal_mailing_export' => [
@@ -1304,7 +1275,6 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
FALSE,
FALSE,
[
- 'exportOption' => CRM_Export_Form_Select::CONTACT_EXPORT,
'suppress_csv_for_testing' => TRUE,
]
);
@@ -1781,7 +1751,6 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
FALSE,
FALSE,
[
- 'exportOption' => CRM_Export_Form_Select::CONTRIBUTE_EXPORT,
'suppress_csv_for_testing' => TRUE,
]
);
@@ -2807,4 +2776,43 @@ class CRM_Export_BAO_ExportTest extends CiviUnitTestCase {
]);
}
+ /**
+ * Test export components.
+ *
+ * Tests the exportComponents function with the provided parameters.
+ *
+ * This exportComponents will export a csv but it will also throw a prematureExitException
+ * which we catch & grab the processor from.
+ *
+ * $this->processor is set to the export processor.
+ *
+ * @param $params
+ *
+ * @throws \CRM_Core_Exception
+ */
+ protected function doExportTest($params) {
+ $this->startCapturingOutput();
+ try {
+ CRM_Export_BAO_Export::exportComponents(
+ CRM_Utils_Array::value('selectAll', $params, (empty($params['fields']))),
+ CRM_Utils_Array::value('ids', $params, []),
+ CRM_Utils_Array::value('params', $params, []),
+ CRM_Utils_Array::value('order', $params),
+ CRM_Utils_Array::value('fields', $params),
+ CRM_Utils_Array::value('moreReturnProperties', $params),
+ CRM_Utils_Array::value('exportMode', $params, CRM_Export_Form_Select::CONTACT_EXPORT),
+ CRM_Utils_Array::value('componentClause', $params, FALSE),
+ CRM_Utils_Array::value('componentTable', $params),
+ CRM_Utils_Array::value('mergeSameAddress', $params, FALSE),
+ CRM_Utils_Array::value('mergeSameHousehold', $params, FALSE)
+ );
+ }
+ catch (CRM_Core_Exception_PrematureExitException $e) {
+ $this->processor = $e->errorData['processor'];
+ ob_end_clean();
+ return;
+ }
+ $this->fail('We expected a premature exit exception');
+ }
+
}