[REF] Build single array of information about output specifications when exporting

[eileenmcnaughton]

Overview

Part of an ongoing cleanup on export class

Before

Several arrays are built with data about the same fields - they need to be kept in sync but are not tied together

After

The data is moved all into one array

Technical Details

This was pretty fiddly but the tests were ruthless in picking things up

Comments

@mattwire I did another cut - I think it would be good to review it but wait for the next rc to be cut before merging this one as it's not very readable as a change

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[mattwire]

@eileenmcnaughton Tested Membership, Contribution and Contact export (with do not merge, merge all contacts with the same address, postal only enabled/disabled).
Two issues:

1. Membership export seems to be missing data from the following fields:

Quantia total | Status de Contribuição | Date Received | Payment Method

2. Contact export for "Merge all contacts with the same address" has an extra "state_province_id" field at the end which is empty.

[eileenmcnaughton]

@mattwire thanks - I'll try to capture those in tests!

[mattwire]

@eileenmcnaughton

To fix 1:
CRM_Export_BAO_ExportProcessor.php@675:

         if ($this->isExportPaymentFields()) {
-          $paymentData = CRM_Utils_Array::value($row[$paymentTableId], $paymentDetails);
+          $paymentData = CRM_Utils_Array::value($iterationDAO->$paymentTableId, $paymentDetails);

To fix 2:
CRM_Export_BAO_ExportProcessor.php@565:

     foreach ($this->outputSpecification as $key => $spec) {
       if (empty($spec['do_not_output_to_csv'])) {
+        if ($key == 'state_province_id') {
+          continue;
+        }

The "fix" for 1 looks like a typo to me. The fix for 2 is slightly less ideal as it "special-cases" that field, but it's only added in (and it's required to be added) for merge same address because internally it's using the ID to compare addresses which cascades down into SQL statements etc. To remove it at source would require significant further refactoring so this would probably be a good fix (with a comment) to get this refactor merged.

[mattwire]

Furthermore, to fix household export I think we can now do:

CRM/Export/BAO/Export.php@@ -1010,7 +1010,7 @@ WHERE  id IN ( $deleteIDString )
     $mappingFields = array(
       'civicrm_primary_id' => 'id',
       'provider_id' => 'im_service_provider',
-      'phone_type_id' => 'phone_type',
+      //'phone_type_id' => 'phone_type',
     );

which gets rid of the unknown field error on export. I did get row size too large and changed the temp table definition to MyISAM so I could test the above. Clearly, further improvements required, but this may help get us closer...

I tested this one on top of #13216

[eileenmcnaughton]

@mattwire I captured both the issues you found in tests and fixed them - can you recheck - I can rebase the other one over this PR

[mattwire]

@eileenmcnaughton Ok tests pass and the issues I listed appear resolved when testing on a demo site. Ok to merge @seamuslee001

[seamuslee001]

Merging as per review my @mattwire