diff --git a/zerver/webhooks/slack_incoming/fixtures/attachment_fields.json b/zerver/webhooks/slack_incoming/fixtures/attachment_fields.json index 173da6bd12..72232d6466 100644 --- a/zerver/webhooks/slack_incoming/fixtures/attachment_fields.json +++ b/zerver/webhooks/slack_incoming/fixtures/attachment_fields.json @@ -17,6 +17,21 @@ "title": "Build pipeline", "value": "ConsumerAddressModule", "short": true + }, + { + "title": "Title without value", + "value": null, + "short": true + }, + { + "title": null, + "value": "Value without title", + "short": true + }, + { + "title": null, + "value": null, + "short": true } ], "pretext": "Build bla bla succeeded", diff --git a/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_all_null.json b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_all_null.json new file mode 100644 index 0000000000..ad1a3e448e --- /dev/null +++ b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_all_null.json @@ -0,0 +1,13 @@ +{ + "attachments": [ + { + "title": null, + "title_link": null, + "image_url": null, + "ts": null, + "text": null, + "pretext": null, + "footer": null + } + ] +} diff --git a/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_footer_null.json b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_footer_null.json new file mode 100644 index 0000000000..01d92ab61e --- /dev/null +++ b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_footer_null.json @@ -0,0 +1,13 @@ +{ + "attachments": [ + { + "title": "Sample title.", + "title_link": "https://www.google.com", + "image_url": "https://pbs.twimg.com/profile_images/625633822235693056/lNGUneLX_400x400.jpg", + "ts": 1655945306, + "text": "Sample text.", + "pretext": "Sample pretext.", + "footer": null + } + ] +} diff --git a/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_image_url_null.json b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_image_url_null.json new file mode 100644 index 0000000000..424f7cea80 --- /dev/null +++ b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_image_url_null.json @@ -0,0 +1,13 @@ +{ + "attachments": [ + { + "title": "Sample title.", + "title_link": "https://www.google.com", + "image_url": null, + "ts": 1655945306, + "text": "Sample text.", + "pretext": "Sample pretext.", + "footer": "Sample footer." + } + ] +} diff --git a/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_pretext_null.json b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_pretext_null.json new file mode 100644 index 0000000000..70c53ef2a1 --- /dev/null +++ b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_pretext_null.json @@ -0,0 +1,13 @@ +{ + "attachments": [ + { + "title": "Sample title.", + "title_link": "https://www.google.com", + "image_url": "https://pbs.twimg.com/profile_images/625633822235693056/lNGUneLX_400x400.jpg", + "ts": 1655945306, + "text": "Sample text.", + "pretext": null, + "footer": "Sample footer." + } + ] +} diff --git a/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_text_null.json b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_text_null.json new file mode 100644 index 0000000000..f9233d3850 --- /dev/null +++ b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_text_null.json @@ -0,0 +1,13 @@ +{ + "attachments": [ + { + "title": "Sample title.", + "title_link": "https://www.google.com", + "image_url": "https://pbs.twimg.com/profile_images/625633822235693056/lNGUneLX_400x400.jpg", + "ts": 1655945306, + "text": null, + "pretext": "Sample pretext.", + "footer": "Sample footer." + } + ] +} diff --git a/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_title_link_null.json b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_title_link_null.json new file mode 100644 index 0000000000..9793750b2a --- /dev/null +++ b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_title_link_null.json @@ -0,0 +1,13 @@ +{ + "attachments": [ + { + "title": "Sample title.", + "title_link": null, + "image_url": "https://pbs.twimg.com/profile_images/625633822235693056/lNGUneLX_400x400.jpg", + "ts": 1655945306, + "text": "Sample text.", + "pretext": "Sample pretext.", + "footer": "Sample footer." + } + ] +} diff --git a/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_title_null.json b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_title_null.json new file mode 100644 index 0000000000..7f7b69d3b6 --- /dev/null +++ b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_title_null.json @@ -0,0 +1,13 @@ +{ + "attachments": [ + { + "title": null, + "title_link": "https://www.google.com", + "image_url": "https://pbs.twimg.com/profile_images/625633822235693056/lNGUneLX_400x400.jpg", + "ts": 1655945306, + "text": "Sample text.", + "pretext": "Sample pretext.", + "footer": "Sample footer." + } + ] +} diff --git a/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_ts_null.json b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_ts_null.json new file mode 100644 index 0000000000..a045f289e3 --- /dev/null +++ b/zerver/webhooks/slack_incoming/fixtures/attachment_pieces_ts_null.json @@ -0,0 +1,13 @@ +{ + "attachments": [ + { + "title": "Sample title.", + "title_link": "https://www.google.com", + "image_url": "https://pbs.twimg.com/profile_images/625633822235693056/lNGUneLX_400x400.jpg", + "ts": null, + "text": "Sample text.", + "pretext": "Sample pretext.", + "footer": "Sample footer." + } + ] +} diff --git a/zerver/webhooks/slack_incoming/tests.py b/zerver/webhooks/slack_incoming/tests.py index 543cd2c17a..51107764b8 100644 --- a/zerver/webhooks/slack_incoming/tests.py +++ b/zerver/webhooks/slack_incoming/tests.py @@ -167,6 +167,8 @@ Build bla bla succeeded **Requested by**: Some user **Duration**: 00:02:03 **Build pipeline**: ConsumerAddressModule +**Title without value** +Value without title """.strip() self.check_webhook( @@ -197,3 +199,148 @@ Build bla bla succeeded else: file_type = "json" return self.webhook_fixture_data("slack_incoming", fixture_name, file_type=file_type) + + def test_attachment_pieces_title_null(self) -> None: + expected_topic = "(no topic)" + expected_message = """ +Sample pretext. + +Sample text. + +[](https://pbs.twimg.com/profile_images/625633822235693056/lNGUneLX_400x400.jpg) + +Sample footer. + + + """.strip() + + self.check_webhook( + "attachment_pieces_title_null", + expected_topic, + expected_message, + ) + + def test_attachment_pieces_image_url_null(self) -> None: + expected_topic = "(no topic)" + expected_message = """ +## [Sample title.](https://www.google.com) + +Sample pretext. + +Sample text. + +Sample footer. + + + """.strip() + + self.check_webhook( + "attachment_pieces_image_url_null", + expected_topic, + expected_message, + ) + + def test_attachment_pieces_ts_null(self) -> None: + expected_topic = "(no topic)" + expected_message = """ +## [Sample title.](https://www.google.com) + +Sample pretext. + +Sample text. + +[](https://pbs.twimg.com/profile_images/625633822235693056/lNGUneLX_400x400.jpg) + +Sample footer. + """.strip() + + self.check_webhook( + "attachment_pieces_ts_null", + expected_topic, + expected_message, + ) + + def test_attachment_pieces_text_null(self) -> None: + expected_topic = "(no topic)" + expected_message = """ +## [Sample title.](https://www.google.com) + +Sample pretext. + +[](https://pbs.twimg.com/profile_images/625633822235693056/lNGUneLX_400x400.jpg) + +Sample footer. + + + """.strip() + + self.check_webhook( + "attachment_pieces_text_null", + expected_topic, + expected_message, + ) + + def test_attachment_pieces_pretext_null(self) -> None: + expected_topic = "(no topic)" + expected_message = """ +## [Sample title.](https://www.google.com) + +Sample text. + +[](https://pbs.twimg.com/profile_images/625633822235693056/lNGUneLX_400x400.jpg) + +Sample footer. + + + """.strip() + + self.check_webhook( + "attachment_pieces_pretext_null", + expected_topic, + expected_message, + ) + + def test_attachment_pieces_footer_null(self) -> None: + expected_topic = "(no topic)" + expected_message = """ +## [Sample title.](https://www.google.com) + +Sample pretext. + +Sample text. + +[](https://pbs.twimg.com/profile_images/625633822235693056/lNGUneLX_400x400.jpg) + + + """.strip() + + self.check_webhook( + "attachment_pieces_footer_null", + expected_topic, + expected_message, + ) + + def test_attachment_pieces_title_link_null(self) -> None: + expected_topic = "(no topic)" + expected_message = """ +## Sample title. + +Sample pretext. + +Sample text. + +[](https://pbs.twimg.com/profile_images/625633822235693056/lNGUneLX_400x400.jpg) + +Sample footer. + + + """.strip() + + self.check_webhook( + "attachment_pieces_title_link_null", + expected_topic, + expected_message, + ) + + def test_attachment_pieces_all_null(self) -> None: + self.check_webhook("attachment_pieces_all_null", expect_noop=True) diff --git a/zerver/webhooks/slack_incoming/view.py b/zerver/webhooks/slack_incoming/view.py index 10901dc276..a3e42d5c65 100644 --- a/zerver/webhooks/slack_incoming/view.py +++ b/zerver/webhooks/slack_incoming/view.py @@ -175,33 +175,43 @@ def render_block_element(element: WildValue) -> str: def render_attachment(attachment: WildValue) -> str: # https://api.slack.com/reference/messaging/attachments + # Slack recommends the usage of "blocks" even within attachments; the + # rest of the fields we handle here are legacy fields. These fields are + # optional and may contain null values. pieces = [] - if "title" in attachment: + if "title" in attachment and attachment["title"]: title = attachment["title"].tame(check_string) - if "title_link" in attachment: + if "title_link" in attachment and attachment["title_link"]: title_link = attachment["title_link"].tame(check_url) pieces.append(f"## [{title}]({title_link})") else: pieces.append(f"## {title}") - if "pretext" in attachment: + if "pretext" in attachment and attachment["pretext"]: pieces.append(attachment["pretext"].tame(check_string)) - if "text" in attachment: + if "text" in attachment and attachment["text"]: pieces.append(attachment["text"].tame(check_string)) if "fields" in attachment: fields = [] for field in attachment["fields"]: - title = field["title"].tame(check_string) - value = field["value"].tame(check_string) - fields.append(f"*{title}*: {value}") + if field["title"] and field["value"]: + title = field["title"].tame(check_string) + value = field["value"].tame(check_string) + fields.append(f"*{title}*: {value}") + elif field["title"]: + title = field["title"].tame(check_string) + fields.append(f"*{title}*") + elif field["value"]: + value = field["value"].tame(check_string) + fields.append(f"{value}") pieces.append("\n".join(fields)) if "blocks" in attachment and attachment["blocks"]: for block in attachment["blocks"]: pieces.append(render_block(block)) - if "image_url" in attachment: + if "image_url" in attachment and attachment["image_url"]: pieces.append("[]({})".format(attachment["image_url"].tame(check_url))) - if "footer" in attachment: + if "footer" in attachment and attachment["footer"]: pieces.append(attachment["footer"].tame(check_string)) - if "ts" in attachment: + if "ts" in attachment and attachment["ts"]: time = attachment["ts"].tame(check_int) pieces.append(f"")