[ready for core team to review] CRM-20614 Don't download export file until asked #10393
Conversation
CRM/Batch/BAO/Batch.php
Outdated
| @@ -594,6 +596,7 @@ public static function exportFinancialBatch($batchIds, $exportFormat) { | |||
| else { | |||
| CRM_Core_Error::fatal("Could not locate exporter: $exporterClass"); | |||
| } | |||
| $exporter->_isDownloadFile = $downloadFile; | |||
There was a problem hiding this comment.
I don't see the property $_isDownloadFile declared in that class.
There was a problem hiding this comment.
@colemanw I have pushed the changes. Can you please review?
There was a problem hiding this comment.
Have you tried it? I think this will give a fatal error trying to set a protected property.
There was a problem hiding this comment.
oops sorry, Extension had a over-ridden copy of this file therefore i didn't get the error while testing. I have updated the changes now.
|
@civicrm-builder test this please |
|
@colemanw I have tested the patch and it works fine now. |
---------------------------------------- * CRM-20614: Donot Download Export file for Financial Batch https://issues.civicrm.org/jira/browse/CRM-20614
---------------------------------------- * CRM-20614: Donot Download Export file for Financial Batch https://issues.civicrm.org/jira/browse/CRM-20614 --fixed fatal error
0fefb76 to
555f69c
Compare
|
@colemanw i have rebased the PR. Can you please review? |
| */ | ||
| public static function exportFinancialBatch($batchIds, $exportFormat) { | ||
| public static function exportFinancialBatch($batchIds, $exportFormat, $downloadFile) { |
There was a problem hiding this comment.
Add a default value to downloadFile to keep compatibility with older versions?
There was a problem hiding this comment.
@mlutfy exportFinancialBatch(...) is used only in one place as I grep`ed in codebase
Monishs-MacBook-Pro:civicrm monish$ grep -irn "exportFinancialBatch" CRM api/
CRM/Batch/BAO/Batch.php:45: * Not sure this is the best way to do this. Depends on how exportFinancialBatch() below gets called.
CRM/Batch/BAO/Batch.php:600: public static function exportFinancialBatch($batchIds, $exportFormat, $downloadFile) {
CRM/Financial/Form/Export.php:178: CRM_Batch_BAO_Batch::exportFinancialBatch($batchIds, $this->_exportFormat, $this->_downloadFile);
So I think its not required to provide a default value. What do you say?
[ready for core team to review] CRM-20614 Don't download export file until asked
https://issues.civicrm.org/jira/browse/CRM-20614