[REF] Copy preProcessFromAddress back into the pdf function

[eileenmcnaughton]

Overview

REF Copy preProcessFromAddress back into the pdf function

Before

We are calling a function on another class that is slated for deprecation. It's really hard to unravel that function with it being called from 3 places & parts of it relating to different places

After

It is copied back for cleanup

Technical Details

This is really hard to unravel without splitting it between classes -
mostly because it does stuff to do with setting properties on the form
that are not even relevant to all forms. This is a divide & conquer
case to cleaning up the function from
the deprecated class

Note that I removed the bounce on no-email. It felt like we would still want
to be able to download pdfs if the logged in user had no email. The bounce param seems
to have been added as an after thought - presumably to block
the bounce in a flow where they realised it made no sense

Comments

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[demeritcowboy]

This seems ok.

It might be slightly cleaner to put the code inside a separate function rather than dumping into preProcess - or are you going to move some of this again in a minute?

The bounce param seems to have been added as an after thought - presumably to block the bounce in a flow where they realised it made no sense

It would probably be for thank-you letters where the From field is required, but yes it probably shouldn't be required if you just want to download, and it would be a very unusual install anyway where the logged-in user and the system have no From emails configured. I'd be curious how many users have ever seen the bounce message.

[eileenmcnaughton]

@demeritcowboy I didn't put it in it's own function because I don't think it's the right grouping on functionality - but I have been thinking about this line

https://github.com/civicrm/civicrm-core/pull/21306/files#diff-42159e86480cd91ed6b03047d5b1e17abe32fd21ca2233c4788b0374924894dfR54

And I'm increasingly sure that it only exists to support the invoice code (& is used by a test out of convenience) - I can't think of any logical reason to set it to logged in user on the pdf class - although I had thought removing it was something I'd do once this is merged

[demeritcowboy]

Ok thanks.