From a43bdcd1665ec220cfdd97087a833896a45f51cd Mon Sep 17 00:00:00 2001 From: Mateusz Mandera Date: Sun, 13 Jul 2025 01:19:14 +0800 Subject: [PATCH] import: Fix is_realm_imported_from_other_zulip_server logic. This logic was fundamentally broken: ``` is_realm_imported_from_other_zulip_server = RealmAuditLog.objects.filter( realm=realm, event_type=AuditLogEventType.REALM_EXPORTED ).exists() if not is_realm_imported_from_other_zulip_server: send_zulip_update_announcements_to_realm( realm, skip_delay=False, realm_imported_from_other_product=True ) ``` Because the `REALM_EXPORTED` was only created after the export completed - meaning it couldn't be included in the export data at all. Thus considering exports to be "not from Zulip" incorrectly. We get around this issue by explicitly including an import_source in the realm dict in the export data from 3rd party apps. The importer can then rely on this value to determine if it's dealing with a Zulip-originated export or not. --- zerver/data_import/import_util.py | 3 ++- zerver/data_import/mattermost.py | 2 +- zerver/data_import/rocketchat.py | 2 +- zerver/data_import/slack.py | 2 +- zerver/lib/export.py | 7 +++++-- zerver/lib/import_realm.py | 8 +++++--- zerver/tests/test_slack_importer.py | 3 ++- 7 files changed, 17 insertions(+), 10 deletions(-) diff --git a/zerver/data_import/import_util.py b/zerver/data_import/import_util.py index 92fe78f3bc..8bfdd3ff69 100644 --- a/zerver/data_import/import_util.py +++ b/zerver/data_import/import_util.py @@ -359,7 +359,7 @@ def build_recipients( def build_realm( - zerver_realm: list[ZerverFieldsT], realm_id: int, domain_name: str + zerver_realm: list[ZerverFieldsT], realm_id: int, domain_name: str, import_source: str ) -> ZerverFieldsT: realm = dict( zerver_client=[ @@ -386,6 +386,7 @@ def build_realm( {"realm": realm_id, "name": name, "id": i} for i, name in enumerate(all_default_backend_names(), start=1) ], + import_source=import_source, ) return realm diff --git a/zerver/data_import/mattermost.py b/zerver/data_import/mattermost.py index 4aab1ddb53..a401abb417 100644 --- a/zerver/data_import/mattermost.py +++ b/zerver/data_import/mattermost.py @@ -54,7 +54,7 @@ def make_realm(realm_id: int, team: dict[str, Any]) -> ZerverFieldsT: realm_subdomain = team["name"] zerver_realm = build_zerver_realm(realm_id, realm_subdomain, NOW, "Mattermost") - realm = build_realm(zerver_realm, realm_id, domain_name) + realm = build_realm(zerver_realm, realm_id, domain_name, import_source="mattermost") # We may override these later. realm["zerver_defaultstream"] = [] diff --git a/zerver/data_import/rocketchat.py b/zerver/data_import/rocketchat.py index bb15e5991b..95513f1f80 100644 --- a/zerver/data_import/rocketchat.py +++ b/zerver/data_import/rocketchat.py @@ -46,7 +46,7 @@ def make_realm( created_at = float(rc_instance["_createdAt"].timestamp()) zerver_realm = build_zerver_realm(realm_id, realm_subdomain, created_at, "Rocket.Chat") - realm = build_realm(zerver_realm, realm_id, domain_name) + realm = build_realm(zerver_realm, realm_id, domain_name, import_source="rocketchat") # We may override these later. realm["zerver_defaultstream"] = [] diff --git a/zerver/data_import/slack.py b/zerver/data_import/slack.py index 15ef702d05..61a0f48c16 100644 --- a/zerver/data_import/slack.py +++ b/zerver/data_import/slack.py @@ -172,7 +172,7 @@ def slack_workspace_to_realm( NOW = float(timezone_now().timestamp()) zerver_realm: list[ZerverFieldsT] = build_zerver_realm(realm_id, realm_subdomain, NOW, "Slack") - realm = build_realm(zerver_realm, realm_id, domain_name) + realm = build_realm(zerver_realm, realm_id, domain_name, import_source="slack") ( zerver_userprofile, diff --git a/zerver/lib/export.py b/zerver/lib/export.py index c8f0152d29..3b6259c892 100644 --- a/zerver/lib/export.py +++ b/zerver/lib/export.py @@ -435,8 +435,9 @@ def write_table_data(output_file: str, data: dict[str, Any]) -> None: # We sort by ids mostly so that humans can quickly do diffs # on two export jobs to see what changed (either due to new # data arriving or new code being deployed). - for table in data.values(): - table.sort(key=lambda row: row["id"]) + for value in data.values(): + if isinstance(value, list): + value.sort(key=lambda row: row["id"]) assert output_file.endswith(".json") @@ -2463,6 +2464,8 @@ def do_export_realm( if export_as_active is not None: response["zerver_realm"][0]["deactivated"] = not export_as_active + response["import_source"] = "zulip" # type: ignore[assignment] # this is an extra info field, not TableData + # Write realm data export_file = os.path.join(output_dir, "realm.json") write_table_data(output_file=export_file, data=response) diff --git a/zerver/lib/import_realm.py b/zerver/lib/import_realm.py index 5a72da3cc9..d4c00363b7 100644 --- a/zerver/lib/import_realm.py +++ b/zerver/lib/import_realm.py @@ -1198,6 +1198,9 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea with open(realm_data_filename, "rb") as f: data = orjson.loads(f.read()) + # Export data has an extra key with info about which app it's from. + import_source = data.pop("import_source") + # Merge in zerver_userprofile_mirrordummy data["zerver_userprofile"] += data["zerver_userprofile_mirrordummy"] del data["zerver_userprofile_mirrordummy"] @@ -1848,6 +1851,7 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea event_time=timezone_now(), extra_data={ RealmAuditLog.ROLE_COUNT: realm_user_count_by_role(realm), + "import_source": import_source, }, ) @@ -1859,9 +1863,7 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea # If the export was NOT generated by another zulip server, the # 'zulip_update_announcements_level' is set to None by default. # Set it to the latest level to avoid receiving older update messages. - is_realm_imported_from_other_zulip_server = RealmAuditLog.objects.filter( - realm=realm, event_type=AuditLogEventType.REALM_EXPORTED - ).exists() + is_realm_imported_from_other_zulip_server = import_source == "zulip" if not is_realm_imported_from_other_zulip_server: send_zulip_update_announcements_to_realm( realm, skip_delay=False, realm_imported_from_other_product=True diff --git a/zerver/tests/test_slack_importer.py b/zerver/tests/test_slack_importer.py index 4bf3ca7196..e5b6093023 100644 --- a/zerver/tests/test_slack_importer.py +++ b/zerver/tests/test_slack_importer.py @@ -1053,7 +1053,8 @@ class SlackImporter(ZulipTestCase): passed_realm["zerver_realm"][0]["description"], "Organization imported from Slack!" ) self.assertEqual(passed_realm["zerver_userpresence"], []) - self.assert_length(passed_realm.keys(), 16) + self.assertEqual(passed_realm["import_source"], "slack") + self.assert_length(passed_realm.keys(), 17) self.assertEqual(realm["zerver_stream"], []) self.assertEqual(realm["zerver_userprofile"], [])