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.
This commit is contained in:
tnmkr 2024-03-23 00:07:19 +05:30 committed by Tim Abbott
parent 9fa7f71125
commit 5128d8f9af
12 changed files with 181 additions and 1 deletions

View File

@ -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),

View File

@ -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

View File

@ -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"],

View File

@ -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)),

View File

@ -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

View File

@ -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,

View File

@ -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,

View File

@ -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]

View File

@ -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,
),
]

View File

@ -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,

View File

@ -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": "<p>Located in the United Kingdom</p>",
"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": "<p>A Scandinavian country</p>",
"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": "<p>A city in Italy</p>",
"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": "<p>Located in the United Kingdom</p>",
"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: |

View File

@ -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()