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

[Ref] Move noisily deprecate BaseIPN->sendMail, call api from it rather than BAO function #17982

Merged
merged 1 commit into from
Jul 31, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jul 28, 2020

Overview

This function has been deprecated since 2015 and it's tempting just to remove it but at this stage
I'm just upping the noise, bypassing it in tests, and making it call the api so we have more of a free hand with the BAO function - notably it makes it clearer we don't need to have params by reference in the BAO function.

Before

Deprecated (& probably unused - certainly unused by core) BaseIPN function calls a BAO function

After

Deprecated (& probably unused - certainly unused by core) BaseIPN function calls the recommended api function.
Unused parameters removed from the signature

Only 2 places still call this BAO function the api and one other.

Technical Details

Baby cleanup step - facilitates #17984

Comments

Fun fact - these tests being altered are probably from our first ever test-driven refactor. They were written before moving the guts of the function out of the BaseIPN class & adding an api

@civibot
Copy link

civibot bot commented Jul 28, 2020

(Standard links)

@civibot civibot bot added the master label Jul 28, 2020
@eileenmcnaughton eileenmcnaughton changed the title Call api from BaseIPN->sendMail rather than BAO function Move noisily deprecate BaseIPN->sendMail, call api from it rather than BAO function Jul 28, 2020
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 28, 2020
Once civicrm#17982 is merged there are only 2 places that call this function. In both
places
1) they instantiate an empty values array just to pass in
2) we can see they don't use input or ids afterwards, so they don't need to be passed by reference
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jul 28, 2020
Once civicrm#17982 is merged there are only 2 places that call this function. In both
places
1) they instantiate an empty values array just to pass in
2) we can see they don't use input or ids afterwards, so they don't need to be passed by reference
This function has been deprecated since 2015 and it's tempting just to remove it but at this stage
it is being noisily deprecated & by-passed
@eileenmcnaughton
Copy link
Contributor Author

CRM_Contribute_BAO_ContributionRecurTest.testAutoRenewalWhenOneMemberIsDeceased

I can't see how that relates but if it's still there after another run I'll dig

@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton eileenmcnaughton changed the title Move noisily deprecate BaseIPN->sendMail, call api from it rather than BAO function [Ref] Move noisily deprecate BaseIPN->sendMail, call api from it rather than BAO function Jul 29, 2020
@seamuslee001
Copy link
Contributor

Jenkins re test this please

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 looks like it has passed now

@seamuslee001
Copy link
Contributor

This looks fine to me merging

@seamuslee001 seamuslee001 merged commit 2aec04e into civicrm:master Jul 31, 2020
@seamuslee001 seamuslee001 deleted the isreceipt branch July 31, 2020 23:25
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