gadgetto / GoodNews

An integrated group and newsletter mailing system for MODX Revolution CMF
GNU General Public License v2.0
10 stars 4 forks source link

Placeholder values cannot be processed by snippets/output filters (in mailing templates) #59

Closed mindeffects closed 7 years ago

mindeffects commented 7 years ago

Inside a mailing template, this "made up" test delivers a strange result: [[+gender:cat=` like wine.`:replace=`Women==2`]]

For a female recipient one would expect this result: Women like wine.

But that's not what happens! This is the result: 2 like wine. The output filter does not have access to the the placeholder!?

When using a custom output filter – or a snippet with [[+gender]] as a property – and looking at the var with var_dump or print_r you get this result: string[11] = 2

It took some time for me to figure out what this means: [[+gender]]has 11 chars! (Thanks, @mark-h). At the time, when the placeholder should get proccessed by the snippet or output filter, its value is not rendered yet! Which makes it's impossible to do any stuff with its value.

But I need to process the gender in order to generate a propper salutation. And I cannot do that right now. Or can I?

gadgetto commented 7 years ago

Just did a quick test with the following tags inside a mailing template:

[[+gender]] = 
[[+gender:is=`1`:then=`male`:else=``]]
[[+gender:is=`2`:then=`female`:else=``]]

It correctly renders:

1 = male or 2 = female

(I didn't test this with a snippet or custom placeholder)

mindeffects commented 7 years ago

OMG, I think I know what happend: When TESTING a mailing, everything works fine, but NOT when VIEWING, because VIEW does not come with any user data! And I used VIEW because debugging is way simpler and faster than sniffing through the mail body. So, me dumb. Sorry.

Maybe GoodNews could use the current users data for VIEW? This way the admin could easily change his own GoN-settings and look how that affects the output. This is a FR, right? ;-)

Thanks for your help, @gadgetto!

gadgetto commented 7 years ago

You are right, this is definitely needed! Already filed as feature request: https://github.com/gadgetto/GoodNews/issues/44