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] Return the sendEmail function to it's owner #21608

Merged
merged 1 commit into from
Sep 25, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Sep 24, 2021

Overview

[REF] Return the sendEmail function to it's owner

Before

Function to send emails from the Email task is on the activity BAO - giving the illusion of being centralised, re-usable code

After

The function on the BAO is deprecated. The real function now lives on the email trait, permitting us to clean up the inputs & outputs in follow ups

Technical Details

This sendEmail function is only called from one place in core and it is not 'generally useful' having
an awful parameter set. This PR moves it back to the class that 'owns' it - which will
allow us to undo all the work of building up that parameter set
and make it possible to support tokens for other entities than those already mangled in.

I would normally add a noisy deprecation notice once a function becomes unused in
core but since that has been done to the pdf task this release I've left this
deprecation a bit quieter for now.

Note that I cleaned up the tokens handled here before deprecating so we
could get rid of those calls fully

Under the OO structure it becomes easier to add the missing token options

  • membership & participant - but the business of 'one email per person, &
    just grab the tokens from the last entity' is messing with
    my head a bit. That's the next bit....

Comments

@demeritcowboy @seamuslee001 @colemanw this one really is a straight copy so should be easy to confirm

This sendEmail function is only called from one place in core and it is not 'generally useful' having
an awful parameter set. This PR moves it back to the class that 'owns' it - which will
allow us to undo all the work of building up that parameter set
and make it possible to support tokens for other entities than those already mangled in.

I would normally add a noisy deprecation notice once a function becomes unused in
core but since that has been done to the pdf task this release I've left this
deprecation a bit quieter for now.

Note that I cleaned up the tokens handled here before deprecating so we
could get rid of those calls fully

Under the OO structure it becomes easier to add the missing token options
- membership & participant - but the business of 'one email per person, &
just grab the tokens from the last entity' is messing with
my head a bit. That's the next bit....
@civibot
Copy link

civibot bot commented Sep 24, 2021

(Standard links)

@civibot civibot bot added the master label Sep 24, 2021
@demeritcowboy demeritcowboy merged commit 0ebeb22 into civicrm:master Sep 25, 2021
@demeritcowboy
Copy link
Contributor

I've logged an issue against https://lab.civicrm.org/extensions/civicrmmailer-d8/-/issues/3.

@eileenmcnaughton eileenmcnaughton deleted the email branch September 25, 2021 19:03
@eileenmcnaughton
Copy link
Contributor Author

thanks @demeritcowboy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants