From 57800e85c7b867d07d191a3fcf11bd0e0215f168 Mon Sep 17 00:00:00 2001
From: Alex Vandiver <alexmv@zulip.com>
Date: Wed, 17 Jan 2024 19:20:31 +0000
Subject: [PATCH] import: Support shared users in huddles/DMs.

1e5c49ad825d added support for shared channels -- but some users may
only currently exist in DMs or MPIMs, and not in channel membership.

Walk the list of MPIM subscriptions and messages, as well as DM users,
and add any such users to the set of mirror dummy users.

(cherry picked from commit 516d1083dbaeb279373f8f5fd4e4e7985889081b)
---
 zerver/data_import/slack.py         | 12 ++++++++-
 zerver/tests/test_slack_importer.py | 40 +++++++++++++++++++++--------
 2 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/zerver/data_import/slack.py b/zerver/data_import/slack.py
index 5c069cb973..2a6daf08ca 100644
--- a/zerver/data_import/slack.py
+++ b/zerver/data_import/slack.py
@@ -1256,11 +1256,21 @@ def fetch_shared_channel_users(
         private_channels = get_data_file(slack_data_dir + "/groups.json")
     except FileNotFoundError:
         private_channels = []
-    for channel in public_channels + private_channels:
+    try:
+        huddles = get_data_file(slack_data_dir + "/mpims.json")
+    except FileNotFoundError:
+        huddles = []
+    for channel in public_channels + private_channels + huddles:
         added_channels[channel["name"]] = True
         for user_id in channel["members"]:
             if user_id not in normal_user_ids:
                 mirror_dummy_user_ids.add(user_id)
+    if os.path.exists(slack_data_dir + "/dms.json"):
+        dms = get_data_file(slack_data_dir + "/dms.json")
+        for dm_data in dms:
+            for user_id in dm_data["members"]:
+                if user_id not in normal_user_ids:
+                    mirror_dummy_user_ids.add(user_id)
 
     all_messages = get_messages_iterator(slack_data_dir, added_channels, {}, {})
     for message in all_messages:
diff --git a/zerver/tests/test_slack_importer.py b/zerver/tests/test_slack_importer.py
index 09d27bf805..45ad1430d5 100644
--- a/zerver/tests/test_slack_importer.py
+++ b/zerver/tests/test_slack_importer.py
@@ -84,6 +84,9 @@ def request_callback(request: PreparedRequest) -> Tuple[int, Dict[str, str], byt
             "U061A3E0G": "T6LARQE2Z",
             "U061A8H1G": "T7KJRQE8Y",
             "U8X25EBAB": "T5YFFM2QY",
+            "U11111111": "T7KJRQE8Y",
+            "U22222222": "T7KJRQE8Y",
+            "U33333333": "T7KJRQE8Y",
         }
         try:
             user_id = qs["user"][0]
@@ -304,11 +307,22 @@ class SlackImporter(ZulipTestCase):
     ) -> None:
         users = [{"id": "U061A1R2R"}, {"id": "U061A5N1G"}, {"id": "U064KUGRJ"}]
         data_file_mock.side_effect = [
-            [
+            [  # Channels
                 {"name": "general", "members": ["U061A1R2R", "U061A5N1G"]},
                 {"name": "sharedchannel", "members": ["U061A1R2R", "U061A3E0G"]},
             ],
-            [],
+            [  # Private channels ("groups")
+                {"name": "private", "members": ["U061A1R2R", "U11111111"]},
+            ],
+            [  # Huddles ("mpims")
+                {
+                    "name": "mpdm-foo--bar--baz-1",
+                    "members": ["U061A1R2R", "U061A5N1G", "U22222222"],
+                },
+            ],
+            [  # DMs
+                {"id": "D123456", "members": ["U064KUGRJ", "U33333333"]},
+            ],
         ]
         messages_mock.return_value = [
             {"user": "U061A1R2R"},
@@ -325,7 +339,7 @@ class SlackImporter(ZulipTestCase):
         fetch_shared_channel_users(users, slack_data_dir, "xoxb-valid-token")
 
         # Normal users
-        self.assert_length(users, 5)
+        self.assert_length(users, 8)
         self.assertEqual(users[0]["id"], "U061A1R2R")
         self.assertEqual(users[0]["is_mirror_dummy"], False)
         self.assertFalse("team_domain" in users[0])
@@ -335,14 +349,18 @@ class SlackImporter(ZulipTestCase):
         # Shared channel users
         # We need to do this because the outcome order of `users` list is
         # not deterministic.
-        fourth_fifth = [users[3], users[4]]
-        fourth_fifth.sort(key=lambda x: x["id"])
-        self.assertEqual(fourth_fifth[0]["id"], "U061A3E0G")
-        self.assertEqual(fourth_fifth[0]["team_domain"], "foreignteam1")
-        self.assertEqual(fourth_fifth[0]["is_mirror_dummy"], True)
-        self.assertEqual(fourth_fifth[1]["id"], "U061A8H1G")
-        self.assertEqual(fourth_fifth[1]["team_domain"], "foreignteam2")
-        self.assertEqual(fourth_fifth[1]["is_mirror_dummy"], True)
+        later_users = sorted(users[3:], key=lambda x: x["id"])
+        expected_users = [
+            ("U061A3E0G", "foreignteam1"),
+            ("U061A8H1G", "foreignteam2"),
+            ("U11111111", "foreignteam2"),
+            ("U22222222", "foreignteam2"),
+            ("U33333333", "foreignteam2"),
+        ]
+        for expected, found in zip(expected_users, later_users):
+            self.assertEqual(found["id"], expected[0])
+            self.assertEqual(found["team_domain"], expected[1])
+            self.assertEqual(found["is_mirror_dummy"], True)
 
     @mock.patch("zerver.data_import.slack.get_data_file")
     def test_users_to_zerver_userprofile(self, mock_get_data_file: mock.Mock) -> None: