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

WIP Activity pdf 71 rebased #16200

Closed
wants to merge 2 commits into from

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Jan 3, 2020

Overview

Rebased #12012 following merge of #14662

Before

What is the old user-interface or technical-contract (as appropriate)?
For optimal clarity, include a concrete example such as a screenshot, GIF (LICEcap, SilentCast), or code-snippet.

After

What changed? What is new old user-interface or technical-contract?
For optimal clarity, include a concrete example such as a screenshot, GIF (LICEcap, SilentCast), or code-snippet.

Technical Details

If the PR involves technical details/changes/considerations which would not be manifest to a casual developer skimming the above sections, please describe the details here.

Comments

Anything else you would like the reviewer to note

@civibot
Copy link

civibot bot commented Jan 3, 2020

(Standard links)

@civibot civibot bot added the master label Jan 3, 2020
@mattwire mattwire changed the title Activity pdf 71 rebased WIP Activity pdf 71 rebased Jan 3, 2020
Create CRM_Actvity_Form_Task_PDFLetterCommon extending CRM_Core_Form_Task_PDFLetterCommon

CRM_Activity_Tokens: add source, target and assignee tokens

Eg: {activity.target_N_display_name}
- N can be substituted for a number to show the details of the nth activity target contact
eg: {activity.target_1_display_name} is the display name of the first target
For ease of use, contacts are numbered from 1.
If the literal N is used or is replaced by 0, it is treated as 1.

Slim down alterActionScheduleQuery() and move most data fetching to prefetch()

Add tests for Activity PDF Letter
@mattwire mattwire force-pushed the activity_pdf_71_rebased branch from 00191bc to ed53716 Compare January 3, 2020 17:16
@seamuslee001
Copy link
Contributor

Jenkins re test this please

@mattwire
Copy link
Contributor Author

mattwire commented Jan 3, 2020

See https://lab.civicrm.org/mattwire/emailapi/tree/newtokenprocessor for a useful way to try it out

Also, I renamed Source / Target per @demeritcowboy comments

@eileenmcnaughton
Copy link
Contributor

@mattwire as you picked up this part got stalled over the difficulty of adding a many type token & was considered to be a change that should be documented/discussed/ considered/ resolved in it's own right. I can't recall the details but I guess the place to flesh out how this works is the PR template & then we can try to figure it out.

@aydun are there any remaining parts that should be picked off & merged separately or is this all about the many token issue now?

@seamuslee001
Copy link
Contributor

test fails relate here @mattwire

@aydun
Copy link
Contributor

aydun commented Feb 21, 2020

@eileenmcnaughton @mattwire I think all that's left here now are the 'special' tokens. But I also have several other fixes/enhancements for this locally from the October sprint. I'll combine those with Matt's changes.

@mattwire
Copy link
Contributor Author

I'm going to close this as @aydun is planning to pick up the remaining bits (and I'm not likely to) but we don't want them getting lost. Tracking via https://lab.civicrm.org/dev/core/issues/1614 @eileenmcnaughton

@mattwire mattwire closed this Feb 21, 2020
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.

4 participants