move conversion of data to json in slack API handling (#1101) (#1120)

* move conversion of data to json in slack API handling

at one point in do_notify_slack, we do operations on the payload
variable assuming it's a dict, but it's not: it's a json encoded string.

it's useful to operate on the payload as a dict rather than a string, so
solve this problem by moving the jsonify call to right before sending
the payload to the slack API.

fixes #1097

* add changelog fragment

* Update changelogs/fragments/1101-slack-ts-fix.yaml

Co-authored-by: Felix Fontein <felix@fontein.de>

* return payload as a json encoded string for backwards compatibility

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit b5b5410575)

Co-authored-by: Andreas Lutro <anlutro@gmail.com>

changelogs/fragments/1101-slack-ts-fix.yaml

@@ -0,0 +1,2 @@
+bugfixes:
+- slack - avoid trying to update existing message when sending messages that contain the string "ts" (https://github.com/ansible-collections/community.general/issues/1097).

plugins/modules/notification/slack.py

@@ -328,7 +328,6 @@ def build_payload_for_slack(module, text, channel, thread_id, username, icon_url
         ]
         payload['blocks'] = recursive_escape_quotes(blocks, block_keys_to_escape)
 
-    payload = module.jsonify(payload)
     return payload
 
 
@@ -377,14 +376,15 @@ def do_notify_slack(module, domain, token, payload):
     if use_webapi:
         headers['Authorization'] = 'Bearer ' + token
 
-    response, info = fetch_url(module=module, url=slack_uri, headers=headers, method='POST', data=payload)
+    data = module.jsonify(payload)
+    response, info = fetch_url(module=module, url=slack_uri, headers=headers, method='POST', data=data)
 
     if info['status'] != 200:
         if use_webapi:
             obscured_incoming_webhook = slack_uri
         else:
             obscured_incoming_webhook = SLACK_INCOMING_WEBHOOK % ('[obscured]')
-        module.fail_json(msg=" failed to send %s to %s: %s" % (payload, obscured_incoming_webhook, info['msg']))
+        module.fail_json(msg=" failed to send %s to %s: %s" % (data, obscured_incoming_webhook, info['msg']))
 
     # each API requires different handling
     if use_webapi:
@@ -459,8 +459,10 @@ def main():
     if 'ok' in slack_response:
         # Evaluate WebAPI response
         if slack_response['ok']:
+            # return payload as a string for backwards compatibility
+            payload_json = module.jsonify(payload)
             module.exit_json(changed=changed, ts=slack_response['ts'], channel=slack_response['channel'],
-                             api=slack_response, payload=payload)
+                             api=slack_response, payload=payload_json)
         else:
             module.fail_json(msg="Slack API error", error=slack_response['error'])
     else:

tests/unit/plugins/modules/notification/test_slack.py

@@ -5,7 +5,7 @@ __metaclass__ = type
 
 import json
 import pytest
-from ansible_collections.community.general.tests.unit.compat.mock import patch
+from ansible_collections.community.general.tests.unit.compat.mock import Mock, patch
 from ansible_collections.community.general.plugins.modules.notification import slack
 from ansible_collections.community.general.tests.unit.plugins.modules.utils import AnsibleExitJson, AnsibleFailJson, ModuleTestCase, set_module_args
 
@@ -88,6 +88,45 @@ class TestSlackModule(ModuleTestCase):
             assert call_data['thread_ts'] == '100.00'
             assert fetch_url_mock.call_args[1]['url'] == "https://hooks.slack.com/services/XXXX/YYYY/ZZZZ"
 
+    # https://github.com/ansible-collections/community.general/issues/1097
+    def test_ts_in_message_does_not_cause_edit(self):
+        set_module_args({
+            'token': 'xoxa-123456789abcdef',
+            'msg': 'test with ts'
+        })
+
+        with patch.object(slack, "fetch_url") as fetch_url_mock:
+            mock_response = Mock()
+            mock_response.read.return_value = '{"fake":"data"}'
+            fetch_url_mock.return_value = (mock_response, {"status": 200})
+            with self.assertRaises(AnsibleExitJson):
+                self.module.main()
+
+            self.assertTrue(fetch_url_mock.call_count, 1)
+            self.assertEquals(fetch_url_mock.call_args[1]['url'], "https://slack.com/api/chat.postMessage")
+
+    def test_edit_message(self):
+        set_module_args({
+            'token': 'xoxa-123456789abcdef',
+            'msg': 'test2',
+            'message_id': '12345'
+        })
+
+        with patch.object(slack, "fetch_url") as fetch_url_mock:
+            mock_response = Mock()
+            mock_response.read.return_value = '{"messages":[{"ts":"12345","msg":"test1"}]}'
+            fetch_url_mock.side_effect = [
+                (mock_response, {"status": 200}),
+                (mock_response, {"status": 200}),
+            ]
+            with self.assertRaises(AnsibleExitJson):
+                self.module.main()
+
+            self.assertTrue(fetch_url_mock.call_count, 2)
+            self.assertEquals(fetch_url_mock.call_args[1]['url'], "https://slack.com/api/chat.update")
+            call_data = json.loads(fetch_url_mock.call_args[1]['data'])
+            self.assertEquals(call_data['ts'], "12345")
+
     def test_message_with_blocks(self):
         """tests sending a message with blocks"""
         set_module_args({