From 5128d8f9af2ac7315485326afa2e6416e2bdd050 Mon Sep 17 00:00:00 2001 From: tnmkr Date: Sat, 23 Mar 2024 00:07:19 +0530 Subject: [PATCH] streams: Add creator field. Adds nullable creator field, containing a reference to the user who created the stream. When creating a stream, acting user is set as the creator of the stream. Since API calls to create streams always have an acting user, this field should always be set when streams are created using the API. Because streams can be created with no acting user, this field is nullable. We try to backfill existing streams using RealmAuditLog table, but not all streams are guaranteed to have a recorded create log. Thus this new field is left null when it cannot be backfilled. We also set this field to null when the creator user is deleted. --- api_docs/changelog.md | 9 ++++ version.py | 2 +- zerver/actions/streams.py | 1 + zerver/lib/event_schema.py | 1 + zerver/lib/import_realm.py | 1 + zerver/lib/streams.py | 2 + zerver/lib/subscription_info.py | 7 ++++ zerver/lib/types.py | 4 ++ zerver/migrations/0511_stream_creator.py | 50 ++++++++++++++++++++++ zerver/models/streams.py | 3 ++ zerver/openapi/zulip.yaml | 49 ++++++++++++++++++++++ zerver/tests/test_subs.py | 53 ++++++++++++++++++++++++ 12 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 zerver/migrations/0511_stream_creator.py diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 957f4fef77..65669f670b 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,15 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 9.0 +**Feature level 254** + +* [`POST /register`](/api/register-queue), [`GET /events`](/api/get-events), + [`GET /streams`](/api/get-streams), + [`GET /streams/{stream_id}`](/api/get-stream-by-id), + [`GET /users/me/subscriptions`](/api/get-subscriptions): Added a new + field `creator_id`, on stream and subscription objects, containing the + user id of the stream creator. + **Feature level 253** * [`PATCH /realm/user_settings_defaults`](/api/update-realm-user-settings-defaults), diff --git a/version.py b/version.py index 48bf1d68d1..8117a08f8a 100644 --- a/version.py +++ b/version.py @@ -33,7 +33,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # Changes should be accompanied by documentation explaining what the # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. -API_FEATURE_LEVEL = 253 +API_FEATURE_LEVEL = 254 # Bump the minor PROVISION_VERSION to indicate that folks should provision # only when going from an old version of the code to a newer version. Bump diff --git a/zerver/actions/streams.py b/zerver/actions/streams.py index d202e753d1..87672a951d 100644 --- a/zerver/actions/streams.py +++ b/zerver/actions/streams.py @@ -450,6 +450,7 @@ def send_subscription_add_events( subscribers=stream_subscribers, # Fields from Stream.API_FIELDS can_remove_subscribers_group=stream_dict["can_remove_subscribers_group"], + creator_id=stream_dict["creator_id"], date_created=stream_dict["date_created"], description=stream_dict["description"], first_message_id=stream_dict["first_message_id"], diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index e7ea9932e1..0aa5c8b4b5 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -51,6 +51,7 @@ from zerver.models import Realm, RealmUserDefault, Stream, UserProfile # larger "subscription" events that also contain personal settings. default_stream_fields = [ ("can_remove_subscribers_group", int), + ("creator_id", OptionalType(int)), ("date_created", int), ("description", str), ("first_message_id", OptionalType(int)), diff --git a/zerver/lib/import_realm.py b/zerver/lib/import_realm.py index eafe0ee67b..699282007f 100644 --- a/zerver/lib/import_realm.py +++ b/zerver/lib/import_realm.py @@ -1048,6 +1048,7 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea # Stream objects are created by Django. fix_datetime_fields(data, "zerver_stream") re_map_foreign_keys(data, "zerver_stream", "realm", related_table="realm") + re_map_foreign_keys(data, "zerver_stream", "creator", related_table="user_profile") if role_system_groups_dict is not None: # Because the system user groups are missing, we manually set up # the defaults for can_remove_subscribers_group for all the diff --git a/zerver/lib/streams.py b/zerver/lib/streams.py index dad6ea24e4..3eebfdbc2a 100644 --- a/zerver/lib/streams.py +++ b/zerver/lib/streams.py @@ -153,6 +153,7 @@ def create_stream_if_needed( name__iexact=stream_name, defaults=dict( name=stream_name, + creator=acting_user, description=stream_description, invite_only=invite_only, is_web_public=is_web_public, @@ -861,6 +862,7 @@ def stream_to_dict( return APIStreamDict( can_remove_subscribers_group=stream.can_remove_subscribers_group_id, + creator_id=stream.creator_id, date_created=datetime_to_timestamp(stream.date_created), description=stream.description, first_message_id=stream.first_message_id, diff --git a/zerver/lib/subscription_info.py b/zerver/lib/subscription_info.py index 717366b872..e1abad74da 100644 --- a/zerver/lib/subscription_info.py +++ b/zerver/lib/subscription_info.py @@ -42,6 +42,7 @@ def get_web_public_subs(realm: Realm) -> SubscriptionInfo: for stream in get_web_public_streams_queryset(realm): # Add Stream fields. can_remove_subscribers_group_id = stream.can_remove_subscribers_group_id + creator_id = stream.creator_id date_created = datetime_to_timestamp(stream.date_created) description = stream.description first_message_id = stream.first_message_id @@ -74,6 +75,7 @@ def get_web_public_subs(realm: Realm) -> SubscriptionInfo: audible_notifications=audible_notifications, can_remove_subscribers_group=can_remove_subscribers_group_id, color=color, + creator_id=creator_id, date_created=date_created, description=description, desktop_notifications=desktop_notifications, @@ -110,6 +112,7 @@ def build_unsubscribed_sub_from_stream_dict( # This function is only called from `apply_event` code. raw_stream_dict = RawStreamDict( can_remove_subscribers_group_id=stream_dict["can_remove_subscribers_group"], + creator_id=stream_dict["creator_id"], date_created=timestamp_to_datetime(stream_dict["date_created"]), description=stream_dict["description"], first_message_id=stream_dict["first_message_id"], @@ -141,6 +144,7 @@ def build_stream_dict_for_sub( ) -> SubscriptionStreamDict: # Handle Stream.API_FIELDS can_remove_subscribers_group_id = raw_stream_dict["can_remove_subscribers_group_id"] + creator_id = raw_stream_dict["creator_id"] date_created = datetime_to_timestamp(raw_stream_dict["date_created"]) description = raw_stream_dict["description"] first_message_id = raw_stream_dict["first_message_id"] @@ -185,6 +189,7 @@ def build_stream_dict_for_sub( audible_notifications=audible_notifications, can_remove_subscribers_group=can_remove_subscribers_group_id, color=color, + creator_id=creator_id, date_created=date_created, description=description, desktop_notifications=desktop_notifications, @@ -213,6 +218,7 @@ def build_stream_dict_for_never_sub( recent_traffic: Optional[Dict[int, int]], ) -> NeverSubscribedStreamDict: can_remove_subscribers_group_id = raw_stream_dict["can_remove_subscribers_group_id"] + creator_id = raw_stream_dict["creator_id"] date_created = datetime_to_timestamp(raw_stream_dict["date_created"]) description = raw_stream_dict["description"] first_message_id = raw_stream_dict["first_message_id"] @@ -238,6 +244,7 @@ def build_stream_dict_for_never_sub( # Our caller may add a subscribers field. return NeverSubscribedStreamDict( can_remove_subscribers_group=can_remove_subscribers_group_id, + creator_id=creator_id, date_created=date_created, description=description, first_message_id=first_message_id, diff --git a/zerver/lib/types.py b/zerver/lib/types.py index 8d46235134..4ab61b820f 100644 --- a/zerver/lib/types.py +++ b/zerver/lib/types.py @@ -139,6 +139,7 @@ class RawStreamDict(TypedDict): """ can_remove_subscribers_group_id: int + creator_id: Optional[int] date_created: datetime description: str first_message_id: Optional[int] @@ -179,6 +180,7 @@ class SubscriptionStreamDict(TypedDict): audible_notifications: Optional[bool] can_remove_subscribers_group: int color: str + creator_id: Optional[int] date_created: int description: str desktop_notifications: Optional[bool] @@ -204,6 +206,7 @@ class SubscriptionStreamDict(TypedDict): class NeverSubscribedStreamDict(TypedDict): can_remove_subscribers_group: int + creator_id: Optional[int] date_created: int description: str first_message_id: Optional[int] @@ -227,6 +230,7 @@ class DefaultStreamDict(TypedDict): """ can_remove_subscribers_group: int + creator_id: Optional[int] date_created: int description: str first_message_id: Optional[int] diff --git a/zerver/migrations/0511_stream_creator.py b/zerver/migrations/0511_stream_creator.py new file mode 100644 index 0000000000..8f25667034 --- /dev/null +++ b/zerver/migrations/0511_stream_creator.py @@ -0,0 +1,50 @@ +# Generated by Django 4.2.11 on 2024-04-05 06:09 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +from django.db.migrations.state import StateApps + + +def backfill_creator_id_from_realm_audit_log( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + RealmAuditLog = apps.get_model("zerver", "RealmAuditLog") + RealmAuditLog.STREAM_CREATED = 601 + Stream = apps.get_model("zerver", "Stream") + + stream_updates = [] + for audit_log_entry in RealmAuditLog.objects.select_related("modified_stream").filter( + event_type=RealmAuditLog.STREAM_CREATED, + acting_user_id__isnull=False, + ): + assert audit_log_entry.modified_stream is not None + stream = audit_log_entry.modified_stream + stream.creator_id = audit_log_entry.acting_user_id + stream_updates.append(stream) + + Stream.objects.bulk_update(stream_updates, ["creator_id"], batch_size=1000) + + +class Migration(migrations.Migration): + atomic = False + + dependencies = [ + ("zerver", "0510_add_realmauditlog_realm_event_type_index"), + ] + + operations = [ + migrations.AddField( + model_name="stream", + name="creator", + field=models.ForeignKey( + null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL + ), + ), + migrations.RunPython( + backfill_creator_id_from_realm_audit_log, + reverse_code=migrations.RunPython.noop, + elidable=True, + ), + ] diff --git a/zerver/models/streams.py b/zerver/models/streams.py index edf5bc8115..c782190213 100644 --- a/zerver/models/streams.py +++ b/zerver/models/streams.py @@ -30,6 +30,7 @@ class Stream(models.Model): name = models.CharField(max_length=MAX_NAME_LENGTH, db_index=True) realm = models.ForeignKey(Realm, db_index=True, on_delete=CASCADE) date_created = models.DateTimeField(default=timezone_now) + creator = models.ForeignKey(UserProfile, null=True, on_delete=models.SET_NULL) deactivated = models.BooleanField(default=False) description = models.CharField(max_length=MAX_DESCRIPTION_LENGTH, default="") rendered_description = models.TextField(default="") @@ -179,6 +180,7 @@ class Stream(models.Model): # * "deactivated" streams are filtered from the API entirely. # * "realm" and "recipient" are not exposed to clients via the API. API_FIELDS = [ + "creator_id", "date_created", "description", "first_message_id", @@ -196,6 +198,7 @@ class Stream(models.Model): def to_dict(self) -> DefaultStreamDict: return DefaultStreamDict( can_remove_subscribers_group=self.can_remove_subscribers_group_id, + creator_id=self.creator_id, date_created=datetime_to_timestamp(self.date_created), description=self.description, first_message_id=self.first_message_id, diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 6be158cff3..b4ac3ff83e 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -680,6 +680,7 @@ paths: { "name": "test_stream", "stream_id": 9, + "creator_id": null, "description": "", "rendered_description": "", "invite_only": false, @@ -1325,6 +1326,7 @@ paths: "description": "", "rendered_description": "", "date_created": 1691057093, + "creator_id": 11, "invite_only": true, "is_web_public": false, "stream_post_policy": 1, @@ -1382,6 +1384,7 @@ paths: "description": "", "rendered_description": "", "date_created": 1691057093, + "creator_id": 11, "invite_only": true, "is_web_public": false, "stream_post_policy": 1, @@ -1958,6 +1961,7 @@ paths: "description": "Located in the United Kingdom", "rendered_description": "

Located in the United Kingdom

", "date_created": 1691057093, + "creator_id": 8, "invite_only": false, "is_web_public": false, "stream_post_policy": 1, @@ -1973,6 +1977,7 @@ paths: "description": "A Scandinavian country", "rendered_description": "

A Scandinavian country

", "date_created": 1691057093, + "creator_id": null, "invite_only": false, "is_web_public": false, "stream_post_policy": 1, @@ -1988,6 +1993,7 @@ paths: "description": "A city in Italy", "rendered_description": "

A city in Italy

", "date_created": 1691057093, + "creator_id": 9, "invite_only": false, "is_web_public": false, "stream_post_policy": 1, @@ -2034,6 +2040,7 @@ paths: "description": "Located in the United Kingdom", "rendered_description": "

Located in the United Kingdom

", "date_created": 1691057093, + "creator_id": 8, "invite_only": false, "is_web_public": false, "stream_post_policy": 1, @@ -9186,6 +9193,7 @@ paths: { "audible_notifications": true, "color": "#e79ab5", + "creator_id": null, "description": "A Scandinavian country", "desktop_notifications": true, "is_muted": false, @@ -9199,6 +9207,7 @@ paths: { "audible_notifications": true, "color": "#e79ab5", + "creator_id": 8, "description": "Located in the United Kingdom", "desktop_notifications": true, "is_muted": false, @@ -13075,6 +13084,8 @@ paths: name: {} description: {} date_created: {} + creator_id: + nullable: true invite_only: {} rendered_description: {} is_web_public: {} @@ -17636,6 +17647,8 @@ paths: name: {} description: {} date_created: {} + creator_id: + nullable: true invite_only: {} rendered_description: {} is_web_public: {} @@ -17677,6 +17690,7 @@ paths: - name - description - date_created + - creator_id - invite_only - rendered_description - is_web_public @@ -17695,6 +17709,7 @@ paths: [ { "can_remove_subscribers_group": 10, + "creator_id": null, "date_created": 1691057093, "description": "A private stream", "first_message_id": 18, @@ -17712,6 +17727,7 @@ paths: }, { "can_remove_subscribers_group": 9, + "creator_id": 12, "date_created": 1691057093, "description": "A default public stream", "first_message_id": 21, @@ -17782,6 +17798,7 @@ paths: "first_message_id": 1, "history_public_to_subscribers": true, "date_created": 1691057093, + "creator_id": null, "invite_only": false, "is_announcement_only": false, "is_web_public": false, @@ -19106,6 +19123,8 @@ components: name: {} description: {} date_created: {} + creator_id: + nullable: true invite_only: {} rendered_description: {} is_web_public: {} @@ -19137,6 +19156,7 @@ components: - name - description - date_created + - creator_id - invite_only - rendered_description - is_web_public @@ -19156,6 +19176,8 @@ components: name: {} description: {} date_created: {} + creator_id: + nullable: true invite_only: {} rendered_description: {} is_web_public: {} @@ -19172,6 +19194,7 @@ components: - name - description - date_created + - creator_id - invite_only - rendered_description - is_web_public @@ -19206,6 +19229,19 @@ components: The UNIX timestamp for when the stream was created, in UTC seconds. **Changes**: New in Zulip 4.0 (feature level 30). + creator_id: + type: integer + nullable: true + description: | + The ID of the user who created this stream. + + Is `null` for streams with no recorded creator, often because they are very + old, or were created via a data import tool or [management + command][management-commands]. + + **Changes**: New in Zulip 9.0 (feature level 254). + + [management-commands]: https://zulip.readthedocs.io/en/latest/production/management-commands.html invite_only: type: boolean description: | @@ -19932,6 +19968,19 @@ components: The UNIX timestamp for when the stream was created, in UTC seconds. **Changes**: New in Zulip 4.0 (feature level 30). + creator_id: + type: integer + nullable: true + description: | + The ID of the user who created this stream. + + Is `null` for streams with no recorded creator, often because they are very + old, or were created via a data import tool or [management + command][management-commands]. + + **Changes**: New in Zulip 9.0 (feature level 254). + + [management-commands]: https://zulip.readthedocs.io/en/latest/production/management-commands.html invite_only: type: boolean description: | diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index 36f1621dc5..fc36a7b775 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -307,6 +307,8 @@ class TestCreateStreams(ZulipTestCase): self.assertTrue(stream.stream_post_policy == Stream.STREAM_POST_POLICY_ADMINS) self.assertTrue(stream.message_retention_days == -1) self.assertEqual(stream.can_remove_subscribers_group.id, moderators_system_group.id) + # Streams created where acting_user is None have no creator + self.assertIsNone(stream.creator_id) new_streams, existing_streams = create_streams_if_needed( realm, @@ -605,6 +607,28 @@ class TestCreateStreams(ZulipTestCase): "'can_remove_subscribers_group' setting cannot be set to 'role:nobody' group.", ) + def test_acting_user_is_creator(self) -> None: + """ + If backend calls provide an acting_user while trying to + create streams, assign acting_user as the stream creator + """ + hamlet = self.example_user("hamlet") + new_streams, _ = create_streams_if_needed( + hamlet.realm, + [ + StreamDict( + name="hamlet's test stream", + description="No description", + invite_only=True, + is_web_public=True, + stream_post_policy=Stream.STREAM_POST_POLICY_ADMINS, + ) + ], + acting_user=hamlet, + ) + created_stream = new_streams[0] + self.assertEqual(created_stream.creator_id, hamlet.id) + class RecipientTest(ZulipTestCase): def test_recipient(self) -> None: @@ -4413,6 +4437,35 @@ class SubscriptionAPITest(ZulipTestCase): "create_web_public_stream_policy", invite_only=False, is_web_public=True ) + def test_stream_creator_id(self) -> None: + iago = self.example_user("iago") + self.login_user(iago) + user1 = self.example_user("hamlet") + user2 = self.example_user("desdemona") + + streams_to_sub = ["new_stream"] + # We create streams by subscribing users to non-existent streams + # Here we subscribe users other than the stream creator + with self.capture_send_event_calls(5) as events: + self.common_subscribe_to_streams( + iago, + streams_to_sub, + dict(principals=orjson.dumps([user1.id, user2.id]).decode()), + ) + self.assertEqual(events[0]["event"]["streams"][0]["creator_id"], iago.id) + created_stream_id = events[0]["event"]["streams"][0]["stream_id"] + + all_streams = self.api_get(iago, "/api/v1/streams") + json = self.assert_json_success(all_streams) + for stream in json["streams"]: + if stream["stream_id"] == created_stream_id: + # Acting user should be the creator for api created streams + self.assertEqual(stream["creator_id"], iago.id) + continue + + # Streams that aren't created using the api should have no creator + self.assertIsNone(stream["creator_id"]) + def test_private_stream_policies(self) -> None: def validation_func(user_profile: UserProfile) -> bool: return user_profile.can_create_private_streams()