From 614caf111e06cc4ac6ef2616d61eb544b82d630b Mon Sep 17 00:00:00 2001 From: sujal shah Date: Tue, 4 Jun 2024 16:06:52 +0530 Subject: [PATCH] user_groups: Add `creator` and `date_created` field in user groups. This commit introduced 'creator' and 'date_created' fields in user groups, allowing users to view who created the groups and when. Both fields can be null for groups without creator data. --- api_docs/changelog.md | 7 +++ version.py | 3 +- web/src/state_data.ts | 2 + web/src/user_group_edit.js | 9 +++ web/src/user_groups.ts | 2 + web/styles/subscriptions.css | 6 +- .../user_group_settings.hbs | 15 +++++ web/tests/composebox_typeahead.test.js | 6 ++ web/tests/lib/events.js | 2 + web/tests/user_groups.test.js | 4 ++ zerver/actions/user_groups.py | 8 +++ zerver/lib/event_schema.py | 2 + zerver/lib/export.py | 1 + zerver/lib/import_realm.py | 8 ++- zerver/lib/user_groups.py | 13 ++++ ...oup_creator_namedusergroup_date_created.py | 31 ++++++++++ ...usergroup_creator_date_created_backfill.py | 53 ++++++++++++++++ zerver/models/groups.py | 5 ++ zerver/openapi/zulip.yaml | 60 +++++++++++++++++++ zerver/tests/test_user_groups.py | 20 ++++++- 20 files changed, 251 insertions(+), 6 deletions(-) create mode 100644 zerver/migrations/0583_namedusergroup_creator_namedusergroup_date_created.py create mode 100644 zerver/migrations/0584_namedusergroup_creator_date_created_backfill.py diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 3f2c21e96d..dc3a63fc8d 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,13 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 10.0 +**Feature level 292** + +* [`POST /register`](/api/register-queue), [`GET + /events`](/api/get-events), [`GET + /user_groups`](/api/get-user-groups): Added `creator_id` and + `date_created` fields to user groups objects. + **Feature level 291** * `PATCH /realm`, [`GET /events`](/api/get-events), diff --git a/version.py b/version.py index 7b8837b3e9..6d6e82dafb 100644 --- a/version.py +++ b/version.py @@ -34,8 +34,7 @@ DESKTOP_WARNING_VERSION = "5.9.3" # new level means in api_docs/changelog.md, as well as "**Changes**" # entries in the endpoint's documentation in `zulip.yaml`. - -API_FEATURE_LEVEL = 291 # Last bumped for can_delete_own_message_group +API_FEATURE_LEVEL = 292 # Last bumped for `namedusergroup_creator_date_created`. # 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/web/src/state_data.ts b/web/src/state_data.ts index 50bfb697bb..e4888ae3bb 100644 --- a/web/src/state_data.ts +++ b/web/src/state_data.ts @@ -129,6 +129,8 @@ export const realm_emoji_map_schema = z.record(server_emoji_schema); export const user_group_schema = z.object({ description: z.string(), id: z.number(), + creator_id: z.number().nullable(), + date_created: z.number().nullable(), name: z.string(), members: z.array(z.number()), is_system_group: z.boolean(), diff --git a/web/src/user_group_edit.js b/web/src/user_group_edit.js index 080be4c8ab..84b68b64b0 100644 --- a/web/src/user_group_edit.js +++ b/web/src/user_group_edit.js @@ -24,6 +24,8 @@ import * as settings_components from "./settings_components"; import * as settings_data from "./settings_data"; import * as settings_org from "./settings_org"; import {current_user, realm} from "./state_data"; +import * as stream_data from "./stream_data"; +import * as timerender from "./timerender"; import * as ui_report from "./ui_report"; import * as user_group_components from "./user_group_components"; import * as user_group_create from "./user_group_create"; @@ -333,6 +335,13 @@ function update_toggler_for_group_setting() { export function show_settings_for(group) { const html = render_user_group_settings({ group, + // We get timestamp in seconds from the API but timerender needs milliseconds. + date_created_string: timerender.get_localized_date_or_time_for_format( + new Date(group.date_created * 1000), + "dayofyear_year", + ), + creator: stream_data.maybe_get_creator_details(group.creator_id), + is_creator: group.creator_id === current_user.user_id, is_member: user_groups.is_direct_member_of(people.my_current_user_id(), group.id), }); diff --git a/web/src/user_groups.ts b/web/src/user_groups.ts index 41b9b3d72c..4a11d44df2 100644 --- a/web/src/user_groups.ts +++ b/web/src/user_groups.ts @@ -45,6 +45,8 @@ export function add(user_group_raw: UserGroupRaw): void { description: user_group_raw.description, id: user_group_raw.id, name: user_group_raw.name, + creator_id: user_group_raw.creator_id, + date_created: user_group_raw.date_created, members: new Set(user_group_raw.members), is_system_group: user_group_raw.is_system_group, direct_subgroup_ids: new Set(user_group_raw.direct_subgroup_ids), diff --git a/web/styles/subscriptions.css b/web/styles/subscriptions.css index 77c119ac8e..0353c8637d 100644 --- a/web/styles/subscriptions.css +++ b/web/styles/subscriptions.css @@ -950,8 +950,10 @@ h4.user_group_setting_subsection_title { } } - .stream_details_box { - > .stream_details_subsection { + .stream_details_box, + .group_detail_box { + > .stream_details_subsection, + > .group_details_box_subsection { margin: 0 0 10px; /* mimic paragraph spacing */ } diff --git a/web/templates/user_group_settings/user_group_settings.hbs b/web/templates/user_group_settings/user_group_settings.hbs index f2486f3233..9807facdd1 100644 --- a/web/templates/user_group_settings/user_group_settings.hbs +++ b/web/templates/user_group_settings/user_group_settings.hbs @@ -43,6 +43,21 @@ {{> group_permissions can_mention_group_widget_name="can_mention_group" can_manage_group_widget_name="can_manage_group"}} + +
+
+

+ {{t "User group details" }} +

+
+
+ {{> ../creator_details }} +
+
+ {{t "User group ID"}}
+ {{group.id}} +
+
diff --git a/web/tests/composebox_typeahead.test.js b/web/tests/composebox_typeahead.test.js index 925c38abad..9e9c6d17e2 100644 --- a/web/tests/composebox_typeahead.test.js +++ b/web/tests/composebox_typeahead.test.js @@ -424,6 +424,8 @@ const harry_item = user_item(harry); const hamletcharacters = user_group_item({ name: "hamletcharacters", id: 1, + creator_id: null, + date_created: 1596710000, description: "Characters of Hamlet", members: new Set([100, 104]), is_system_group: false, @@ -435,6 +437,8 @@ const hamletcharacters = user_group_item({ const backend = user_group_item({ name: "Backend", id: 2, + creator_id: null, + date_created: 1596710000, description: "Backend team", members: new Set([101]), is_system_group: false, @@ -446,6 +450,8 @@ const backend = user_group_item({ const call_center = user_group_item({ name: "Call Center", id: 3, + creator_id: null, + date_created: 1596710000, description: "folks working in support", members: new Set([102]), is_system_group: false, diff --git a/web/tests/lib/events.js b/web/tests/lib/events.js index 0520d5e05d..c4fbd2dbe9 100644 --- a/web/tests/lib/events.js +++ b/web/tests/lib/events.js @@ -809,6 +809,8 @@ exports.fixtures = { group: { id: 555, name: "Mobile", + creator_id: null, + date_created: fake_now, description: "mobile folks", members: [1], is_system_group: false, diff --git a/web/tests/user_groups.test.js b/web/tests/user_groups.test.js index 23f02af722..739b9c6568 100644 --- a/web/tests/user_groups.test.js +++ b/web/tests/user_groups.test.js @@ -13,6 +13,8 @@ run_test("user_groups", () => { const students = { description: "Students group", name: "Students", + creator_id: null, + date_created: 1596710000, id: 0, members: new Set([1, 2]), is_system_group: false, @@ -32,6 +34,8 @@ run_test("user_groups", () => { const admins = { name: "Admins", description: "foo", + creator_id: null, + date_created: 1596710000, id: 1, members: new Set([3]), is_system_group: false, diff --git a/zerver/actions/user_groups.py b/zerver/actions/user_groups.py index b1fe606e79..484ba9a403 100644 --- a/zerver/actions/user_groups.py +++ b/zerver/actions/user_groups.py @@ -8,6 +8,7 @@ from django.utils.timezone import now as timezone_now from django.utils.translation import gettext as _ from zerver.lib.exceptions import JsonableError +from zerver.lib.timestamp import datetime_to_timestamp from zerver.lib.user_groups import ( AnonymousSettingGroupDict, get_group_setting_value_for_api, @@ -53,6 +54,7 @@ def create_user_group_in_database( description=description, is_system_group=is_system_group, realm_for_sharding=realm, + creator=acting_user, ) for setting_name, setting_value in group_settings_map.items(): @@ -170,11 +172,17 @@ def do_send_create_user_group_event( members: list[UserProfile], direct_subgroups: Sequence[UserGroup] = [], ) -> None: + creator_id = user_group.creator_id + assert user_group.date_created is not None + date_created = datetime_to_timestamp(user_group.date_created) + event = dict( type="user_group", op="add", group=dict( name=user_group.name, + creator_id=creator_id, + date_created=date_created, members=[member.id for member in members], description=user_group.description, id=user_group.id, diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index f20c678ede..4f73918c85 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -1812,6 +1812,8 @@ group_type = DictType( required_keys=[ ("id", int), ("name", str), + ("creator_id", OptionalType(int)), + ("date_created", OptionalType(int)), ("members", ListType(int)), ("direct_subgroup_ids", ListType(int)), ("description", str), diff --git a/zerver/lib/export.py b/zerver/lib/export.py index 4032747dc9..1394fa6059 100644 --- a/zerver/lib/export.py +++ b/zerver/lib/export.py @@ -301,6 +301,7 @@ DATE_FIELDS: dict[TableName, list[Field]] = { "zerver_realm": ["date_created"], "zerver_scheduledmessage": ["scheduled_timestamp"], "zerver_stream": ["date_created"], + "zerver_namedusergroup": ["date_created"], "zerver_useractivityinterval": ["start", "end"], "zerver_useractivity": ["last_visit"], "zerver_onboardingstep": ["timestamp"], diff --git a/zerver/lib/import_realm.py b/zerver/lib/import_realm.py index 5c20a62685..59476e862b 100644 --- a/zerver/lib/import_realm.py +++ b/zerver/lib/import_realm.py @@ -754,13 +754,15 @@ def bulk_import_named_user_groups(data: TableData) -> None: group["can_manage_group_id"], group["can_mention_group_id"], group["deactivated"], + group["creator_id"], + group["date_created"], ) for group in data["zerver_namedusergroup"] ] query = SQL( """ - INSERT INTO zerver_namedusergroup (usergroup_ptr_id, realm_id, name, description, is_system_group, can_manage_group_id, can_mention_group_id, deactivated) + INSERT INTO zerver_namedusergroup (usergroup_ptr_id, realm_id, name, description, is_system_group, can_manage_group_id, can_mention_group_id, deactivated, creator_id, date_created) VALUES %s """ ) @@ -1197,6 +1199,10 @@ def do_import_realm(import_dir: Path, subdomain: str, processes: int = 1) -> Rea bulk_import_model(data, UserGroup) if "zerver_namedusergroup" in data: + re_map_foreign_keys( + data, "zerver_namedusergroup", "creator", related_table="user_profile" + ) + fix_datetime_fields(data, "zerver_namedusergroup") re_map_foreign_keys( data, "zerver_namedusergroup", "usergroup_ptr", related_table="usergroup" ) diff --git a/zerver/lib/user_groups.py b/zerver/lib/user_groups.py index 8f0cf64f50..d232ff0523 100644 --- a/zerver/lib/user_groups.py +++ b/zerver/lib/user_groups.py @@ -17,6 +17,7 @@ from zerver.lib.exceptions import ( PreviousSettingValueMismatchedError, SystemGroupRequiredError, ) +from zerver.lib.timestamp import datetime_to_timestamp from zerver.lib.types import GroupPermissionSetting, ServerSupportedPermissionSettings from zerver.models import ( GroupGroupMembership, @@ -50,6 +51,8 @@ class UserGroupDict(TypedDict): description: str members: list[int] direct_subgroup_ids: list[int] + creator_id: int | None + date_created: int | None is_system_group: bool can_manage_group: int | AnonymousSettingGroupDict can_mention_group: int | AnonymousSettingGroupDict @@ -516,9 +519,19 @@ def user_groups_in_realm_serialized( if user_group.id in group_subgroups: direct_subgroup_ids = group_subgroups[user_group.id] + creator_id = user_group.creator_id + + date_created = ( + datetime_to_timestamp(user_group.date_created) + if user_group.date_created is not None + else None + ) + group_dicts[user_group.id] = dict( id=user_group.id, name=user_group.name, + creator_id=creator_id, + date_created=date_created, description=user_group.description, members=direct_member_ids, direct_subgroup_ids=direct_subgroup_ids, diff --git a/zerver/migrations/0583_namedusergroup_creator_namedusergroup_date_created.py b/zerver/migrations/0583_namedusergroup_creator_namedusergroup_date_created.py new file mode 100644 index 0000000000..a46a82de41 --- /dev/null +++ b/zerver/migrations/0583_namedusergroup_creator_namedusergroup_date_created.py @@ -0,0 +1,31 @@ +# Generated by Django 5.0.8 on 2024-08-31 08:09 + +import django.db.models.deletion +import django.utils.timezone +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0582_remove_realm_delete_own_message_policy"), + ] + + operations = [ + migrations.AddField( + model_name="namedusergroup", + name="creator", + field=models.ForeignKey( + db_column="creator_id", + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="+", + to=settings.AUTH_USER_MODEL, + ), + ), + migrations.AddField( + model_name="namedusergroup", + name="date_created", + field=models.DateTimeField(default=django.utils.timezone.now, null=True), + ), + ] diff --git a/zerver/migrations/0584_namedusergroup_creator_date_created_backfill.py b/zerver/migrations/0584_namedusergroup_creator_date_created_backfill.py new file mode 100644 index 0000000000..d6f9d30dd1 --- /dev/null +++ b/zerver/migrations/0584_namedusergroup_creator_date_created_backfill.py @@ -0,0 +1,53 @@ +# Generated by Django 5.0.8 on 2024-08-31 08:09 + +from django.db import migrations +from django.db.backends.base.schema import BaseDatabaseSchemaEditor +from django.db.migrations.state import StateApps + + +def backfill_creator_id_and_date_created_from_realm_audit_log( + apps: StateApps, schema_editor: BaseDatabaseSchemaEditor +) -> None: + RealmAuditLog = apps.get_model("zerver", "RealmAuditLog") + RealmAuditLog.USER_GROUP_CREATED = 701 + NamedUserGroup = apps.get_model("zerver", "NamedUserGroup") + + user_group_creator_updates = [] + for audit_log_entry in RealmAuditLog.objects.select_related("modified_user_group").filter( + event_type=RealmAuditLog.USER_GROUP_CREATED, + acting_user_id__isnull=False, + ): + assert audit_log_entry.modified_user_group is not None + user_group = audit_log_entry.modified_user_group + user_group.creator_id = audit_log_entry.acting_user_id + user_group_creator_updates.append(user_group) + + NamedUserGroup.objects.bulk_update(user_group_creator_updates, ["creator_id"], batch_size=1000) + + user_group_date_created_updates = [] + for audit_log_entry in RealmAuditLog.objects.select_related("modified_user_group").filter( + event_type=RealmAuditLog.USER_GROUP_CREATED, + event_time__isnull=False, + ): + assert audit_log_entry.modified_user_group is not None + user_group = audit_log_entry.modified_user_group + user_group.date_created = audit_log_entry.event_time + user_group_date_created_updates.append(user_group) + + NamedUserGroup.objects.bulk_update( + user_group_date_created_updates, ["date_created"], batch_size=1000 + ) + + +class Migration(migrations.Migration): + dependencies = [ + ("zerver", "0583_namedusergroup_creator_namedusergroup_date_created"), + ] + + operations = [ + migrations.RunPython( + backfill_creator_id_and_date_created_from_realm_audit_log, + reverse_code=migrations.RunPython.noop, + elidable=True, + ), + ] diff --git a/zerver/models/groups.py b/zerver/models/groups.py index 9decab12b2..c785c9412e 100644 --- a/zerver/models/groups.py +++ b/zerver/models/groups.py @@ -1,5 +1,6 @@ from django.db import models from django.db.models import CASCADE +from django.utils.timezone import now as timezone_now from django_cte import CTEManager from zerver.lib.types import GroupPermissionSetting @@ -52,6 +53,10 @@ class NamedUserGroup(UserGroup): # type: ignore[django-manager-missing] # djang ) name = models.CharField(max_length=MAX_NAME_LENGTH, db_column="name") description = models.TextField(default="", db_column="description") + date_created = models.DateTimeField(default=timezone_now, null=True) + creator = models.ForeignKey( + UserProfile, null=True, on_delete=models.SET_NULL, related_name="+", db_column="creator_id" + ) is_system_group = models.BooleanField(default=False, db_column="is_system_group") can_manage_group = models.ForeignKey(UserGroup, on_delete=models.RESTRICT, related_name="+") diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 00263b44d1..1f45e168b6 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -3148,6 +3148,8 @@ paths: { "name": "backend", "members": [12], + "creator_id": 9, + "date_created": 1717484476, "description": "Backend team", "id": 2, "is_system_group": false, @@ -20026,6 +20028,34 @@ paths: type: integer description: | The user group's integer ID. + date_created: + type: integer + nullable: true + description: | + The UNIX timestamp for when the user group was created, in UTC seconds. + + A `null` value means the user group has no recorded date, which is often + because the group predates the metadata being tracked starting in Zulip 8.0, + or because it was created via a data import tool + or [management command][management-commands]. + + **Changes**: New in Zulip 10.0 (feature level 292). + + [management-commands]: https://zulip.readthedocs.io/en/latest/production/management-commands.html + creator_id: + type: integer + nullable: true + description: | + The ID of the user who created this user group. + + A `null` value means the user group has no recorded creator, which is often + because the group predates the metadata being tracked starting in Zulip 8.0, + or because it was created via a data import tool + or [management command][management-commands]. + + **Changes**: New in Zulip 10.0 (feature level 292). + + [management-commands]: https://zulip.readthedocs.io/en/latest/production/management-commands.html members: type: array description: | @@ -20107,6 +20137,8 @@ paths: { "description": "Owners of this organization", "id": 1, + "creator_id": null, + "date_created": null, "name": "role:owners", "members": [1], "direct_subgroup_ids": [], @@ -20117,6 +20149,8 @@ paths: { "description": "Administrators of this organization, including owners", "id": 2, + "creator_id": null, + "date_created": null, "name": "role:administrators", "members": [2], "direct_subgroup_ids": [1], @@ -20127,6 +20161,8 @@ paths: { "description": "Characters of Hamlet", "id": 3, + "creator_id": null, + "date_created": 1717484476, "name": "hamletcharacters", "members": [3, 4], "direct_subgroup_ids": [], @@ -21269,6 +21305,30 @@ components: type: string description: | The name of the user group. + date_created: + type: integer + nullable: true + description: | + The UNIX timestamp for when the user group was created, in UTC seconds. + + A `null` value means the user group has no recorded date, which is often + because the user group is very old, or because it was created via a data + import tool or [management command][management-commands]. + + **Changes**: New in Zulip 10.0 (feature level 292). + [management-commands]: https://zulip.readthedocs.io/en/latest/production/management-commands.html + creator_id: + type: integer + nullable: true + description: | + The ID of the user who created this user group. + + A `null` value means the user group has no recorded creator, which is often + because the user group is very old, or because it was created via a data + import tool or [management command][management-commands]. + + **Changes**: New in Zulip 10.0 (feature level 292). + [management-commands]: https://zulip.readthedocs.io/en/latest/production/management-commands.html description: type: string description: | diff --git a/zerver/tests/test_user_groups.py b/zerver/tests/test_user_groups.py index 8dac697d7d..09f8d181bc 100644 --- a/zerver/tests/test_user_groups.py +++ b/zerver/tests/test_user_groups.py @@ -33,6 +33,7 @@ from zerver.lib.mention import silent_mention_syntax_for_user from zerver.lib.streams import ensure_stream from zerver.lib.test_classes import ZulipTestCase from zerver.lib.test_helpers import most_recent_usermessage +from zerver.lib.timestamp import datetime_to_timestamp from zerver.lib.user_groups import ( AnonymousSettingGroupDict, get_direct_user_groups, @@ -80,13 +81,16 @@ class UserGroupTestCase(ZulipTestCase): def test_user_groups_in_realm_serialized(self) -> None: realm = get_realm("zulip") + user = self.example_user("iago") user_group = NamedUserGroup.objects.filter(realm=realm).first() assert user_group is not None - empty_user_group = check_add_user_group(realm, "newgroup", [], acting_user=None) + empty_user_group = check_add_user_group(realm, "newgroup", [], acting_user=user) user_groups = user_groups_in_realm_serialized(realm, allow_deactivated=False) self.assert_length(user_groups, 10) self.assertEqual(user_groups[0]["id"], user_group.id) + self.assertEqual(user_groups[0]["creator_id"], user_group.creator_id) + self.assertEqual(user_groups[0]["date_created"], user_group.date_created) self.assertEqual(user_groups[0]["name"], SystemGroups.NOBODY) self.assertEqual(user_groups[0]["description"], "Nobody") self.assertEqual(user_groups[0]["members"], []) @@ -100,6 +104,8 @@ class UserGroupTestCase(ZulipTestCase): "user_profile_id", flat=True ) self.assertEqual(user_groups[1]["id"], owners_system_group.id) + self.assertEqual(user_groups[1]["creator_id"], owners_system_group.creator_id) + self.assertEqual(user_groups[1]["date_created"], owners_system_group.date_created) self.assertEqual(user_groups[1]["name"], SystemGroups.OWNERS) self.assertEqual(user_groups[1]["description"], "Owners of this organization") self.assertEqual(set(user_groups[1]["members"]), set(membership)) @@ -119,6 +125,11 @@ class UserGroupTestCase(ZulipTestCase): name=SystemGroups.EVERYONE, realm=realm, is_system_group=True ) self.assertEqual(user_groups[9]["id"], empty_user_group.id) + self.assertEqual(user_groups[9]["creator_id"], empty_user_group.creator_id) + assert empty_user_group.date_created is not None + self.assertEqual( + user_groups[9]["date_created"], datetime_to_timestamp(empty_user_group.date_created) + ) self.assertEqual(user_groups[9]["name"], "newgroup") self.assertEqual(user_groups[9]["description"], "") self.assertEqual(user_groups[9]["members"], []) @@ -143,6 +154,13 @@ class UserGroupTestCase(ZulipTestCase): ) user_groups = user_groups_in_realm_serialized(realm, allow_deactivated=False) self.assertEqual(user_groups[10]["id"], new_user_group.id) + self.assertEqual(user_groups[10]["creator_id"], new_user_group.creator_id) + new_user_group_date_created = ( + datetime_to_timestamp(new_user_group.date_created) + if new_user_group.date_created is not None + else None + ) + self.assertEqual(user_groups[10]["date_created"], new_user_group_date_created) self.assertEqual(user_groups[10]["name"], "newgroup2") self.assertEqual(user_groups[10]["description"], "") self.assertEqual(user_groups[10]["members"], [othello.id])