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

Use api to create activity and removed hardcoded status id #14621

Merged
merged 1 commit into from
Sep 4, 2019

Conversation

pradpnayak
Copy link
Contributor

Overview

Use api to create activity and removed hardcoded status id

Before

Status Id was hardcoded to 2

After

Replaced hardcoded value with name and used api to create activity

@civibot
Copy link

civibot bot commented Jun 23, 2019

(Standard links)

@civibot civibot bot added the master label Jun 23, 2019
@eileenmcnaughton
Copy link
Contributor

This seems straight forward, sensible, and inline with our general approach.

Do we have any tests that cover this createActivityExport function or more generally the CRM/Financial/BAO/ExportFormat class @pradpnayak

@pradpnayak
Copy link
Contributor Author

I am afraid to say there isn't any unit test, I guess we had written webtest when it was implemented in 2014.

@eileenmcnaughton
Copy link
Contributor

@pradpnayak any chance you could add 'something' with this change - I'd settle for 'anything that takes us forwards on test cover in this area - even if it just shows the function being called without a fatal

@eileenmcnaughton
Copy link
Contributor

test this please

@pradpnayak
Copy link
Contributor Author

@eileenmcnaughton yes i will, can you please help me if we have test related to export? I can come up with a unit test if i get a starting point.

@eileenmcnaughton
Copy link
Contributor

@pradpnayak I would add something to CRM_Batch_BAO_BatchTest that calls

CRM_Batch_BAO_Batch::exportFinancialBatch($batchIds, $this->_exportFormat, $this->_downloadFile);

I feel like if you can just get to the point of calling the function with no errors that is enough to have extended our cover. - the CRM_Utils_System::civiExit(); function now throws an exception although per #14608 I'm still struggling to quite figure out how to do the testing using it (will have another dig on that one now)

eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 25, 2019
This adds the minimal test suggested as needed for civicrm#14621

In the process I needed to add some api defaults
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 25, 2019
This adds the minimal test suggested as needed for civicrm#14621

In the process I needed to add some api defaults
@eileenmcnaughton
Copy link
Contributor

test this please

@eileenmcnaughton eileenmcnaughton merged commit 8437566 into civicrm:master Sep 4, 2019
magnolia61 pushed a commit to magnolia61/civicrm-core that referenced this pull request Oct 13, 2019
This adds the minimal test suggested as needed for civicrm#14621

In the process I needed to add some api defaults
@pradpnayak pradpnayak deleted the CleanUps branch March 23, 2020 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants