PR#226: Add WYSIWYG editor and solve urllib3/requests dependency error - fedora-commops/fedora-happiness-packets

fedora-commops / fedora-happiness-packets

#226 Add WYSIWYG editor and solve urllib3/requests dependency error

Merged 4 years ago by jflory7. Opened 4 years ago by shraddhaag.

fedora-commops/ shraddhaag/fedora-happiness-packets WYSIWYG into master

Remove opbeat as a requirement

Shraddha Agrawal • 4 years ago

83a0ebb

Add WYSIWYG editor and solve urllib3/requests dependency error

ShraddhaAg • 4 years ago

a1ed1b2

happinesspackets/messaging/forms.py

file modified

+32 -1

		`@@ -12,6 +12,7 @@`
		`from django.db.models import Q`
		`from django.utils import timezone`
		`from email_normalize import normalize`
		`+ import bleach`

		`from .models import Message, strip_email`

		`@@ -75,7 +76,37 @@`
		`return True`
		`else:`
		`return False`
		`-`
		`+`
		`+ def clean_message(self):`
		`+ """ Cleans given HTML with bleach.clean() """`
		`+`
		`+ allowed_tags = set(bleach.ALLOWED_TAGS + [`
		`+ 'a', 'blockquote', 'code', 'del', 'dd', 'dl', 'dt',`
		`+ 'h1', 'h2', 'h3', 'h3', 'h4', 'h5', 'i', 'img', 'kbd',`
		`+ 'li', 'ol', 'ul', 'p', 'pre', 's', 'sup', 'sub', 'em',`
		`+ 'strong', 'strike', 'ul', 'br', 'hr', ])`
		`+`
		`+ allowed_styles = set(bleach.ALLOWED_STYLES + [`
		`+ 'color', 'background-color', 'font', 'font-weight',`
		`+ 'height', 'max-height', 'min-height',`
		`+ 'width', 'max-width', 'min-width', ])`
		`+`
		`+ allowed_attributes = {}`
		`+ allowed_attributes.update(bleach.ALLOWED_ATTRIBUTES)`
		`+ allowed_attributes.update({`
		`+ '*': ['class', 'title'],`
		`+ 'a': ['href', 'rel'],`
		`+ 'img': ['alt', 'src', 'width', 'height', 'align', 'style', 'max-width'],`
		`+ })`
		`+ html = self.cleaned_data['message']`
		`+ return bleach.clean(`
		`+ html,`
		`+ strip=True,`
		`+ tags=allowed_tags,`
		`+ attributes=allowed_attributes,`
		`+ styles=allowed_styles`
		`+ )`
		`+`
		`def clean(self):`
		`super(MessageSendForm, self).clean()`
		`isREEqualsSE = self.is_recipient_email_equals_sender_email()`

happinesspackets/messaging/migrations/0007_message_field.py

file added

+19

		`@@ -0,0 +1,19 @@`
		`+ # Generated by Django 2.0 on 2019-05-08 11:53`
		`+`
		`+ import ckeditor.fields`
		`+ from django.db import migrations`
		`+`
		`+`
		`+ class Migration(migrations.Migration):`
		`+`
		`+ dependencies = [`
		`+ ('messaging', '0006_message_recipient_username'),`
		`+ ]`
		`+`
		`+ operations = [`
		`+ migrations.AlterField(`
		`+ model_name='message',`
		`+ name='message',`
		`+ field=ckeditor.fields.RichTextField(),`
		`+ ),`
		`+ ]`
		`\ No newline at end of file`

happinesspackets/messaging/models.py

file modified

+2 -1

		`@@ -10,6 +10,7 @@`
		`from django.utils.crypto import salted_hmac`
		`from model_utils import Choices`
		`from model_utils.models import TimeStampedModel`
		`+ from ckeditor.fields import RichTextField`

		`from happinesspackets.utils.misc import readable_random_token`
		`from happinesspackets.tasks import send_html_mail`
		`@@ -40,7 +41,7 @@`
		`recipient_email_stripped = models.CharField(max_length=255)`
		`recipient_email_token = models.CharField(max_length=255, db_index=True)`

		`- message = models.TextField()`
		`+ message = RichTextField()`

		`sender_named = models.BooleanField(default=False)`
		`sender_approved_public = models.BooleanField(default=False)`

happinesspackets/settings/base.py

file modified

+13 -1

		`@@ -11,6 +11,18 @@`

		`PROJECT_DIR = Path(__file__).ancestor(3)`

		`+ # CKEditor configurations`
		`+ CKEDITOR_ALLOW_NONIMAGE_FILES = False`
		`+`
		`+ CKEDITOR_CONFIGS = {`
		`+ 'default': {`
		`+ 'removePlugins':'smiley',`
		`+ 'extraPlugins': 'stylesheetparser',`
		`+ 'width': 'auto',`
		`+ 'contentsCss': 'html, iframe, body, img {max-width:100%;}',`
		`+ },`
		`+ }`
		`+`
		`# For clean_pyc to work without complaining`
		`BASE_DIR = PROJECT_DIR`

		`@@ -64,7 +76,6 @@`
		`MIDDLEWARE = [`
		`'django.contrib.sessions.middleware.SessionMiddleware',`
		`'happinesspackets.utils.middleware.SetRemoteAddrFromForwardedFor',`
		`- 'opbeat.contrib.django.middleware.OpbeatAPMMiddleware',`
		`'dogslow.WatchdogMiddleware',`
		`'django.middleware.common.CommonMiddleware',`
		`'django.middleware.csrf.CsrfViewMiddleware',`
		`@@ -118,6 +129,7 @@`
		`'haystack',`
		`'happinesspackets.messaging',`
		`'djcelery_email',`
		`+ 'ckeditor',`
		`]`

happinesspackets/settings/deployment.py

file modified

-2

		`@@ -20,8 +20,6 @@`
		`}`
		`}`

		`- INSTALLED_APPS.append('opbeat.contrib.django')`
		`-`
		`TEMPLATES[0]['OPTIONS']['loaders'] = (`
		`('django.template.loaders.cached.Loader', (`
		`'django.template.loaders.filesystem.Loader',`

happinesspackets/settings/dev.py

file modified

		`@@ -88,3 +88,4 @@`
		`secrets = json.load(f)`
		`ADMIN_USERNAME = secrets["ADMIN_USERNAME"]`
		`ADMIN_PASSWORD = secrets["ADMIN_PASSWORD"]`
		`+`

requirements/base.txt

file modified

+7 -2

		`@@ -28,15 +28,16 @@`
		`pep8==1.7.0`

		`# Misc`
		`+ urllib3==1.24.2 # Dependency for requests`
		`python-dateutil==2.5.0`
		`factory-boy==2.9.2`
		`- opbeat==3.6.1`
		`- mozilla-django-oidc==1.2.1`
		`+ mozilla-django-oidc==1.2.2`
		`fedora-messaging>=1.4.0`
		`happinesspacket-schema>=0.1.2`
		`celery[redis]==4.2.1`
		`django-celery-email==2.0.1`
		`psycopg2==2.7.5 # PostgreSQL driver`
		`+ bleach>=3.1.0`

		`# Search engine`
		`Whoosh==2.7.4`
		`@@ -50,3 +51,7 @@`

		`# Dependency for YAML file`
		`pyyaml==5.1`
		`+`
		`+ # WYSIWYG addition`
		`+ django-ckeditor==5.7.0`
		`+`

file modified

-1

		`@@ -1,3 +1,2 @@`
		`export DJANGO_SETTINGS_MODULE=happinesspackets.settings.tsting &&`
		`- OPBEAT_DISABLE_SEND=true coverage run ./manage.py test $@ &&`
		`coverage report --fail-under=100`

templates/messaging/_message_list.html

file modified

+1 -1

		`@@ -7,7 +7,7 @@`
		`{% endif %}`
		`</h4>`
		`<blockquote>`
		`- {{ message.message\|linebreaksbr }}`
		`+ {{ message.message\|safe }}`
		`</blockquote>`
		`{% endfor %} {% block pagination %} {% if is_paginated %}`
		`{% include 'messaging/_pagination.html' %}`

templates/messaging/message_recipient_form.html

file modified

+1 -1

		`@@ -16,7 +16,7 @@`
		`Your Happiness Packet contains:`
		`</p>`

		`- <blockquote>{{ message.message\|linebreaksbr }}</blockquote>`
		`+ <blockquote>{{ message.message\|safe }}</blockquote>`

		`{% crispy form %}`
		`{% endblock content %}`

templates/messaging/message_send_form.html

file modified

+10

		`@@ -3,6 +3,9 @@`
		`{% block extra_head %}<title>Send a Happiness Packet</title>{% endblock %}`

		`{% load crispy_forms_tags %}`
		`+ {% load static %}`
		`+ <script type="text/javascript" src="{% static "ckeditor/ckeditor-init.js" %}"></script>`
		`+ <script type="text/javascript" src="{% static "ckeditor/ckeditor/ckeditor.js" %}"></script>`

		`{% block content %}`

		`@@ -13,4 +16,11 @@`
		`{% crispy form %}`

		`<button id="search_fasid">Search</button>`
		`+`
		`+ <script>`
		`+ $("#submit-id-submit").click(function() {`
		`+ $(".cke_wysiwyg_frame").contents().find("img")`
		`+ .css({"max-width": "100%", "height": "auto"});`
		`+ });`
		`+ </script>`
		`{% endblock content %}`
		`\ No newline at end of file`

templates/messaging/received_messages.html

file modified

+1 -1

		`@@ -20,7 +20,7 @@`
		`{% endif %}`
		`</h4>`
		`<blockquote>`
		`- {{ message.message\|linebreaksbr }}`
		`+ {{ message.message\|safe }}`
jflory7 commented 4 years ago Just curious, what does this change?
		`</blockquote>`
		`{% endfor %}`

templates/messaging/recipient_mail.html

file modified

+1 -1

		`@@ -19,7 +19,7 @@`

		`<p>Your Happiness Packet contains:</p>`

		`- <p><em>{{ message.message\|linebreaksbr }}</em></p>`
		`+ <p><em>{{ message.message\|safe }}</em></p>`

		`<p>`
		`If you and the sender of the Happiness Packet both agree, we'd love to publish the message to our`

templates/messaging/sender_confirmation_mail.html

file modified

+1 -1

		`@@ -12,7 +12,7 @@`
		`Your message reads:`
		`</p>`

		`- <p><em>{{ message.message\|linebreaksbr }}</em></p>`
		`+ <p><em>{{ message.message\|safe }}</em></p>`

		`<p>`
		`To confirm and send your message, click or copy this link to a web browser:`

templates/messaging/sender_confirmation_mail.txt

file modified

+1 -1

		`@@ -5,7 +5,7 @@`
		`Your message reads:`

		`---------------------`
		`- {{ message.message }}`
		`+ {{ message.message\|safe }}`
		`---------------------`

		`To confirm and send your message, click or copy this link to a web browser:`

templates/messaging/sent_messages.html

file modified

+1 -1

		`@@ -17,7 +17,7 @@`
		`Sent to {{ message.recipient_name }}`
		`</h4>`
		`<blockquote>`
		`- {{ message.message\|linebreaksbr }}`
		`+ {{ message.message\|safe }}`
		`</blockquote>`
		`{% endfor %}`

shraddhaag commented 4 years ago

This commit includes the following:

Adds a WYSIWYG text editor using django-ckeditor.
Resolve urllib3 dependency error requests 2.21.0 has requirement urllib3<1.25,>=1.21.1, but you'll have urllib3 1.25.2 which is incompatible.

The usage of all the formatting options is tested on email. The output looks like this on the website and same on email as well.

Metadata Update from @jflory7:
- Pull-request tagged with: PASSED, needs testing, new change, type - frontend, type - summer coding

4 years ago

Metadata Update from @jflory7:
- Request assigned

4 years ago

jflory7 commented on line 5 of requirements/base.txt 4 years ago

A few weeks ago, CVE-2019-11324 was opened on urllib versions 1.24.1 and earlier. Please bump this dependency to 1.24.2 to patch the CVE.

jflory7 commented on line 6 of templates/messaging/received_messages.html 4 years ago

Just curious, what does this change?

jflory7 commented on line 24 of happinesspackets/settings/base.py 4 years ago

I don't know if this is standard or not, but I prefer to put variables outside of a method after import statements at the top of the module. Since these are used for configuration, it would help to group all related configuration values at the top of the file for easier reading. You could group it with the PROJECT_DIR variable on line 12.

Metadata Update from @jflory7:
- Pull-request untagged with: needs testing
- Pull-request tagged with: needs changes

4 years ago

jflory7 commented 4 years ago

Hi @shraddhaag, nice work! :thumbsup: It's super exciting to see the WYSIWYG editor!

I left some feedback in-line, but I have a few other comments:

Happiness Archive does not display rich text formatting
Editor window should fit user's screen size (not sure about mobile devices?)
Embedded images should always adjust for the user's screen size in email and Happiness Archive / confirmation screen

Screenshot examples of each are below:

Comment 1: Happiness Archive does not render rich-text formatting:

Comment 2: Editor window goes off-screen when sending a new Happiness Packet:

Do you think you can figure these out? If we can make these improvements, the PR is ready to go. :grinning:

rebased onto 114b245d56f1f57c9379f859a48f1f76b7ab86c9

4 years ago

shraddhaag commented 4 years ago

@jflory7 I integrated all the changes. Please let me know if I need to change anything else :)

shraddhaag commented 4 years ago

Just curious, what does this change?

By default HTML is not escaped and so is displayed as text. We need to mark the field as safe so that Django renders it as HTML. It's detailed here in the documentation.