Use global date format for CFP end date in submission email #1185
Conversation
There was a problem hiding this comment.
I agree with the intention of this PR, but I believe the format of dates is a presentation concern, so rather than injecting date formats into controllers or services, I would prefer it if the formatting generally occurred in the presentation layer.
What do you think, @chartjes?
|
The reason I injected it at the controller level is I wasn't sure that Twig was able to access the date format string in that particular context. I'll check again shortly. |
|
Looks like that date is globally injected, so I'm going to completely re-roll this. |
|
False alarm; the "site" variables don't appear to exist in the Twig instance that's being used by the emails for some reason, so it's falling back to the Twig-declared date format. Tracking this down further now. |
|
Okay; the issue here is that we're using an internal Twig method to render each block, rather than rendering the entire template at once, for the email. There are a few easier solutions than what I'm about to attempt:
What I'm going to try is to have a patched Twig_Environment that allows for rendering a block with the global context added in. Wish me luck. |
All of these calls are in email templates, including the talk submission ones, but now anything that's available when rendering an entire template is now available when rendering a block, as long as you call renderBlockWithContext().
iansltx
force-pushed
the
formatted-end-date-in-email
branch
from
6bd7c09
to
58b621d
Compare
|
Quite a bit more work involved in the above, but in return emails can now include global Twig vars in their blocks without explicitly passing things in, and those vars will actually be there (since emails are rendered one block at a time). I've confirmed that this works exactly as expected on our instance (the correct date format is respected). |
|
Don't worry too much, we can still follow up on this! 👍 |
|
@localheinz This now passes CI due to my fix of a flakey test elsewhere. What else do I need to do to get this merged? |
Everything else uses the global date format, so it looks like this is a bit of an omission as it stands.