email_mirror: Add prefer-html and prefer-text address options.

Closes #13484.

These options tell zulip whether to prefer the plaintext or html version
of the email message. prefer-text is the default behavior, so including
the option doesn't change anything as of now, but we're adding it to
prepare to potentially change the default behavior in the future.
This commit is contained in:
Mateusz Mandera
2020-01-15 16:28:46 +01:00
committed by Tim Abbott
parent 170e0ac2dd
commit 0c9c218e91
4 changed files with 70 additions and 8 deletions

View File

@@ -59,3 +59,16 @@ below by sending email to
(e.g. a missed message email), a copy of the original message is (e.g. a missed message email), a copy of the original message is
automatically added to the bottom of your reply. By default, Zulip tries automatically added to the bottom of your reply. By default, Zulip tries
to remove that copied message. With this option, Zulip will include it. to remove that copied message. With this option, Zulip will include it.
* **.prefer-html**: The body of an email is typically encoded using
one or both of two common formats: plain text (`text/plain`) and
HTML (`text/html`). Zulip supports constructing the Zulip message
content using either (converting HTML to markdown for the HTML
format). By default, Zulip will prefer using the plain text version
of an email over the converted HTML version if both are present.
This option overrides that behavior to prefer the HTML version
instead.
* **.prefer-text**: Similar to `.prefer-html`, but explicitly asks
Zulip to prefer the plain text version of the email if both are
present (the current default behavior).

View File

@@ -134,8 +134,9 @@ def create_missed_message_address(user_profile: UserProfile, message: Message) -
return str(mm_address) return str(mm_address)
def construct_zulip_body(message: message.Message, realm: Realm, show_sender: bool=False, def construct_zulip_body(message: message.Message, realm: Realm, show_sender: bool=False,
include_quotes: bool=False, include_footer: bool=False) -> str: include_quotes: bool=False, include_footer: bool=False,
body = extract_body(message, include_quotes) prefer_text: bool=True) -> str:
body = extract_body(message, include_quotes, prefer_text)
# Remove null characters, since Zulip will reject # Remove null characters, since Zulip will reject
body = body.replace("\x00", "") body = body.replace("\x00", "")
if not include_footer: if not include_footer:
@@ -186,23 +187,21 @@ def get_message_part_by_type(message: message.Message, content_type: str) -> Opt
return None return None
talon_initialized = False talon_initialized = False
def extract_body(message: message.Message, include_quotes: bool=False) -> str: def extract_body(message: message.Message, include_quotes: bool=False, prefer_text: bool=True) -> str:
import talon import talon
global talon_initialized global talon_initialized
if not talon_initialized: if not talon_initialized:
talon.init() talon.init()
talon_initialized = True talon_initialized = True
# If the message contains a plaintext version of the body, use
# that.
plaintext_content = get_message_part_by_type(message, "text/plain") plaintext_content = get_message_part_by_type(message, "text/plain")
if plaintext_content: if prefer_text and plaintext_content:
if include_quotes: if include_quotes:
return plaintext_content return plaintext_content
else: else:
return talon.quotations.extract_from_plain(plaintext_content) return talon.quotations.extract_from_plain(plaintext_content)
# If we only have an HTML version, try to make that look nice. # If we have to use the HTML version, try to make that look nice.
html_content = get_message_part_by_type(message, "text/html") html_content = get_message_part_by_type(message, "text/html")
if html_content: if html_content:
if include_quotes: if include_quotes:

View File

@@ -17,6 +17,8 @@ optional_address_tokens = {
"show-sender": default_option_handler_factory("show-sender"), "show-sender": default_option_handler_factory("show-sender"),
"include-footer": default_option_handler_factory("include-footer"), "include-footer": default_option_handler_factory("include-footer"),
"include-quotes": default_option_handler_factory("include-quotes"), "include-quotes": default_option_handler_factory("include-quotes"),
"prefer-text": lambda options: options.update(prefer_text=True),
"prefer-html": lambda options: options.update(prefer_text=False),
} }
class ZulipEmailForwardError(Exception): class ZulipEmailForwardError(Exception):

View File

@@ -62,10 +62,12 @@ from typing import Any, Callable, Dict, Mapping, Optional
class TestEncodeDecode(ZulipTestCase): class TestEncodeDecode(ZulipTestCase):
def _assert_options(self, options: Dict[str, bool], show_sender: bool=False, def _assert_options(self, options: Dict[str, bool], show_sender: bool=False,
include_footer: bool=False, include_quotes: bool=False) -> None: include_footer: bool=False, include_quotes: bool=False,
prefer_text: bool=True) -> None:
self.assertEqual(show_sender, ('show_sender' in options) and options['show_sender']) self.assertEqual(show_sender, ('show_sender' in options) and options['show_sender'])
self.assertEqual(include_footer, ('include_footer' in options) and options['include_footer']) self.assertEqual(include_footer, ('include_footer' in options) and options['include_footer'])
self.assertEqual(include_quotes, ('include_quotes' in options) and options['include_quotes']) self.assertEqual(include_quotes, ('include_quotes' in options) and options['include_quotes'])
self.assertEqual(prefer_text, options.get('prefer_text', True))
def test_encode_decode(self) -> None: def test_encode_decode(self) -> None:
realm = get_realm('zulip') realm = get_realm('zulip')
@@ -161,6 +163,17 @@ class TestEncodeDecode(ZulipTestCase):
self._assert_options(options, show_sender=True) self._assert_options(options, show_sender=True)
self.assertEqual(token, stream.email_token) self.assertEqual(token, stream.email_token)
def test_decode_prefer_text_options(self) -> None:
stream = get_stream("Denmark", get_realm("zulip"))
address_prefer_text = "Denmark.{}.prefer-text@testserver".format(stream.email_token)
address_prefer_html = "Denmark.{}.prefer-html@testserver".format(stream.email_token)
token, options = decode_email_address(address_prefer_text)
self._assert_options(options, prefer_text=True)
token, options = decode_email_address(address_prefer_html)
self._assert_options(options, prefer_text=False)
class TestGetMissedMessageToken(ZulipTestCase): class TestGetMissedMessageToken(ZulipTestCase):
def test_get_missed_message_token(self) -> None: def test_get_missed_message_token(self) -> None:
with self.settings(EMAIL_GATEWAY_PATTERN="%s@example.com"): with self.settings(EMAIL_GATEWAY_PATTERN="%s@example.com"):
@@ -487,6 +500,41 @@ class TestEmailMirrorMessagesWithAttachments(ZulipTestCase):
mock_warn.assert_called_with("Payload is not bytes (invalid attachment %s in message from %s)." % mock_warn.assert_called_with("Payload is not bytes (invalid attachment %s in message from %s)." %
('some_attachment', self.example_email('hamlet'))) ('some_attachment', self.example_email('hamlet')))
def test_receive_plaintext_and_html_prefer_text_html_options(self) -> None:
user_profile = self.example_user('hamlet')
self.login(user_profile.email)
self.subscribe(user_profile, "Denmark")
stream = get_stream("Denmark", user_profile.realm)
stream_address = "Denmark.{}@testserver".format(stream.email_token)
stream_address_prefer_html = "Denmark.{}.prefer-html@testserver".format(stream.email_token)
text = "Test message"
html = "<html><body><b>Test html message</b></body></html>"
incoming_valid_message = MIMEMultipart()
text_message = MIMEText(text)
html_message = MIMEText(html, 'html')
incoming_valid_message.attach(text_message)
incoming_valid_message.attach(html_message)
incoming_valid_message['Subject'] = 'TestStreamEmailMessages Subject'
incoming_valid_message['From'] = self.example_email('hamlet')
incoming_valid_message['To'] = stream_address
incoming_valid_message['Reply-to'] = self.example_email('othello')
process_message(incoming_valid_message)
message = most_recent_message(user_profile)
self.assertEqual(message.content, "Test message")
del incoming_valid_message['To']
incoming_valid_message['To'] = stream_address_prefer_html
process_message(incoming_valid_message)
message = most_recent_message(user_profile)
self.assertEqual(message.content, "**Test html message**")
class TestStreamEmailMessagesEmptyBody(ZulipTestCase): class TestStreamEmailMessagesEmptyBody(ZulipTestCase):
def test_receive_stream_email_messages_empty_body(self) -> None: def test_receive_stream_email_messages_empty_body(self) -> None:
# build dummy messages for stream # build dummy messages for stream