twomice / com.joineryhq.reltoken

CiviCRM extension: Provides tokens for any related contact, for all available contact tokens. E.g., to get the spouse's first name, or parent's marriage date.
Other
1 stars 5 forks source link

In New Email, related token in Subject inhibits related tokens in body #15

Closed rgilman closed 3 years ago

rgilman commented 3 years ago

I run an educational program where some people are mentors to others, which is represented in civi by a relationship. I'm trying to create a direct mail template, using Related Tokens, for an introduction email that gets sent to each student with information about the mentor.

Ideally, the subject looks something like Meet your new mentor {related.first_name___reltype_b_Ally_for_Allied_by}

In the body there are further tokens for the mentor.

What I'm finding is that:

1) If there is no token in the subject, the tokens in the body work as expected. 2) If there is a contact token (useless for the project but interesting for debugging), also the tokens in the body work as expected. 3) If there is a Related Token in the subject, then in the body the contact tokens work but the related tokens return nothing.

I'm happy to clarify or do further tests if that would help.

twomice commented 3 years ago

Thanks for the report @rgilman I'm not sure I completely understand the bug. Could I ask you to describe specific steps to reproduce this on a default civicrm installation (i.e., using native relationship types such as Employer) so that we can reproduce the bug in our development environment with minimal configuration changes?

rgilman commented 3 years ago

Sure.

1) Create a relation between two contacts (with email addresses that come to you) using a native relationship type such as Employee / Employer. 2) Go to Contact > New Email 3) Enter one of the contacts (let's call it A) as the recipient. 4) In the Subject, include a related token associated with the other contact (B) 5) In the HTML body, include at least one related token associated with B 6) Send the email so you can see the results.

Using this pattern, what I found was that the B tokens in the subject returned the appropriate content but in the body they returned nothing, no content.

Does that help?

twomice commented 3 years ago

Yes, thanks @rgilman We'll look into it.

Attention: @lucky091588 please look into this, ask me in Trello or email if you have questions.

lucky091588 commented 3 years ago

@rgilman Thanks for the report. Base on my findings, this issue will also appear on other token extension. I conclude that this is more of a CiviCRM core issue. I already reported this bug and here is the link https://lab.civicrm.org/dev/core/-/issues/2673.

rgilman commented 3 years ago

Thanks @lucky091588 for investigating this so well. Since it looks like you've found a potential fix, will you be submitting a PR to CiviCRM core?

lucky091588 commented 3 years ago

@rgilman We might be. Anyway, this is already reported on the core. Thanks again.

twomice commented 3 years ago

BTW @rgilman If you like, you can try the fix mentioned in the core bug report (https://lab.civicrm.org/dev/core/-/issues/2673) and comment there as to whether it works for you. That might get you a fix that you can use, and will also further the work of fixing it in core.

rgilman commented 3 years ago

@twomice, will do. I can also post about my experience on the mattermost dev channel. Given that there are 363 sig:bug issues, I'm happy to give this one a boost where I can.

rgilman commented 3 years ago

I've waited until 5.39 was available before testing the patch. I've done so now and it all looks good. I've also commented on the core bug report.

What's next? I'm not properly set up to do a proper PR for this. If would be wonderful if you could.

rgilman commented 3 years ago

I've also posted about this issue and its fix in the mattermost dev channel.

twomice commented 3 years ago

Thanks @rgilman We aim to work on a PR for that bug next week.

rgilman commented 3 years ago

Any news on the PR for this issue?

twomice commented 3 years ago

@rgilman Thanks for the nudge, see PR referenced in core ticket https://lab.civicrm.org/dev/core/-/issues/2673

twomice commented 3 years ago

FYI, PR https://lab.civicrm.org/dev/core/-/issues/2673 has been marked "merge ready". I recommend following that PR for updates, as I don't intend to post further updates in this ticket. Thanks again to @rgilman and @lucky091588

rgilman commented 3 years ago

Excellent news! And my thanks again to you both, @twomice and @lucky091588.