Add email placeholder for picture #6580

vtran99 · 2024-10-15T09:45:36Z

Request to add a placeholder to display the personal data picture in email.

ThiefMaster · 2024-10-15T09:49:50Z

indico/modules/events/registration/placeholders/registrations.py

+    def render(cls, regform, registration):
+        picture_data = registration.get_personal_data_picture()
+        if not picture_data:
+            return _('NO PICTURE')


That looks quite ugly, especially w/ the uppercase.

Two potential options:

Returning an empty string

Returning a placeholder image (like the one with the initial we use e.g. in meeting participant lists)

ThiefMaster · 2024-10-15T09:50:43Z

indico/modules/events/registration/placeholders/registrations.py

+        url = url_for('event_registration.registration_picture', picture_data.locator.file,
+                      _external=True, token=registration.uuid)


I thought we agreed to use the cid and attach the image to the email? External image references in emails are generally a bad idea.

vtran99 · 2024-10-16T11:13:41Z

I updated the code using cid, but for the preview it doesn't work as attachments can't be done.
Do you have a solution for it ?

ThiefMaster · 2024-10-16T11:17:09Z

indico/util/string.py

    css_sanitizer = IndicoCSSSanitizer(allowed_css_properties=BLEACH_ALLOWED_STYLES_HTML)
    return bleach.clean(string, tags=BLEACH_ALLOWED_TAGS_HTML, attributes=BLEACH_ALLOWED_ATTRIBUTES_HTML,
-                        css_sanitizer=css_sanitizer)
+                        protocols=BLEACH_ALLOWED_PROTOCOLS_HTML, css_sanitizer=css_sanitizer)


That should not be applied globally, but only when sending emails that require it (ie make it a default-false arg of this function).

I tried to define the 'sanitise_html' with default arg:
app.add_template_filter(lambda s: Markup(sanitize_html(s or '', default_protocols=False)), 'sanitize_html')
but when I use the filter
{{ email_body | sanitize_html(True) }}
got error:
"setup_jinja..() takes 1 positional argument but 2 were given"

Could you please help ?

app.add_template_filter(lambda s, **kwargs: Markup(sanitize_html(s or '', **kwargs)), 'sanitize_html')

And then just add , *, allow_cid=False to the argument list of sanitize_html and depending on that arg use the default protocols or default protocols + cid

same error: "setup_jinja..() takes 1 positional argument but 2 were given"
when calling the filter with arg:
app.add_template_filter(lambda s, **kwargs: Markup(sanitize_html(s or '', **kwargs)), 'sanitize_html')
def sanitize_html(string, *, allow_cid=False):
{{ email_body | sanitize_html(True) }}

You are passing it as a positional arg, but it should be a keyword arg...

ThiefMaster · 2024-10-16T11:17:16Z

indico/util/string.py

    'text-decoration', 'text-indent', 'text-shadow', 'text-transform', 'unicode-bidi', 'visibility', 'vertical-align',
    'width', 'widows', 'white-space', 'word-spacing', 'word-wrap', 'z-index'
 ]
+BLEACH_ALLOWED_PROTOCOLS_HTML = ['cid']  # used for embedded image


This will most likely break the current list of allowed protocols since you're only passing cid but no http/https now.

ThiefMaster · 2024-10-16T11:18:42Z

I updated the code using cid, but for the preview it doesn't work as attachments can't be done.
Do you have a solution for it ?

Damn, forgot about the preview... One solution could be to use a base64 data url for this placeholder while rendering the preview. Or use your original solution for the preview and the cid only when generating the actual mail.

vtran99 · 2024-10-16T16:30:53Z

To render differently the picture in preview, I need to know the context "preview".
I see 3 ways to do it:

check directly flask.request in PicturePlaceHolder.render();
add context preview when calling replace_placeholder of email_body, then add '**kwargs' to all placeholders 'render':

email_body = replace_placeholders('registration-email', request.form['body'], regform=self.regform,
                                  registration=registration, preview=True)

def render(cls, regform, registration, **kwargs):
       if kwargs['preview']...

get new placeholders for 'registration-email-preview' with a PicturePreviewPlaceHolder:

email_body = replace_placeholders('registration-email-preview', request.form['body'], regform=self.regform,
                                  registration=registration)
@signals.core.get_placeholders.connect_via('registration-email-preview')
def _get_registration_placeholders_preview(sender, regform, registration, **kwargs):
	yield PicturePreviewPlaceHolder
	yield ...

What would be your suggestion ?

ThiefMaster · 2024-10-16T16:33:55Z

i had option 2 in mind, but i somehow thought we already accept kwargs and thus would not need to modify every placeholder... maybe checking if request.endpoint == 'event_registration.email_registrants_preview' (so kind of option 1) is not too ugly

vtran99 · 2024-10-16T16:52:59Z

One last point:
in picture rendering, you use a fixed max-height/width = 250px, we would like to have a different value.
Can I use a parameter in config for that value ?
(same for REGISTRATION_PICTURE_THUMBNAIL_SIZE which is in a constants.py file)

ThiefMaster · 2024-10-16T16:55:09Z

constant in python sure, but I don't think having an indico.conf setting for this would really make sense - it's SO specific and probably you guys are the only ones who would change this...

indico/util/string.py

indico/modules/events/registration/templates/emails/custom_email.html

indico/modules/events/registration/placeholders/registrations.py

indico/modules/events/registration/models/registrations.py

…il.html Co-authored-by: Adrian <[email protected]>

Co-authored-by: Adrian <[email protected]>

ThiefMaster reviewed Oct 15, 2024

View reviewed changes

vtran99 requested a review from ThiefMaster October 16, 2024 11:13

ThiefMaster reviewed Oct 16, 2024

View reviewed changes

vtran99 requested a review from ThiefMaster October 16, 2024 17:23

ThiefMaster reviewed Oct 30, 2024

View reviewed changes

vtran99 and others added 7 commits October 30, 2024 14:53

Add email placeholder for picture

96834a6

Use cid for image

be51f45

allow cid in sanitize_html

1f888ef

render picture in preview

d31dc04

fix header

efeff1a

Small improvements

6b04ead

Add changelog entry

3ca0fb5

ThiefMaster force-pushed the Add_email_placeholder_for_picture branch from 506289a to 3ca0fb5 Compare October 30, 2024 14:26

vtran99 and others added 2 commits October 30, 2024 15:42

Update indico/modules/events/registration/templates/emails/custom_ema…

5e0c921

…il.html Co-authored-by: Adrian <[email protected]>

Update indico/util/string.py

fd87635

Co-authored-by: Adrian <[email protected]>

ThiefMaster added this to the v3.3 milestone Oct 30, 2024

ThiefMaster approved these changes Oct 30, 2024

View reviewed changes

ThiefMaster merged commit 11c5081 into indico:master Oct 30, 2024

ThiefMaster deleted the Add_email_placeholder_for_picture branch October 30, 2024 14:53

OmeGak pushed a commit to UNOG-Indico/indico-core that referenced this pull request Nov 5, 2024

Add email placeholder for registrant picture (indico#6580)

0afdcd9

		url = url_for('event_registration.registration_picture', picture_data.locator.file,
		_external=True, token=registration.uuid)

Add email placeholder for picture #6580

Add email placeholder for picture #6580

Uh oh!

Conversation

vtran99 commented Oct 15, 2024

Uh oh!

ThiefMaster Oct 15, 2024

Choose a reason for hiding this comment

Uh oh!

ThiefMaster Oct 15, 2024

Choose a reason for hiding this comment

Uh oh!

vtran99 commented Oct 16, 2024

Uh oh!

ThiefMaster Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

vtran99 Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

ThiefMaster Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

vtran99 Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ThiefMaster Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

ThiefMaster Oct 16, 2024

Choose a reason for hiding this comment

Uh oh!

ThiefMaster commented Oct 16, 2024

Uh oh!

vtran99 commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ThiefMaster commented Oct 16, 2024

Uh oh!

vtran99 commented Oct 16, 2024

Uh oh!

ThiefMaster commented Oct 16, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vtran99 Oct 16, 2024 •

edited

Loading

vtran99 commented Oct 16, 2024 •

edited

Loading