Explore using Jinja for templating #376

H-Shay · 2021-08-09T18:24:26Z

This PR implements Jinja as a templating engine for emails in Sydent.

Signed-off-by: H.Shay <shaysquared@gmail.com>

reivilibre

Definitely on the right track. Don't think we will need too much back and forth on this.

I had in mind that we were going to keep support for old-school templates; if someone else has said we're not, then let me know :).

res/matrix-org/invite_template.eml

sydent/util/emailutils.py

changelog.d/376.misc

setup.py

reivilibre

very close to ready to roll, I think :)

matrix_is_test/res/is-test/verification_template.eml

res/matrix-org/invite_template.eml

res/matrix-org/invite_template.eml.j2

sydent/util/emailutils.py

res/matrix-org/invite_template.eml

changelog.d/376.feature

sydent/terms/terms.py

sydent/util/emailutils.py

H-Shay · 2021-08-12T17:06:46Z

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!

sydent/util/emailutils.py

tests/test_templates.py

reivilibre

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.

res/matrix-org/invite_template.eml.j2

res/matrix-org/verification_template.eml.j2

res/vector-im/invite_template.eml.j2

sydent/sydent.py

sydent/util/emailutils.py

reivilibre

this is coming along :)

res/matrix-org/verification_template.eml.j2

res/vector-im/verification_template.eml.j2

res/vector_verification_sample.txt

tests/test_jinja_templates.py

reivilibre

looking great, let's get another pair of eyes on this

tests/test_jinja_templates.py

clokep

Looks good overall, but some of the code is a bit confusing still!

clokep · 2021-08-23T14:37:28Z

res/matrix-org/invite_template.eml.j2

+
+<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>


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.

This is indeed from the older version, I will fix in separate PR.

res/matrix-org/verification_template.eml.j2

res/vector-im/invite_template.eml

clokep · 2021-08-23T14:40:11Z

res/vector-im/invite_template.eml.j2

+
+<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>


Same thing about web_client_location.

sydent/util/emailutils.py

tests/test_jinja_templates.py

sydent/util/emailutils.py

clokep

I think this looks good with the one proposed change! It would be nice to have some upgrade notes for this too, although I'm not sure if we have a normal place for that in sydent or if we just put them in the release notes.

clokep · 2021-08-24T15:59:25Z

sydent/util/emailutils.py

+        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


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!)

Suggested change

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

reivilibre

veerrry close now :)

sydent/sydent.py

sydent/util/emailutils.py

reivilibre

seems good to me

H-Shay added 5 commits August 9, 2021 11:10

add jinja to setup

ed107ee

update send_email function to render templates with jinja

338bd5d

change templates to use jinja

589c396

lints

8e40335

Add changelog

7c81e22

Signed-off-by: H.Shay <shaysquared@gmail.com>

clokep mentioned this pull request Aug 10, 2021

Initial room_type work, MSC3288 implementation #375

Merged

2 tasks

reivilibre self-requested a review August 10, 2021 17:33

reivilibre requested changes Aug 10, 2021

View reviewed changes

H-Shay added 5 commits August 11, 2021 08:30

merge with main to update branch

c34bd99

add .j2 templates

a2106c1

check if jinja templates available and if so use them

35eea47

rename changelog file, version jinja in setup.py

3f22e70

lints

a97b0ae

clokep requested a review from reivilibre August 11, 2021 16:58

reivilibre requested changes Aug 11, 2021

View reviewed changes

clokep reviewed Aug 11, 2021

View reviewed changes

sydent/terms/terms.py Outdated Show resolved Hide resolved

sydent/util/emailutils.py Outdated Show resolved Hide resolved

H-Shay added 2 commits August 11, 2021 10:37

requested changes

525be72

revoke change to terms.py

308f9dc

H-Shay requested a review from reivilibre August 11, 2021 17:47

H-Shay added 6 commits August 11, 2021 15:27

add some tests

a721485

use jinja environment and autoescape

a542201

format jinja templates for autoescape and test

fb7db3c

fix tests, fix jinja templates

35c2640

lints + update tests

a3adf42

reset terms

dd46fae

H-Shay requested a review from clokep August 12, 2021 17:06

reivilibre requested changes Aug 13, 2021

View reviewed changes

sydent/util/emailutils.py Outdated Show resolved Hide resolved

tests/test_templates.py Outdated Show resolved Hide resolved

reivilibre reviewed Aug 13, 2021

View reviewed changes

tests/test_templates.py Outdated Show resolved Hide resolved

update templates with safe filter

96580b9

reset terms

f0f25cd

H-Shay requested a review from reivilibre August 16, 2021 18:34

reivilibre requested changes Aug 17, 2021

View reviewed changes

H-Shay added 7 commits August 18, 2021 10:04

update templates

e75c4d7

only manually escape old templates

ab9ccec

update tests

5c8dc2e

add smaple text for testing

e43f0f9

update tests name

3e3b116

lints + rename jinja tests

6c60693

update sydent.py

9880e0c

H-Shay requested a review from reivilibre August 18, 2021 17:47

reivilibre requested changes Aug 19, 2021

View reviewed changes

requested changes + lints

5379f85

H-Shay requested a review from reivilibre August 19, 2021 18:08

reivilibre requested changes Aug 23, 2021

View reviewed changes

tests/test_jinja_templates.py Show resolved Hide resolved

clokep suggested changes Aug 23, 2021

View reviewed changes

H-Shay added 4 commits August 23, 2021 09:36

update templates

611ee4a

requested changes + add licence

0baa33c

lints

4010fd5

slight refactor

a9dcbcf

H-Shay requested a review from clokep August 23, 2021 16:50

clokep approved these changes Aug 24, 2021

View reviewed changes

H-Shay added 2 commits August 24, 2021 09:33

requested change + lints

d4d0486

Merge branch 'main' into jinja

2da3529

reivilibre requested changes Aug 24, 2021

View reviewed changes

sydent/sydent.py Show resolved Hide resolved

sydent/util/emailutils.py Outdated Show resolved Hide resolved

fix error in redering emails

b0599a7

reivilibre approved these changes Aug 24, 2021

View reviewed changes

reivilibre merged commit 17ba907 into matrix-org:main Aug 24, 2021

clokep added a commit to t3chguy/sydent that referenced this pull request Aug 30, 2021

Update for changes from matrix-org#376.

90774cb

clokep mentioned this pull request Sep 2, 2021

Use the provided web client in the HTML email. #380

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Explore using Jinja for templating #376

Explore using Jinja for templating #376

H-Shay commented Aug 9, 2021

reivilibre left a comment

reivilibre left a comment

H-Shay commented Aug 12, 2021

reivilibre left a comment

reivilibre left a comment

reivilibre left a comment

clokep left a comment

clokep Aug 23, 2021

H-Shay Aug 23, 2021

clokep Aug 23, 2021

clokep left a comment

clokep Aug 24, 2021

reivilibre left a comment

reivilibre left a comment

Explore using Jinja for templating #376

Explore using Jinja for templating #376

Conversation

H-Shay commented Aug 9, 2021

reivilibre left a comment

Choose a reason for hiding this comment

reivilibre left a comment

Choose a reason for hiding this comment

H-Shay commented Aug 12, 2021

reivilibre left a comment

Choose a reason for hiding this comment

reivilibre left a comment

Choose a reason for hiding this comment

reivilibre left a comment

Choose a reason for hiding this comment

clokep left a comment

Choose a reason for hiding this comment

clokep Aug 23, 2021

Choose a reason for hiding this comment

H-Shay Aug 23, 2021

Choose a reason for hiding this comment

clokep Aug 23, 2021

Choose a reason for hiding this comment

clokep left a comment

Choose a reason for hiding this comment

clokep Aug 24, 2021

Choose a reason for hiding this comment

reivilibre left a comment

Choose a reason for hiding this comment

reivilibre left a comment

Choose a reason for hiding this comment