Conversation
jrief
left a comment
There was a problem hiding this comment.
Before fixing my code annotations, I'd rather rethink, if it doesn't make more sense to se a WYSIWYG-Editor straight ahead. This would be my preferred long term solution.
Thanks for your work, but this new feature must be approved by @selwin anyway. As I said, this shall only be a temporary solution.
|
The pull request definitely needs cleanup, but since I didn't get a reply in the original issue anymore I thought it would be good to create a pull request so it doesn't get lost :) Personally I think that this addition is useful even if you would have a WYSIWYG editor. The renderings of those editors are almost always different from a regular render. It's not my call however, if the WYSIWYG editor is the preferred solution than I'll close the pull request and wait for that release. With that in mind... a form to send out a test-mail would be very useful as well. |
|
Having rendered HTML preview in admin would be great. Aside from @jrief 's comments, please make sure that all JS are stripped to prevent XSS attacks. |
The easiest and safest way would be to sandbox the iframe, is that enough or should I also do a few search/replaces? |
|
@wolph we have https://bleach.readthedocs.io/ as optional dependency. We can use that to easily strip all JS. |
|
I think we can even make |
155c969 to
db00311
Compare
|
I've cleaned up the commits and added the I should note that there is another similar attack vector in the code-base outside of this addition: django-post_office/post_office/admin.py Lines 179 to 182 in 5297f00 And possibly this one: django-post_office/post_office/admin.py Lines 121 to 135 in 5297f00 |
5929eaa to
0f98a1a
Compare
| default='', blank=True) | ||
| default_template = models.ForeignKey('self', related_name='translated_templates', | ||
| null=True, default=None, verbose_name=_('Default template'), on_delete=models.CASCADE) | ||
| example_context = context_field_class(_('Context'), blank=True, null=True) |
There was a problem hiding this comment.
Why are we adding a new field to the model? Adding a preview to admin shouldn't necessitate adding a new field.
There was a problem hiding this comment.
I think the variable name is self explanatory, but it's to add a context to the rendered preview so all variables can be rendered with a useful value.
| } | ||
| change_form_template = 'admin/post_office/EmailTemplate/change_form.html' | ||
|
|
||
| def change_view(self, request, object_id, form_url='', extra_context=None): |
There was a problem hiding this comment.
Can you move this dedicated preview functionality to a new view?
|
@wolph gentle reminder to update this PR so we can merge this in. |
# Conflicts: # post_office/admin.py
As discussed in #325, this adds an easy to use preview in the admin:
