diff --git a/redash/destinations/webex.py b/redash/destinations/webex.py index 16d8ed05a..599c485e3 100644 --- a/redash/destinations/webex.py +++ b/redash/destinations/webex.py @@ -1,3 +1,5 @@ +import html +import json import logging from copy import deepcopy @@ -37,6 +39,129 @@ class Webex(BaseDestination): @staticmethod def formatted_attachments_template(subject, description, query_link, alert_link): + # Attempt to parse the description to find a 2D array + try: + # Extract the part of the description that looks like a JSON array + start_index = description.find("[") + end_index = description.rfind("]") + 1 + json_array_str = description[start_index:end_index] + + # Decode HTML entities + json_array_str = html.unescape(json_array_str) + + # Replace single quotes with double quotes for valid JSON + json_array_str = json_array_str.replace("'", '"') + + # Load the JSON array + data_array = json.loads(json_array_str) + + # Check if it's a 2D array + if isinstance(data_array, list) and all(isinstance(i, list) for i in data_array): + # Create a table for the Adaptive Card + table_rows = [] + for row in data_array: + table_rows.append( + { + "type": "ColumnSet", + "columns": [ + {"type": "Column", "items": [{"type": "TextBlock", "text": str(item), "wrap": True}]} + for item in row + ], + } + ) + + # Create the body of the card with the table + body = ( + [ + { + "type": "TextBlock", + "text": f"{subject}", + "weight": "bolder", + "size": "medium", + "wrap": True, + }, + { + "type": "TextBlock", + "text": f"{description[:start_index]}", + "isSubtle": True, + "wrap": True, + }, + ] + + table_rows + + [ + { + "type": "TextBlock", + "text": f"Click [here]({query_link}) to check your query!", + "wrap": True, + "isSubtle": True, + }, + { + "type": "TextBlock", + "text": f"Click [here]({alert_link}) to check your alert!", + "wrap": True, + "isSubtle": True, + }, + ] + ) + else: + # Fallback to the original description if no valid 2D array is found + body = [ + { + "type": "TextBlock", + "text": f"{subject}", + "weight": "bolder", + "size": "medium", + "wrap": True, + }, + { + "type": "TextBlock", + "text": f"{description}", + "isSubtle": True, + "wrap": True, + }, + { + "type": "TextBlock", + "text": f"Click [here]({query_link}) to check your query!", + "wrap": True, + "isSubtle": True, + }, + { + "type": "TextBlock", + "text": f"Click [here]({alert_link}) to check your alert!", + "wrap": True, + "isSubtle": True, + }, + ] + except json.JSONDecodeError: + # If parsing fails, fallback to the original description + body = [ + { + "type": "TextBlock", + "text": f"{subject}", + "weight": "bolder", + "size": "medium", + "wrap": True, + }, + { + "type": "TextBlock", + "text": f"{description}", + "isSubtle": True, + "wrap": True, + }, + { + "type": "TextBlock", + "text": f"Click [here]({query_link}) to check your query!", + "wrap": True, + "isSubtle": True, + }, + { + "type": "TextBlock", + "text": f"Click [here]({alert_link}) to check your alert!", + "wrap": True, + "isSubtle": True, + }, + ] + return [ { "contentType": "application/vnd.microsoft.card.adaptive", @@ -44,44 +169,7 @@ class Webex(BaseDestination): "$schema": "http://adaptivecards.io/schemas/adaptive-card.json", "type": "AdaptiveCard", "version": "1.0", - "body": [ - { - "type": "ColumnSet", - "columns": [ - { - "type": "Column", - "width": 4, - "items": [ - { - "type": "TextBlock", - "text": {subject}, - "weight": "bolder", - "size": "medium", - "wrap": True, - }, - { - "type": "TextBlock", - "text": {description}, - "isSubtle": True, - "wrap": True, - }, - { - "type": "TextBlock", - "text": f"Click [here]({query_link}) to check your query!", - "wrap": True, - "isSubtle": True, - }, - { - "type": "TextBlock", - "text": f"Click [here]({alert_link}) to check your alert!", - "wrap": True, - "isSubtle": True, - }, - ], - }, - ], - } - ], + "body": body, }, } ] @@ -116,6 +204,10 @@ class Webex(BaseDestination): # destinations is guaranteed to be a comma-separated string for destination_id in destinations.split(","): + destination_id = destination_id.strip() # Remove any leading or trailing whitespace + if not destination_id: # Check if the destination_id is empty or blank + continue # Skip to the next iteration if it's empty or blank + payload = deepcopy(template_payload) payload[payload_tag] = destination_id self.post_message(payload, headers) diff --git a/tests/handlers/test_destinations.py b/tests/handlers/test_destinations.py index 95fcffad0..00a52cbb0 100644 --- a/tests/handlers/test_destinations.py +++ b/tests/handlers/test_destinations.py @@ -261,15 +261,19 @@ def test_webex_notify_calls_requests_post(): alert.name = "Test Alert" alert.custom_subject = "Test custom subject" alert.custom_body = "Test custom body" - alert.render_template = mock.Mock(return_value={"Rendered": "template"}) + query = mock.Mock() query.id = 1 user = mock.Mock() app = mock.Mock() host = "https://localhost:5000" - options = {"webex_bot_token": "abcd", "to_room_ids": "1234"} + options = { + "webex_bot_token": "abcd", + "to_room_ids": "1234,5678", + "to_person_emails": "example1@test.com,example2@test.com", + } metadata = {"Scheduled": False} new_state = Alert.TRIGGERED_STATE @@ -277,7 +281,7 @@ def test_webex_notify_calls_requests_post(): with mock.patch("redash.destinations.webex.requests.post") as mock_post: mock_response = mock.Mock() - mock_response.status_code = 204 + mock_response.status_code = 200 mock_post.return_value = mock_response destination.notify(alert, query, user, new_state, app, host, metadata, options) @@ -285,13 +289,111 @@ def test_webex_notify_calls_requests_post(): query_link = f"{host}/queries/{query.id}" alert_link = f"{host}/alerts/{alert.id}" - formatted_attachments = Webex.formatted_attachments_template( + expected_attachments = Webex.formatted_attachments_template( + alert.custom_subject, alert.custom_body, query_link, alert_link + ) + + expected_payload_room = { + "markdown": alert.custom_subject + "\n" + alert.custom_body, + "attachments": expected_attachments, + "roomId": "1234", + } + + expected_payload_email = { + "markdown": alert.custom_subject + "\n" + alert.custom_body, + "attachments": expected_attachments, + "toPersonEmail": "example1@test.com", + } + + # Check that requests.post was called for both roomId and toPersonEmail destinations + mock_post.assert_any_call( + destination.api_base_url, + json=expected_payload_room, + headers={"Authorization": "Bearer abcd"}, + timeout=5.0, + ) + + mock_post.assert_any_call( + destination.api_base_url, + json=expected_payload_email, + headers={"Authorization": "Bearer abcd"}, + timeout=5.0, + ) + + assert mock_response.status_code == 200 + + +def test_webex_notify_handles_blank_entries(): + alert = mock.Mock(spec_set=["id", "name", "custom_subject", "custom_body", "render_template"]) + alert.id = 1 + alert.name = "Test Alert" + alert.custom_subject = "Test custom subject" + alert.custom_body = "Test custom body" + alert.render_template = mock.Mock(return_value={"Rendered": "template"}) + + query = mock.Mock() + query.id = 1 + + user = mock.Mock() + app = mock.Mock() + host = "https://localhost:5000" + options = { + "webex_bot_token": "abcd", + "to_room_ids": "", + "to_person_emails": "", + } + metadata = {"Scheduled": False} + + new_state = Alert.TRIGGERED_STATE + destination = Webex(options) + + with mock.patch("redash.destinations.webex.requests.post") as mock_post: + destination.notify(alert, query, user, new_state, app, host, metadata, options) + + # Ensure no API calls are made when destinations are blank + mock_post.assert_not_called() + + +def test_webex_notify_handles_2d_array(): + alert = mock.Mock(spec_set=["id", "name", "custom_subject", "custom_body", "render_template"]) + alert.id = 1 + alert.name = "Test Alert" + alert.custom_subject = "Test custom subject" + alert.custom_body = "Test custom body with table [['Col1', 'Col2'], ['Val1', 'Val2']]" + alert.render_template = mock.Mock(return_value={"Rendered": "template"}) + + query = mock.Mock() + query.id = 1 + + user = mock.Mock() + app = mock.Mock() + host = "https://localhost:5000" + options = { + "webex_bot_token": "abcd", + "to_room_ids": "1234", + } + metadata = {"Scheduled": False} + + new_state = Alert.TRIGGERED_STATE + destination = Webex(options) + + with mock.patch("redash.destinations.webex.requests.post") as mock_post: + mock_response = mock.Mock() + mock_response.status_code = 200 + mock_post.return_value = mock_response + + destination.notify(alert, query, user, new_state, app, host, metadata, options) + + query_link = f"{host}/queries/{query.id}" + alert_link = f"{host}/alerts/{alert.id}" + + expected_attachments = Webex.formatted_attachments_template( alert.custom_subject, alert.custom_body, query_link, alert_link ) expected_payload = { "markdown": alert.custom_subject + "\n" + alert.custom_body, - "attachments": formatted_attachments, + "attachments": expected_attachments, "roomId": "1234", } @@ -302,7 +404,60 @@ def test_webex_notify_calls_requests_post(): timeout=5.0, ) - assert mock_response.status_code == 204 + assert mock_response.status_code == 200 + + +def test_webex_notify_handles_1d_array(): + alert = mock.Mock(spec_set=["id", "name", "custom_subject", "custom_body", "render_template"]) + alert.id = 1 + alert.name = "Test Alert" + alert.custom_subject = "Test custom subject" + alert.custom_body = "Test custom body with 1D array, however unlikely ['Col1', 'Col2']" + alert.render_template = mock.Mock(return_value={"Rendered": "template"}) + + query = mock.Mock() + query.id = 1 + + user = mock.Mock() + app = mock.Mock() + host = "https://localhost:5000" + options = { + "webex_bot_token": "abcd", + "to_room_ids": "1234", + } + metadata = {"Scheduled": False} + + new_state = Alert.TRIGGERED_STATE + destination = Webex(options) + + with mock.patch("redash.destinations.webex.requests.post") as mock_post: + mock_response = mock.Mock() + mock_response.status_code = 200 + mock_post.return_value = mock_response + + destination.notify(alert, query, user, new_state, app, host, metadata, options) + + query_link = f"{host}/queries/{query.id}" + alert_link = f"{host}/alerts/{alert.id}" + + expected_attachments = Webex.formatted_attachments_template( + alert.custom_subject, alert.custom_body, query_link, alert_link + ) + + expected_payload = { + "markdown": alert.custom_subject + "\n" + alert.custom_body, + "attachments": expected_attachments, + "roomId": "1234", + } + + mock_post.assert_called_once_with( + destination.api_base_url, + json=expected_payload, + headers={"Authorization": "Bearer abcd"}, + timeout=5.0, + ) + + assert mock_response.status_code == 200 def test_datadog_notify_calls_requests_post():