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] dev/core#2790 Pre process cleanup on pdf tasks #21310

Merged
merged 1 commit into from
Aug 31, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

[REF] Pre process cleanup on pdf tasks

Before

Three variables are set on the class that were originally set because of shared code but don't seem to make sense

  1. $form->_single - this was presumably an enotice fix - if any enotices re-emerge we can fix the right way - by setting a property on the form
  2. $form->_contactIds - I believe this was added to support the invoice form - it doesn't make sense to default to writing a letter to your self
  3. $form->_emails - this is used in the emails classes but not the pdf classes on searching

After

Poof

Technical Details

Comments

@demeritcowboy this is the clean up I thought needed to be done.... With this done I'll move the preProcess to the trait & move the default setting into it's own function or, more likely, into the setDefaults function already on the trait

@civibot
Copy link

civibot bot commented Aug 29, 2021

(Standard links)

@civibot civibot bot added the master label Aug 29, 2021
@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy also in the same function I'm mystified by

$messageText = [];
$messageSubject = [];
$dao = new CRM_Core_BAO_MessageTemplate();
$dao->is_active = 1;
$dao->find();
while ($dao->fetch()) {
$messageText[$dao->id] = $dao->msg_text;
$messageSubject[$dao->id] = $dao->msg_subject;
}
$form->assign('message', $messageText);
$form->assign('messageSubject', $messageSubject);

  • load 'any' message template & assign to the tpl? - it doesn't seem to be used in PDFLetterCommon.tpl or anywhere else I can find....

@eileenmcnaughton eileenmcnaughton changed the title [REF] Pre process cleanup on pdf tasks [REF] dev/core#2790 Pre process cleanup on pdf tasks Aug 31, 2021
@demeritcowboy
Copy link
Contributor

I'm reviewing this at the same time as #21276.

And yes I can't see what purpose those messageText/messageSubject tpl vars serve. The same pattern exists in the msgtpltools extension, but I expect was copy/paste since it doesn't seem used there either.

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy ok - so lets remove it - alter this PR - or put up a follow up once this is merged?

@demeritcowboy
Copy link
Contributor

Here would be ok. Either way is fine by me.

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy I've updated to rip out those lines - it's starting to look like less code....

@demeritcowboy demeritcowboy merged commit 7420e2e into civicrm:master Aug 31, 2021
@eileenmcnaughton eileenmcnaughton deleted the prepro branch August 31, 2021 19:01
@eileenmcnaughton
Copy link
Contributor Author

yay - thanks

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