From c8c480baef3e754fe6b314a56dd9d6e059554e40 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Mon, 19 Aug 2024 21:03:17 +0000 Subject: [PATCH] mattermost: Handle duplicate dm-groups. Observed in the wild, cause unknown. Partially fixes: #24131. Co-authored-by: Mateusz Mandera --- zerver/data_import/mattermost.py | 3 +++ .../direct_channel/export.json | 1 + zerver/tests/test_mattermost_importer.py | 23 +++++++++++-------- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/zerver/data_import/mattermost.py b/zerver/data_import/mattermost.py index dab0a29546..4d45465de7 100644 --- a/zerver/data_import/mattermost.py +++ b/zerver/data_import/mattermost.py @@ -244,6 +244,9 @@ def convert_direct_message_group_data( for direct_message_group in direct_message_group_data: if len(direct_message_group["members"]) > 2: direct_message_group_members = frozenset(direct_message_group["members"]) + if direct_message_group_id_mapper.has(direct_message_group_members): + logging.info("Duplicate direct message group found in the export data. Skipping.") + continue direct_message_group_id = direct_message_group_id_mapper.get( direct_message_group_members ) diff --git a/zerver/tests/fixtures/mattermost_fixtures/direct_channel/export.json b/zerver/tests/fixtures/mattermost_fixtures/direct_channel/export.json index 3f3b2ce09d..be785e3875 100644 --- a/zerver/tests/fixtures/mattermost_fixtures/direct_channel/export.json +++ b/zerver/tests/fixtures/mattermost_fixtures/direct_channel/export.json @@ -13,6 +13,7 @@ {"type":"post","post":{"team":"gryffindor","channel":"gryffindor-common-room","user":"harry","message":"Looks like this channel is empty","create_at":1553166567370,"reactions":[{"user":"ron","create_at":1553166584976,"emoji_name":"rocket"}],"replies":null,"attachments":[{"path":"20210622/teams/noteam/channels/mcrm7xee5bnpzn7u9ktsd91dwy/users/knq189b88fdxbdkeeasdynia4o/smaa5epsnp89tgjszzue1691ao/this is a file"}]}} {"type":"direct_channel","direct_channel":{"members":["ron","harry"],"favorited_by":null,"header":""}} {"type":"direct_channel","direct_channel":{"members":["ron","harry", "ginny"],"favorited_by":null,"header":""}} +{"type":"direct_channel","direct_channel":{"members":["harry","ron", "ginny"],"favorited_by":null,"header":""}} {"type":"direct_post","direct_post":{"channel_members":["ron","harry"],"user":"ron","message":"hey harry","create_at":1566376137676,"flagged_by":null,"reactions":null,"replies":null,"attachments":[{"path":"20210622/teams/noteam/channels/mcrm7xee5bnpzn7u9ktsd91dwy/users/knq189b88fdxbdkeeasdynia4o/o3to4ezua3bajj31mzpkn96n5e/harry-ron.jpg"}]}} {"type":"direct_post","direct_post":{"channel_members":["ron","harry"],"user":"harry","message":"what's up","create_at":1566376318568,"flagged_by":null,"reactions":null,"replies":null,"attachments":null}} {"type":"direct_post","direct_post":{"channel_members":["ron","harry","ginny"],"user":"ginny","message":"Who is going to Hogsmeade this weekend?","create_at":1566376226493,"flagged_by":null,"reactions":null,"replies":null,"attachments":null}} diff --git a/zerver/tests/test_mattermost_importer.py b/zerver/tests/test_mattermost_importer.py index ab0dbb4644..b368db337d 100644 --- a/zerver/tests/test_mattermost_importer.py +++ b/zerver/tests/test_mattermost_importer.py @@ -357,15 +357,16 @@ class MatterMostImporter(ZulipTestCase): team_name=team_name, ) - zerver_huddle = convert_direct_message_group_data( - direct_message_group_data=mattermost_data["direct_channel"], - user_data_map=username_to_user, - subscriber_handler=subscriber_handler, - direct_message_group_id_mapper=direct_message_group_id_mapper, - user_id_mapper=user_id_mapper, - realm_id=3, - team_name=team_name, - ) + with self.assertLogs(level="INFO") as mock_log: + zerver_huddle = convert_direct_message_group_data( + direct_message_group_data=mattermost_data["direct_channel"], + user_data_map=username_to_user, + subscriber_handler=subscriber_handler, + direct_message_group_id_mapper=direct_message_group_id_mapper, + user_id_mapper=user_id_mapper, + realm_id=3, + team_name=team_name, + ) self.assert_length(zerver_huddle, 1) direct_message_group_members = frozenset(mattermost_data["direct_channel"][1]["members"]) @@ -379,6 +380,10 @@ class MatterMostImporter(ZulipTestCase): ), {1, 2, 3}, ) + self.assertEqual( + mock_log.output, + ["INFO:root:Duplicate direct message group found in the export data. Skipping."], + ) def test_write_emoticon_data(self) -> None: fixture_file_name = self.fixture_file_name("export.json", "mattermost_fixtures")