Export code cleanup - Use getComponentPaymentFields from processorClass #13082
Conversation
|
(Standard links)
|
eileenmcnaughton
force-pushed
the
export
branch
from
d142831 to
8eca64a
Compare
| // get payment related in for event and members | ||
| $paymentDetails = CRM_Contribute_BAO_Contribution::getContributionDetails($exportMode, $ids); | ||
| //get all payment headers. | ||
| // If we haven't selected specific payment fields, load in all the | ||
| // payment headers. | ||
| if (!$processor->isExportSpecifiedPaymentFields()) { | ||
| $paymentHeaders = self::componentPaymentFields(); |
There was a problem hiding this comment.
Replaced by $processor->getPaymentHeaders(
| else { | ||
| $paymentHeaders = array(); | ||
| } | ||
| $nullContributionDetails = array_fill_keys(array_keys($paymentHeaders), NULL); |
There was a problem hiding this comment.
calculated when it is used
| @@ -468,6 +459,16 @@ public static function exportComponents( | |||
|
|
|||
| list($outputColumns, $headerRows, $sqlColumns, $metadata) = self::getExportStructureArrays($returnProperties, $processor); | |||
|
|
|||
| // add payment headers if required | |||
| if ($addPaymentHeader && $processor->isExportPaymentFields()) { | |||
There was a problem hiding this comment.
moved to being before the row-by-row loop
eileenmcnaughton
changed the title
|
@eileenmcnaughton The code looks reasonable to me; I don't have any improvements to suggest. I also don't expect that manual testing would turn up anything useful, but can do it if you think it's helpful. |
|
@twomice so in general we do try to test each change - I'm OK with merging this & then doing the next one if you commit to do a thorough round of testing towards the end of the month (ie. test all at once rather than individually) |
|
@eileenmcnaughton If you're saying manual testing would be appropriate here, I can do that. I suppose it would be good to try a few different exports with primary fields, custom fields, memberships, and contributions? |
|
@twomice In this case the focus would be on exporting entities with contributions - ie. some entities support (or at least according to the code :-) exporting related contribution data |
|
Great, will do. Thanks! |
eileenmcnaughton
force-pushed
the
export
branch
from
8eca64a to
62e4758
Compare
The saving of not creating an empty file occassionally doesn't warrant the code complexity
eileenmcnaughton
force-pushed
the
export
branch
from
62e4758 to
0b94b40
Compare
|
@eileenmcnaughton I've done some basic testing with this PR:
I think this is ok to merge |
|
thanks @mattwire - merging |
Overview
Minor code refactor as part of ongoing cleanup on export processor
Before
Less functions on new processor class
After
More functions on new processor class
Technical Details
We have been cleaning up this class preliminary to address bugs, including moving code to use the new ExportProcessor class
Comments
@twomice I've added another small refactor - this affects the compilation of payment fields being exported (eg from a participant export or member export) - it seems they should be included in 'primary fields' for the entity or be selectable for export - but the way this functionality was added is pretty bad.
Next step is to move getTransformedFieldValue off to the ExportProcessor class & extracting a buildRow function. The goal would be to merge the household in that function (maybe via array merge) and completely eliminate the mergeSameHousehold function