- Issue created by @adamps
- πΊπ¦Ukraine abyss
Updated the title and description as it was misplaced with β¨ Email body policy form should show available TWIG variables Needs work .
- Assigned to abyss
- Merge request !78Issue #3404625: Added display of TWIG variables in the BodyEmailAdjuster. β (Open) created by abyss
- πΊπ¦Ukraine abyss
@AdamPS, I added display of TWIG variables in the BodyEmailAdjuster.
But I would like to clarify, from which version it is better to note that getCommonAdjusters is deprecated, and in which version it should be removed? - π¬π§United Kingdom adamps
Great thanks I'll take a look. It would be deprecated in 1.4.1 and removed in 2.0.0.
- Status changed to Needs work
about 1 year ago 7:23pm 1 December 2023 - π¬π§United Kingdom adamps
Yes it looks very good. I didn't have a chance to test it yet, however I reviewed the code in the MR and made a few simple comments.
- Status changed to Needs review
about 1 year ago 4:00pm 6 December 2023 - πΊπ¦Ukraine abyss
@AdamPS, I made changes as per the comments in MR and added a deprecated message for MailerPolicyInterface::getCommonAdjusters.
If you have any other suggestions, I will be happy to add them. - Status changed to Needs work
about 1 year ago 2:06pm 14 December 2023 - π¬π§United Kingdom adamps
Great thanks, it's better. I tested it now and I like it. I made some more comments in the MR - they should all be quick and easy.
Two bigger comments:
1) Variables can also be used in SubjectEmailAdjuster and we should do the same. Please put the code in a function (don't just copy itπ) e.g. MailerHelperInterface::showReplacementHelp()
2) It would be great to have a test. At the end of TestEmailTest::testTest(), add a call
$this->submitForm([], 'Edit');
then can check that the page contains the replacement pattern for day. - π¬π§United Kingdom adamps
Actually the are issues open to use tokens in other places too: e.g. plain text body or addresses. So perhaps we should have the variable/token helpers on the form just once - in PolicyEditForm, after all the adjusters. Then for each adjuster that accepts tokens/variables we can put a message to say that.
- π¬π§United Kingdom adamps
OK if you can do it for the email body then that's already good. We can raise a follow on issue for the other places.