From a96c61442027bf34d0eade3362aaf8dc0dfbd4d7 Mon Sep 17 00:00:00 2001 From: Riken Shah Date: Mon, 5 Jul 2021 15:55:02 +0000 Subject: [PATCH] message: Fix the edit topic bug by removing leading \n from msg content. The reason for this bug is because of different striping processes in the backend and frontend, i.e The frontend checks if the message's `raw_content` has changed to decide if the `content` of the message should be sent in the request to the backend, or not. So, it removes the leading new line ('\n') from the message `raw_content` when checking it, which is causing the "Error saving edit: You don't have permission to edit this message" error. This commit fixes it by removing the leading new line when cleaning message content. The bug was explained by @punchagan and its solution by @timabbott. --- zerver/lib/message.py | 2 +- zerver/lib/webhooks/common.py | 2 +- zerver/tests/test_message_send.py | 15 +++++++++++++++ zerver/webhooks/opbeat/tests.py | 12 ++++++------ zerver/webhooks/sentry/tests.py | 32 +++++++++++++++---------------- 5 files changed, 39 insertions(+), 24 deletions(-) diff --git a/zerver/lib/message.py b/zerver/lib/message.py index d3a864870f..0d5fa61066 100644 --- a/zerver/lib/message.py +++ b/zerver/lib/message.py @@ -128,7 +128,7 @@ def truncate_content(content: str, max_length: int, truncation_message: str) -> def normalize_body(body: str) -> str: - body = body.rstrip() + body = body.rstrip().lstrip("\n") if len(body) == 0: raise JsonableError(_("Message must not be empty")) if "\x00" in body: diff --git a/zerver/lib/webhooks/common.py b/zerver/lib/webhooks/common.py index 36b0030020..e1d146779a 100644 --- a/zerver/lib/webhooks/common.py +++ b/zerver/lib/webhooks/common.py @@ -20,7 +20,7 @@ from zerver.lib.timestamp import timestamp_to_datetime from zerver.lib.validator import check_list, check_string from zerver.models import UserProfile -MISSING_EVENT_HEADER_MESSAGE = """ +MISSING_EVENT_HEADER_MESSAGE = """\ Hi there! Your bot {bot_name} just sent an HTTP request to {request_path} that is missing the HTTP {header_name} header. Because this header is how {integration_name} indicates the event type, this usually indicates a configuration diff --git a/zerver/tests/test_message_send.py b/zerver/tests/test_message_send.py index 3efcb494f4..94a6133ffe 100644 --- a/zerver/tests/test_message_send.py +++ b/zerver/tests/test_message_send.py @@ -899,6 +899,21 @@ class MessagePOSTTest(ZulipTestCase): sent_message = self.get_last_message() self.assertEqual(sent_message.content, " I like whitespace at the end!") + # Test if it removes the new line from the beginning of the message. + post_data = { + "type": "stream", + "to": "Verona", + "client": "test suite", + "content": "\nAvoid the new line at the beginning of the message.", + "topic": "Test topic", + } + result = self.client_post("/json/messages", post_data) + self.assert_json_success(result) + sent_message = self.get_last_message() + self.assertEqual( + sent_message.content, "Avoid the new line at the beginning of the message." + ) + @override_settings(MAX_MESSAGE_LENGTH=25) def test_long_message(self) -> None: """ diff --git a/zerver/webhooks/opbeat/tests.py b/zerver/webhooks/opbeat/tests.py index fcc3d96181..9ce9b49ccd 100644 --- a/zerver/webhooks/opbeat/tests.py +++ b/zerver/webhooks/opbeat/tests.py @@ -9,7 +9,7 @@ class OpbeatHookTests(WebhookTestCase): def test_comment(self) -> None: expected_topic = "foo commented on E#2" - expected_message = """ + expected_message = """\ **[foo commented on E#2](https://opbeat.com/foo/test-flask-app/errors/2/#activity-5df00003ea4e42458db48446692f6d37)** test comment @@ -25,7 +25,7 @@ test comment def test_new_app(self) -> None: expected_topic = "foo" - expected_message = """ + expected_message = """\ **foo** App foo created @@ -41,7 +41,7 @@ App foo created def test_no_subject_type(self) -> None: expected_topic = "test title" - expected_message = """ + expected_message = """\ **test title** test summary""" self.check_webhook( @@ -53,7 +53,7 @@ test summary""" def test_error_fixed(self) -> None: expected_topic = "foo marked E#2 as fixed" - expected_message = """ + expected_message = """\ **[foo marked E#2 as fixed](https://opbeat.com/test_org/test-flask-app/errors/2/#activity-bf991a45d9184b0ca6fb3d48d3db4c38)** foo marked the error group as fixed @@ -68,7 +68,7 @@ foo marked the error group as fixed def test_error_reopened(self) -> None: expected_topic = "foo reopened E#2" - expected_message = """ + expected_message = """\ **[foo reopened E#2](https://opbeat.com/test_org/test-flask-app/errors/2/#activity-38a556dfc0b04a59a586359bbce1463d)** foo reopened the error group @@ -83,7 +83,7 @@ foo reopened the error group def test_error_regressed(self) -> None: expected_topic = "E#2 regressed" - expected_message = """ + expected_message = """\ **[E#2 regressed](https://opbeat.com/test_org/test-flask-app/errors/2/#activity-c0396f38323a4fa7b314f87d5ed9cdd2)** The error group regressed diff --git a/zerver/webhooks/sentry/tests.py b/zerver/webhooks/sentry/tests.py index dad46968f1..977f7f6ec7 100644 --- a/zerver/webhooks/sentry/tests.py +++ b/zerver/webhooks/sentry/tests.py @@ -8,7 +8,7 @@ class SentryHookTests(WebhookTestCase): def test_event_for_exception_golang(self) -> None: expected_topic = '*url.Error: Get "bad_url": unsupported protocol scheme ""' - expected_message = """ + expected_message = """\ **New exception:** [*url.Error: Get "bad_url": unsupported protocol scheme ""](https://sentry.io/organizations/hypro999-personal-organization/issues/1637164584/events/80777a9cc30e4d0eb8904333d5c298b0/) ```quote **level:** error @@ -34,7 +34,7 @@ Traceback: def test_event_for_exception_node(self) -> None: expected_topic = "Error: Sample error from node." - expected_message = """ + expected_message = """\ **New exception:** [Error: Sample error from node.](https://sentry.io/organizations/hypro999-personal-organization/issues/1638852747/events/f9cb0f2afff74a5aa92e766fb7ac3fe3/) ```quote **level:** error @@ -60,7 +60,7 @@ Traceback: def test_event_for_exception_python(self) -> None: expected_topic = "Exception: Custom exception!" - expected_message = """ + expected_message = """\ **New exception:** [Exception: Custom exception!](https://sentry.io/organizations/hypro999-personal-organization/issues/1635244907/events/599349254a1447a99774b5310711c1a8/) ```quote **level:** error @@ -84,7 +84,7 @@ Traceback: def test_webhook_event_for_exception_python(self) -> None: expected_topic = "ValueError: new sentry error." - expected_message = """ + expected_message = """\ **New exception:** [ValueError: new sentry error.](https://sentry.io/organizations/bar-foundation/issues/1972208801/events/c916dccfd58e41dcabaebef0091f0736/) ```quote **level:** error @@ -107,7 +107,7 @@ Traceback: def test_webhook_event_for_exception_javascript(self) -> None: expected_topic = 'TypeError: can\'t access property "bar", x.foo is undefined' - expected_message = """ + expected_message = """\ **New exception:** [TypeError: can't access property "bar", x.foo is undefined](https://sentry.io/organizations/foo-bar-org/issues/1982047746/events/f3bf5fc4e354451db9e885a69b2a2b51/) ```quote **level:** error @@ -120,7 +120,7 @@ Traceback: def test_event_for_exception_js(self) -> None: expected_topic = "Error: Something external broke." - expected_message = """ + expected_message = """\ **New exception:** [Error: Something external broke.](https://sentry.io/organizations/hypro999-personal-organization/issues/1731239773/events/355c3b2a142046629dd410db2fdda003/) ```quote **level:** error @@ -131,7 +131,7 @@ Traceback: def test_event_for_message_golang(self) -> None: expected_topic = "A test message event from golang." - expected_message = """ + expected_message = """\ **New message event:** [A test message event from golang.](https://sentry.io/organizations/hypro999-personal-organization/issues/1638844654/events/01ecb45633bc4f5ca940ada671124c8f/) ```quote **level:** info @@ -141,7 +141,7 @@ Traceback: def test_event_for_message_node(self) -> None: expected_topic = "Test event from node." - expected_message = """ + expected_message = """\ **New message event:** [Test event from node.](https://sentry.io/organizations/hypro999-personal-organization/issues/1638840427/events/6886bb1fe7ce4497b7836f6083d5fd34/) ```quote **level:** info @@ -151,7 +151,7 @@ Traceback: def test_event_for_message_python(self) -> None: expected_topic = "A simple message-based issue." - expected_message = """ + expected_message = """\ **New message event:** [A simple message-based issue.](https://sentry.io/organizations/hypro999-personal-organization/issues/1635261062/events/8da63b42375e4d3b803c377fefb062f8/) ```quote **level:** info @@ -161,17 +161,17 @@ Traceback: def test_issue_assigned_to_individual(self) -> None: expected_topic = "A test message event from golang." - expected_message = """\nIssue **A test message event from golang.** has now been assigned to **Hemanth V. Alluri** by **Hemanth V. Alluri**.""" + expected_message = """Issue **A test message event from golang.** has now been assigned to **Hemanth V. Alluri** by **Hemanth V. Alluri**.""" self.check_webhook("issue_assigned_to_individual", expected_topic, expected_message) def test_issue_assigned_to_team(self) -> None: expected_topic = "Exception: program has entered an invalid state." - expected_message = """\nIssue **Exception: program has entered an invalid state.** has now been assigned to **team lone-wolf** by **Hemanth V. Alluri**.""" + expected_message = """Issue **Exception: program has entered an invalid state.** has now been assigned to **team lone-wolf** by **Hemanth V. Alluri**.""" self.check_webhook("issue_assigned_to_team", expected_topic, expected_message) def test_issue_created_for_exception(self) -> None: expected_topic = "Exception: Custom exception!" - expected_message = """ + expected_message = """\ **New issue created:** Exception: Custom exception! ```quote **level:** error @@ -182,7 +182,7 @@ Traceback: def test_issue_created_for_message(self) -> None: expected_topic = "A simple message-based issue." - expected_message = """ + expected_message = """\ **New issue created:** A simple message-based issue. ```quote **level:** info @@ -193,17 +193,17 @@ Traceback: def test_issue_ignored(self) -> None: expected_topic = "Exception: program has entered an invalid state." - expected_message = """\nIssue **Exception: program has entered an invalid state.** was ignored by **Hemanth V. Alluri**.""" + expected_message = """Issue **Exception: program has entered an invalid state.** was ignored by **Hemanth V. Alluri**.""" self.check_webhook("issue_ignored", expected_topic, expected_message) def test_issue_resolved(self) -> None: expected_topic = "Exception: program has entered an invalid state." - expected_message = """\nIssue **Exception: program has entered an invalid state.** was marked as resolved by **Hemanth V. Alluri**.""" + expected_message = """Issue **Exception: program has entered an invalid state.** was marked as resolved by **Hemanth V. Alluri**.""" self.check_webhook("issue_resolved", expected_topic, expected_message) def test_deprecated_exception_message(self) -> None: expected_topic = "zulip" - expected_message = """ + expected_message = """\ New [issue](https://sentry.io/zulip/zulip/issues/156699934/) (level: ERROR): ``` quote