[REF] dev/core#2790 towards pdf task trait #21276
Conversation
|
(Standard links)
|
|
test this please |
|
Ah sorry now this has conflicts. |
|
thanks @demeritcowboy - I've updated it |
|
Is conflicted again - 21290 |
99e4e87 to
22cf409
Compare
|
@demeritcowboy rebased again! |
|
& conflicted again - but I just rebased |
| @@ -31,18 +33,24 @@ public function preProcess() { | |||
| */ | |||
| public function buildQuickForm() { | |||
| $this->addPDFElementsToForm(); | |||
| // Remove types other than pdf as they are not working (have never worked) and don't want fix | |||
| // for them to block pdf. | |||
There was a problem hiding this comment.
So far so good just this seems like a useful comment? Otherwise it's not clear why this override is here.
There was a problem hiding this comment.
@demeritcowboy good catch - I had wondered why that line was there, not realising I had removed the explanation - back now
Deprecate CRM_Activity_Form_Task_PDFLetterCommon This gets us to having 2 classes rather than 3 that manage the activityPDF task functionality. CRM_Activity_Form_Task_PDFLetterCommon doesn't really add anything from a structure POV but it does make it more confusing. There are also functions on the parent that are only used by this class - which makes switching to a trait harder. This untangles that part. Note that once we have the trait (& some more token cleanup done) we will be well placed to re-share some of these functions again
| */ | ||
| public function createTokenProcessor() { | ||
| return new TokenProcessor(\Civi::dispatcher(), [ | ||
| 'controller' => get_class(), |
There was a problem hiding this comment.
get_class() here is a different class than before the patch, but it doesn't seem to make a difference on the activity print/merge. When I look around the token classes for "controller", I only really see it in hook_tokenValues. I don't see any hook implementations in universe that depend specifically on CRM_Activity_Form_Task_PDFLetterCommon.
|
thanks @demeritcowboy |
Overview
Deprecate CRM_Activity_Form_Task_PDFLetterCommon
https://lab.civicrm.org/dev/core/-/issues/2790
Before
3 classes in play to render the activity pdf form - as a quasi OO
After
Down to 2
Technical Details
This gets us to having 2 classes rather than 3 that manage the activityPDF task
functionality.
CRM_Activity_Form_Task_PDFLetterCommon doesn't really add anything from a structure POV
but it does make it more confusing. There are also functions on the parent
that are only used by this class - which makes switching to a trait
harder. This untangles that part.
Note that once we have the trait (& some more token cleanup done) we will be well placed
to share some of these functions so that they are actually used by more than one class
Comments