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

dev/core#4287 Fix event_offline template phones & emails #26297

Merged
merged 1 commit into from
May 26, 2023

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented May 22, 2023

Overview

Fix event_offline template phones & emails

https://lab.civicrm.org/dev/core/-/issues/4287

Before

We have reports of the old smarty variables causing havoc

After

Using (tested) tokens - we can now preview them in Message Admin & see how they look...

image

image

Technical Details

Comments

@civibot
Copy link

civibot bot commented May 22, 2023

(Standard links)

@civibot civibot bot added the master label May 22, 2023
@eileenmcnaughton eileenmcnaughton changed the title Fix event_offline template phones & emails dev/core#4287 Fix event_offline template phones & emails May 22, 2023
@demeritcowboy
Copy link
Contributor

This doesn't seem to be working for me. I get almost the same E_NOTICEs before and after, and now the phones don't get output. I can't check the test site here because the ability to redirect-to-database was blocked a few months ago. Will try to check another install.

(Some of the notices aren't related to location info so those would be out of scope, but the main thing I'd say would be the phones are missing.)

@demeritcowboy
Copy link
Contributor

If I log the compiled smarty before it calls eval() I can see that
{if {event.loc_block_id.phone_id.phone|boolean}}
is coming out as
<?php if (0): ?>

and {event.loc_block_id.phone_id.phone} is blank.

@demeritcowboy
Copy link
Contributor

Ah I see whatever cache these tokens are using doesn't get cleared when you edit an event location (or at least when you add a phone). That's not the PR's fault but that seems like a little bit of a bug.

So I'll merge, and then this will need regen too. There are still warnings in the output, but they're from other things in the template.

@demeritcowboy demeritcowboy merged commit 58d85f3 into civicrm:master May 26, 2023
@eileenmcnaughton eileenmcnaughton deleted the event_phone branch May 27, 2023 00:23
@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy thanks for testing & merging & yeah the caching is a bit of a gotcha!

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