Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move relationship return properties to the processor class to fix leakage related test fail #12521

Merged
merged 1 commit into from
Jul 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 1 addition & 81 deletions CRM/Export/BAO/Export.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ class CRM_Export_BAO_Export {
// CRM-7675
const EXPORT_ROW_COUNT = 100000;

protected static $relationshipReturnProperties = [];

/**
* Key representing the head of household in the relationship array.
*
Expand Down Expand Up @@ -70,66 +68,6 @@ class CRM_Export_BAO_Export {
*/
protected static $relationshipTypes = [];

/**
* @param $value
* @param $locationTypeFields
* @param $relationshipTypes
*
* @return array
*/
protected static function setRelationshipReturnProperties($value, $locationTypeFields, $relationshipTypes) {
$locationTypes = CRM_Core_PseudoConstant::get('CRM_Core_DAO_Address', 'location_type_id');
$relPhoneTypeId = $relIMProviderId = NULL;
if (!empty($value[2])) {
$relationField = CRM_Utils_Array::value(2, $value);
if (trim(CRM_Utils_Array::value(3, $value))) {
$relLocTypeId = CRM_Utils_Array::value(3, $value);
}
else {
$relLocTypeId = 'Primary';
}

if ($relationField == 'phone') {
$relPhoneTypeId = CRM_Utils_Array::value(4, $value);
}
elseif ($relationField == 'im') {
$relIMProviderId = CRM_Utils_Array::value(4, $value);
}
}
elseif (!empty($value[4])) {
$relationField = CRM_Utils_Array::value(4, $value);
$relLocTypeId = CRM_Utils_Array::value(5, $value);
if ($relationField == 'phone') {
$relPhoneTypeId = CRM_Utils_Array::value(6, $value);
}
elseif ($relationField == 'im') {
$relIMProviderId = CRM_Utils_Array::value(6, $value);
}
}
if (in_array($relationField, $locationTypeFields) && is_numeric($relLocTypeId)) {
if ($relPhoneTypeId) {
self::$relationshipReturnProperties[$relationshipTypes]['location'][$locationTypes[$relLocTypeId]]['phone-' . $relPhoneTypeId] = 1;
}
elseif ($relIMProviderId) {
self::$relationshipReturnProperties[$relationshipTypes]['location'][$locationTypes[$relLocTypeId]]['im-' . $relIMProviderId] = 1;
}
else {
self::$relationshipReturnProperties[$relationshipTypes]['location'][$locationTypes[$relLocTypeId]][$relationField] = 1;
}
}
else {
self::$relationshipReturnProperties[$relationshipTypes][$relationField] = 1;
}
return array($relationField);
}

/**
* @return array
*/
public static function getRelationshipReturnProperties() {
return self::relationshipReturnProperties;
}

/**
* Get default return property for export based on mode
*
Expand Down Expand Up @@ -326,22 +264,6 @@ public static function exportComponents(
if ($fields) {
//construct return properties
$locationTypes = CRM_Core_PseudoConstant::get('CRM_Core_DAO_Address', 'location_type_id');
$locationTypeFields = array(
'street_address',
'supplemental_address_1',
'supplemental_address_2',
'supplemental_address_3',
'city',
'postal_code',
'postal_code_suffix',
'geo_code_1',
'geo_code_2',
'state_province',
'country',
'phone',
'email',
'im',
);

foreach ($fields as $key => $value) {
$fieldName = CRM_Utils_Array::value(1, $value);
Expand All @@ -350,9 +272,7 @@ public static function exportComponents(
}

if ($processor->isRelationshipTypeKey($fieldName) && (!empty($value[2]) || !empty($value[4]))) {
self::setRelationshipReturnProperties($value, $locationTypeFields, $fieldName);
// @todo we can later not add this to this array but maintain a separate array.
$returnProperties = array_merge($returnProperties, self::$relationshipReturnProperties);
$returnProperties[$fieldName] = $processor->setRelationshipReturnProperties($value, $fieldName);
}
elseif (is_numeric(CRM_Utils_Array::value(2, $value))) {
$locTypeId = $value[2];
Expand Down
94 changes: 93 additions & 1 deletion CRM/Export/BAO/ExportProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ class CRM_Export_BAO_ExportProcessor {
*/
protected $relationshipTypes = [];

/**
* Array of properties to retrieve for relationships.
*
* @var array
*/
protected $relationshipReturnProperties = [];

/**
* CRM_Export_BAO_ExportProcessor constructor.
*
Expand Down Expand Up @@ -334,7 +341,11 @@ public function getPaymentTableID() {
public function getDefaultReturnProperties() {
$returnProperties = [];
$fields = CRM_Contact_BAO_Contact::exportableFields('All', TRUE, TRUE);
$skippedFields = ($this->getQueryMode() === CRM_Contact_BAO_Query::MODE_CONTACTS) ? [] : ['groups', 'tags', 'notes'];
$skippedFields = ($this->getQueryMode() === CRM_Contact_BAO_Query::MODE_CONTACTS) ? [] : [
'groups',
'tags',
'notes'
];

foreach ($fields as $key => $var) {
if ($key && (substr($key, 0, 6) != 'custom') && !in_array($key, $skippedFields)) {
Expand All @@ -345,4 +356,85 @@ public function getDefaultReturnProperties() {
return $returnProperties;
}

/**
* Add the field to relationship return properties & return it.
*
* This function is doing both setting & getting which is yuck but it is an interim
* refactor.
*
* @param array $value
* @param string $relationshipKey
*
* @return array
*/
public function setRelationshipReturnProperties($value, $relationshipKey) {
$locationTypes = CRM_Core_PseudoConstant::get('CRM_Core_DAO_Address', 'location_type_id');
$relPhoneTypeId = $relIMProviderId = NULL;
if (!empty($value[2])) {
$relationField = CRM_Utils_Array::value(2, $value);
if (trim(CRM_Utils_Array::value(3, $value))) {
$relLocTypeId = CRM_Utils_Array::value(3, $value);
}
else {
$relLocTypeId = 'Primary';
}

if ($relationField == 'phone') {
$relPhoneTypeId = CRM_Utils_Array::value(4, $value);
}
elseif ($relationField == 'im') {
$relIMProviderId = CRM_Utils_Array::value(4, $value);
}
}
elseif (!empty($value[4])) {
$relationField = CRM_Utils_Array::value(4, $value);
$relLocTypeId = CRM_Utils_Array::value(5, $value);
if ($relationField == 'phone') {
$relPhoneTypeId = CRM_Utils_Array::value(6, $value);
}
elseif ($relationField == 'im') {
$relIMProviderId = CRM_Utils_Array::value(6, $value);
}
}
if (in_array($relationField, $this->getValidLocationFields()) && is_numeric($relLocTypeId)) {
if ($relPhoneTypeId) {
$this->relationshipReturnProperties[$relationshipKey]['location'][$locationTypes[$relLocTypeId]]['phone-' . $relPhoneTypeId] = 1;
}
elseif ($relIMProviderId) {
$this->relationshipReturnProperties[$relationshipKey]['location'][$locationTypes[$relLocTypeId]]['im-' . $relIMProviderId] = 1;
}
else {
$this->relationshipReturnProperties[$relationshipKey]['location'][$locationTypes[$relLocTypeId]][$relationField] = 1;
}
}
else {
$this->relationshipReturnProperties[$relationshipKey][$relationField] = 1;
}
return $this->relationshipReturnProperties[$relationshipKey];
}

/**
* Get the default location fields to request.
*
* @return array
*/
public function getValidLocationFields() {
return [
'street_address',
'supplemental_address_1',
'supplemental_address_2',
'supplemental_address_3',
'city',
'postal_code',
'postal_code_suffix',
'geo_code_1',
'geo_code_2',
'state_province',
'country',
'phone',
'email',
'im',
];
}

}
15 changes: 8 additions & 7 deletions tests/phpunit/CRM/Export/BAO/ExportTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public function testExportComponentsContribution() {
array('Contribution', 'trxn_id'),
);

list($tableName, $sqlColumns) = CRM_Export_BAO_Export::exportComponents(
list($tableName) = CRM_Export_BAO_Export::exportComponents(
TRUE,
$this->contributionIDs,
array(),
Expand Down Expand Up @@ -595,7 +595,7 @@ public function testExportIMData() {
}
}
}
list($tableName) = $this->doExport($fields, $this->contactIDs[0]);
list($tableName, $sqlColumns) = $this->doExport($fields, $this->contactIDs[0]);

$dao = CRM_Core_DAO::executeQuery('SELECT * FROM ' . $tableName);
while ($dao->fetch()) {
Expand All @@ -611,9 +611,6 @@ public function testExportIMData() {
}
}

// early return for now until we solve a leakage issue.
return;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing this enables a lot of coverage


$this->assertEquals([
'billing_im_provider' => 'billing_im_provider text',
'billing_im_screen_name' => 'billing_im_screen_name text',
Expand Down Expand Up @@ -775,7 +772,7 @@ public function testExportDeceasedDoNotMail() {
));

//export and merge contacts with same address
list($tableName, $sqlColumns) = CRM_Export_BAO_Export::exportComponents(
list($tableName) = CRM_Export_BAO_Export::exportComponents(
TRUE,
array($contactA['id'], $contactB['id']),
array(),
Expand Down Expand Up @@ -841,7 +838,11 @@ protected function setUpHousehold() {
}

/**
* Do a CiviCRM export.
*
* @param $selectedFields
* @param int $id
*
* @return array
*/
protected function doExport($selectedFields, $id) {
Expand Down Expand Up @@ -995,7 +996,7 @@ public function getExtraReturnProperties() {
/**
* Get basic return properties.
*
* @bool $isContactMode
* @param bool $isContactMode
* Are we in contact mode or not
*
* @return array
Expand Down