Explore using Jinja for templating #376
Conversation
Signed-off-by: H.Shay <shaysquared@gmail.com>
|
Okay so we've got some tests, jinja is now using an environment and autoescaping is enabled-turns out we were already manually cleaning the substitutions for html/url, so I left that in the code. Jinja doesn't seem to be double escaping anything so I think we are okay? Happy to rework that if this solution isn't amenable. Let me know! |
There was a problem hiding this comment.
I think the tests are likely to need a little bit of change once you make some of these changes.
I wonder if it might be sensible to, in one of the tests:
- patch out the path to the templates (so you can load a test-only template)
- have a little test template, testing a variety of things
- check the entire output to see that it's what we wanted.
You might be able to inject templates straight into the Jinja environment, that might be a good way to go about it instead.
|
|
||
| <p> | ||
| <a | ||
| href="https://app.element.io/#/room/{{ room_id|urlencode }}?email={{ to|urlencode }}&signurl=https%3A%2F%2Fmatrix.org%2F_matrix%2Fidentity%2Fapi%2Fv1%2Fsign-ed25519%3Ftoken%3D{{ token|urlencode }}%26private_key%3D{{ ephemeral_private_key|urlencode }}&room_name={{ room_name|urlencode }}&room_avatar_url={{ room_avatar_url|urlencode }}&inviter_name={{ sender_display_name|urlencode }}&guest_access_token={{ guest_access_token|urlencode }}&guest_user_id={{ guest_user_id|urlencode }}">Join the conversation.</a> |
There was a problem hiding this comment.
Huh, should this be using {{ web_client_location }} here too? (Probably copied from the non-Jinja version though.) We might want to fix that separately.
There was a problem hiding this comment.
This is indeed from the older version, I will fix in separate PR.
|
|
||
| <p> | ||
| <a | ||
| href="https://app.element.io/#/room/{{ room_id|urlencode }}?email={{ to|urlencode }}&signurl=https%3A%2F%2Fvector.im%2F_matrix%2Fidentity%2Fapi%2Fv1%2Fsign-ed25519%3Ftoken%3D{{ token|urlencode }}%26private_key%3D{{ ephemeral_private_key|urlencode }}&room_name={{ room_name|urlencode }}&room_avatar_url={{ room_avatar_url|urlencode }}&inviter_name={{ sender_display_name|urlencode }}&guest_access_token={{ guest_access_token|urlencode }}&guest_user_id={{ guest_user_id|urlencode }}">Join the conversation.</a> |
There was a problem hiding this comment.
Same thing about web_client_location.
sydent/util/emailutils.py
Outdated
| with open(templateFile) as template_file: | ||
| for k, v in substitutions.items(): | ||
| allSubstitutions[k + "_forhtml"] = escape(v) | ||
| allSubstitutions[k + "_forurl"] = urllib.parse.quote(v) | ||
| allSubstitutions["multipart_boundary"] = generateAlphanumericTokenOfLength( | ||
| 32 | ||
| ) | ||
| mailString = template_file.read() % allSubstitutions |
There was a problem hiding this comment.
Since allSubstitutions is now only used in this branch, we can move it into here. It is also usually best to keep the file open for as short as possible! (I think I gave you the previous code, my bad!)
| with open(templateFile) as template_file: | |
| for k, v in substitutions.items(): | |
| allSubstitutions[k + "_forhtml"] = escape(v) | |
| allSubstitutions[k + "_forurl"] = urllib.parse.quote(v) | |
| allSubstitutions["multipart_boundary"] = generateAlphanumericTokenOfLength( | |
| 32 | |
| ) | |
| mailString = template_file.read() % allSubstitutions | |
| allSubstitutions = {} | |
| for k, v in substitutions.items(): | |
| allSubstitutions[k + "_forhtml"] = escape(v) | |
| allSubstitutions[k + "_forurl"] = urllib.parse.quote(v) | |
| allSubstitutions["multipart_boundary"] = generateAlphanumericTokenOfLength( | |
| 32 | |
| ) | |
| with open(templateFile) as template_file: | |
| mailString = template_file.read() % allSubstitutions |
This PR implements Jinja as a templating engine for emails in Sydent.