diff --git a/api_docs/changelog.md b/api_docs/changelog.md index 4c5d5f3659..0103e5c1c4 100644 --- a/api_docs/changelog.md +++ b/api_docs/changelog.md @@ -20,6 +20,18 @@ format used by the Zulip server that they are interacting with. ## Changes in Zulip 8.0 +**Feature level 222** + +* [`GET /events`](/api/get-events): When a user is deactivated or + reactivated, the server uses `realm_user` events with `op: "update"` + updating the `is_active` field, instead of `realm_user` events with + `op: "remove"` and `op: "add"`, respectively. + +* [`GET /events`](/api/get-events): When a bot is deactivated or + reactivated, the server sends `realm_bot` events with `op: "update"` + updating the `is_active` field, instead of `realm_bot` events with + `op: "remove"` and `op: "add"`, respectively. + **Feature level 221** * [`POST /register`](/api/register-queue): Added `server_supported_permission_settings` diff --git a/version.py b/version.py index 62222385bf..4ce0f7054c 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 = 221 +API_FEATURE_LEVEL = 222 # 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/bot_data.ts b/web/src/bot_data.ts index c457577a7d..e88907b295 100644 --- a/web/src/bot_data.ts +++ b/web/src/bot_data.ts @@ -67,10 +67,6 @@ export function add(bot_data: ServerAddBotData): void { services.set(clean_bot.user_id, bot_services); } -export function deactivate(bot_id: number): void { - bots.get(bot_id)!.is_active = false; -} - export function del(bot_id: number): void { bots.delete(bot_id); services.delete(bot_id); diff --git a/web/src/server_events_dispatch.js b/web/src/server_events_dispatch.js index 8b2b71c1cd..28cf6b5738 100644 --- a/web/src/server_events_dispatch.js +++ b/web/src/server_events_dispatch.js @@ -8,7 +8,6 @@ import * as audible_notifications from "./audible_notifications"; import * as blueslip from "./blueslip"; import * as bot_data from "./bot_data"; import * as browser_history from "./browser_history"; -import {buddy_list} from "./buddy_list"; import * as compose_call from "./compose_call"; import * as compose_call_ui from "./compose_call_ui"; import * as compose_pm_pill from "./compose_pm_pill"; @@ -358,11 +357,6 @@ export function dispatch_normal_event(event) { bot_data.add(event.bot); settings_bots.render_bots(); break; - case "remove": - bot_data.deactivate(event.bot.user_id); - event.bot.is_active = false; - settings_bots.render_bots(); - break; case "delete": bot_data.del(event.bot.user_id); settings_bots.render_bots(); @@ -468,16 +462,6 @@ export function dispatch_normal_event(event) { settings_users.redraw_bots_list(); } break; - case "remove": - people.deactivate(event.person); - stream_events.remove_deactivated_user_from_all_streams(event.person.user_id); - settings_users.update_view_on_deactivate(event.person.user_id); - buddy_list.maybe_remove_key({key: event.person.user_id}); - settings_account.maybe_update_deactivate_account_button(); - if (people.user_is_bot(event.person.user_id)) { - settings_users.update_bot_data(event.person.user_id); - } - break; case "update": user_events.update_person(event.person); settings_account.maybe_update_deactivate_account_button(); diff --git a/web/src/user_events.js b/web/src/user_events.js index 5cc9c8aeab..eab38e266f 100644 --- a/web/src/user_events.js +++ b/web/src/user_events.js @@ -6,6 +6,7 @@ import $ from "jquery"; import * as activity_ui from "./activity_ui"; import * as blueslip from "./blueslip"; +import {buddy_list} from "./buddy_list"; import * as compose_state from "./compose_state"; import * as message_live_update from "./message_live_update"; import * as narrow_state from "./narrow_state"; @@ -21,6 +22,7 @@ import * as settings_profile_fields from "./settings_profile_fields"; import * as settings_realm_user_settings_defaults from "./settings_realm_user_settings_defaults"; import * as settings_streams from "./settings_streams"; import * as settings_users from "./settings_users"; +import * as stream_events from "./stream_events"; export const update_person = function update(person) { const person_obj = people.maybe_get_user_by_id(person.user_id); @@ -135,4 +137,19 @@ export const update_person = function update(person) { if (Object.hasOwn(person, "bot_owner_id")) { person_obj.bot_owner_id = person.bot_owner_id; } + + if (Object.hasOwn(person, "is_active")) { + if (person.is_active) { + people.add_active_user(person_obj); + } else { + people.deactivate(person_obj); + stream_events.remove_deactivated_user_from_all_streams(person.user_id); + settings_users.update_view_on_deactivate(person.user_id); + buddy_list.maybe_remove_key({key: person.user_id}); + } + settings_account.maybe_update_deactivate_account_button(); + if (people.user_is_bot(person.user_id)) { + settings_users.update_bot_data(person.user_id); + } + } }; diff --git a/web/tests/bot_data.test.js b/web/tests/bot_data.test.js index 0a1035f992..e6b6c03543 100644 --- a/web/tests/bot_data.test.js +++ b/web/tests/bot_data.test.js @@ -161,6 +161,12 @@ test("test_basics", () => { bot = bot_data.get(43); assert.equal(bot.owner_id, fred.user_id); + + bot_data.update(43, {...test_bot, is_active: false}); + assert.ok(!bot_data.get(43).is_active); + + bot_data.update(43, {...test_bot, is_active: true}); + assert.ok(bot_data.get(43).is_active); })(); (function test_embedded_bot_update() { @@ -176,19 +182,6 @@ test("test_basics", () => { assert.equal("embedded bot service", services[0].service_name); })(); - (function test_remove() { - let bot; - - bot_data.add({...test_bot, is_active: true}); - - bot = bot_data.get(43); - assert.equal("Bot 1", bot.full_name); - assert.ok(bot.is_active); - bot_data.deactivate(43); - bot = bot_data.get(43); - assert.equal(bot.is_active, false); - })(); - (function test_all_user_ids() { const all_ids = bot_data.all_user_ids(); all_ids.sort(); diff --git a/web/tests/dispatch.test.js b/web/tests/dispatch.test.js index 5a52e91ee1..ec11ccd050 100644 --- a/web/tests/dispatch.test.js +++ b/web/tests/dispatch.test.js @@ -67,7 +67,6 @@ const settings_user_groups_legacy = mock_esm("../src/settings_user_groups_legacy const settings_users = mock_esm("../src/settings_users"); const sidebar_ui = mock_esm("../src/sidebar_ui"); const stream_data = mock_esm("../src/stream_data"); -const stream_events = mock_esm("../src/stream_events"); const stream_list = mock_esm("../src/stream_list"); const stream_settings_ui = mock_esm("../src/stream_settings_ui"); const stream_list_sort = mock_esm("../src/stream_list_sort"); @@ -603,18 +602,6 @@ run_test("realm_bot add", ({override}) => { assert_same(args.bot, event.bot); }); -run_test("realm_bot remove", ({override}) => { - const event = event_fixtures.realm_bot__remove; - const bot_stub = make_stub(); - override(bot_data, "deactivate", bot_stub.f); - override(settings_bots, "render_bots", () => {}); - dispatch(event); - - assert.equal(bot_stub.num_calls, 1); - const args = bot_stub.get_args("user_id"); - assert_same(args.user_id, event.bot.user_id); -}); - run_test("realm_bot delete", ({override}) => { const event = event_fixtures.realm_bot__delete; const bot_stub = make_stub(); @@ -728,16 +715,6 @@ run_test("realm_user", ({override}) => { assert.ok(people.is_active_user_for_popover(event.person.user_id)); - event = event_fixtures.realm_user__remove; - override(stream_events, "remove_deactivated_user_from_all_streams", noop); - override(settings_users, "update_view_on_deactivate", noop); - dispatch(event); - - // We don't actually remove the person, we just deactivate them. - const removed_person = people.get_by_user_id(event.person.user_id); - assert.equal(removed_person.full_name, "Test User"); - assert.ok(!people.is_active_user_for_popover(event.person.user_id)); - event = event_fixtures.realm_user__update; const stub = make_stub(); override(user_events, "update_person", stub.f); @@ -753,12 +730,6 @@ run_test("realm_user", ({override}) => { dispatch({...event}); assert.equal(add_bot_stub.num_calls, 1); - const remove_bot_stub = make_stub(); - event = event_fixtures.realm_user__remove; - override(settings_users, "update_bot_data", remove_bot_stub.f); - dispatch(event); - assert.equal(remove_bot_stub.num_calls, 1); - const update_bot_stub = make_stub(); event = event_fixtures.realm_user__update; override(settings_users, "update_bot_data", update_bot_stub.f); diff --git a/web/tests/lib/events.js b/web/tests/lib/events.js index 2f53b369e3..521517768e 100644 --- a/web/tests/lib/events.js +++ b/web/tests/lib/events.js @@ -440,15 +440,6 @@ exports.fixtures = { }, }, - realm_bot__remove: { - type: "realm_bot", - op: "remove", - bot: { - user_id: 42, - full_name: "The Bot", - }, - }, - realm_bot__update: { type: "realm_bot", op: "update", @@ -568,15 +559,6 @@ exports.fixtures = { }, }, - realm_user__remove: { - type: "realm_user", - op: "remove", - person: { - full_name: test_user.full_name, - user_id: test_user.user_id, - }, - }, - realm_user__update: { type: "realm_user", op: "update", diff --git a/web/tests/user_events.test.js b/web/tests/user_events.test.js index 95dcf78bb2..27c5e808cb 100644 --- a/web/tests/user_events.test.js +++ b/web/tests/user_events.test.js @@ -10,10 +10,16 @@ const {page_params} = require("./lib/zpage_params"); const message_live_update = mock_esm("../src/message_live_update"); const settings_account = mock_esm("../src/settings_account", { + maybe_update_deactivate_account_button() {}, update_email() {}, update_full_name() {}, update_account_settings_display() {}, }); +const settings_users = mock_esm("../src/settings_users", { + update_user_data() {}, + update_view_on_deactivate() {}, +}); +const stream_events = mock_esm("../src/stream_events"); mock_esm("../src/activity_ui", { redraw() {}, @@ -42,9 +48,6 @@ mock_esm("../src/settings_realm_user_settings_defaults", { mock_esm("../src/settings_streams", { maybe_disable_widgets() {}, }); -mock_esm("../src/settings_users", { - update_user_data() {}, -}); page_params.is_admin = true; @@ -259,4 +262,30 @@ run_test("updates", () => { user_events.update_person({user_id: test_bot.user_id, bot_owner_id: me.user_id}); person = people.get_by_email(test_bot.email); assert.equal(person.bot_owner_id, me.user_id); + + let user_removed_from_streams = false; + stream_events.remove_deactivated_user_from_all_streams = (user_id) => { + assert.equal(user_id, isaac.user_id); + user_removed_from_streams = true; + }; + user_events.update_person({user_id: isaac.user_id, is_active: false}); + assert.ok(!people.is_person_active(isaac.user_id)); + assert.ok(user_removed_from_streams); + + user_events.update_person({user_id: isaac.user_id, is_active: true}); + assert.ok(people.is_person_active(isaac.user_id)); + + stream_events.remove_deactivated_user_from_all_streams = () => {}; + + let bot_data_updated = false; + settings_users.update_bot_data = (user_id) => { + assert.equal(user_id, test_bot.user_id); + bot_data_updated = true; + }; + user_events.update_person({user_id: test_bot.user_id, is_active: false}); + assert.equal(bot_data_updated, true); + + bot_data_updated = false; + user_events.update_person({user_id: test_bot.user_id, is_active: true}); + assert.ok(bot_data_updated); }); diff --git a/zerver/actions/create_user.py b/zerver/actions/create_user.py index 4c8ca644f2..a7cfde23e8 100644 --- a/zerver/actions/create_user.py +++ b/zerver/actions/create_user.py @@ -46,6 +46,7 @@ from zerver.models import ( UserGroupMembership, UserMessage, UserProfile, + active_user_ids, bot_owner_user_ids, get_system_bot, ) @@ -639,10 +640,21 @@ def do_reactivate_user(user_profile: UserProfile, *, acting_user: Optional[UserP if settings.BILLING_ENABLED: update_license_ledger_if_needed(user_profile.realm, event_time) - notify_created_user(user_profile) + event = dict( + type="realm_user", op="update", person=dict(user_id=user_profile.id, is_active=True) + ) + send_event_on_commit(user_profile.realm, event, active_user_ids(user_profile.realm_id)) if user_profile.is_bot: - notify_created_bot(user_profile) + event = dict( + type="realm_bot", + op="update", + bot=dict( + user_id=user_profile.id, + is_active=True, + ), + ) + send_event_on_commit(user_profile.realm, event, bot_owner_user_ids(user_profile)) if bot_owner_changed: from zerver.actions.bots import send_bot_owner_update_events diff --git a/zerver/actions/users.py b/zerver/actions/users.py index eea5c92a1f..3cf9964bb7 100644 --- a/zerver/actions/users.py +++ b/zerver/actions/users.py @@ -299,23 +299,23 @@ def do_deactivate_user( transaction.on_commit(lambda: delete_user_sessions(user_profile)) - event_remove_user = dict( + event_deactivate_user = dict( type="realm_user", - op="remove", - person=dict(user_id=user_profile.id, full_name=user_profile.full_name), + op="update", + person=dict(user_id=user_profile.id, is_active=False), ) send_event_on_commit( - user_profile.realm, event_remove_user, active_user_ids(user_profile.realm_id) + user_profile.realm, event_deactivate_user, active_user_ids(user_profile.realm_id) ) if user_profile.is_bot: - event_remove_bot = dict( + event_deactivate_bot = dict( type="realm_bot", - op="remove", - bot=dict(user_id=user_profile.id, full_name=user_profile.full_name), + op="update", + bot=dict(user_id=user_profile.id, is_active=False), ) send_event_on_commit( - user_profile.realm, event_remove_bot, bot_owner_user_ids(user_profile) + user_profile.realm, event_deactivate_bot, bot_owner_user_ids(user_profile) ) diff --git a/zerver/lib/event_schema.py b/zerver/lib/event_schema.py index 4d70a1dd80..2abd77a73c 100644 --- a/zerver/lib/event_schema.py +++ b/zerver/lib/event_schema.py @@ -644,15 +644,6 @@ bot_type_for_remove = DictType( ] ) -realm_bot_remove_event = event_dict_type( - required_keys=[ - ("type", Equals("realm_bot")), - ("op", Equals("remove")), - ("bot", bot_type_for_remove), - ] -) -check_realm_bot_remove = make_checker(realm_bot_remove_event) - bot_type_for_update = DictType( required_keys=[ ("user_id", int), @@ -664,6 +655,7 @@ bot_type_for_update = DictType( ("default_events_register_stream", OptionalType(str)), ("default_sending_stream", OptionalType(str)), ("full_name", str), + ("is_active", bool), ("owner_id", int), ("services", bot_services_type), ], @@ -1108,15 +1100,6 @@ removed_user_type = DictType( ] ) -realm_user_remove_event = event_dict_type( - required_keys=[ - ("type", Equals("realm_user")), - ("op", Equals("remove")), - ("person", removed_user_type), - ], -) -check_realm_user_remove = make_checker(realm_user_remove_event) - custom_profile_field_type = DictType( required_keys=[ ("id", int), @@ -1190,6 +1173,12 @@ realm_user_person_types = dict( ("timezone", str), ], ), + is_active=DictType( + required_keys=[ + ("user_id", int), + ("is_active", bool), + ], + ), ) realm_user_update_event = event_dict_type( diff --git a/zerver/lib/events.py b/zerver/lib/events.py index 3d5463d19a..63f17e48af 100644 --- a/zerver/lib/events.py +++ b/zerver/lib/events.py @@ -865,13 +865,6 @@ def apply_event( if not person["is_bot"]: person["profile_data"] = {} state["raw_users"][person_user_id] = person - elif event["op"] == "remove": - state["raw_users"][person_user_id]["is_active"] = False - if include_subscribers: - for sub in state["subscriptions"]: - sub["subscribers"] = [ - user_id for user_id in sub["subscribers"] if user_id != person_user_id - ] elif event["op"] == "update": is_me = person_user_id == user_profile.id @@ -980,16 +973,17 @@ def apply_event( if "new_email" in person: p["email"] = person["new_email"] + + if "is_active" in person and not person["is_active"] and include_subscribers: + for sub in state["subscriptions"]: + sub["subscribers"] = [ + user_id for user_id in sub["subscribers"] if user_id != person_user_id + ] else: raise AssertionError("Unexpected event type {type}/{op}".format(**event)) elif event["type"] == "realm_bot": if event["op"] == "add": state["realm_bots"].append(event["bot"]) - elif event["op"] == "remove": - user_id = event["bot"]["user_id"] - for bot in state["realm_bots"]: - if bot["user_id"] == user_id: - bot["is_active"] = False elif event["op"] == "delete": state["realm_bots"] = [ item for item in state["realm_bots"] if item["user_id"] != event["bot"]["user_id"] diff --git a/zerver/openapi/zulip.yaml b/zerver/openapi/zulip.yaml index 7a23ff3c81..2b87427269 100644 --- a/zerver/openapi/zulip.yaml +++ b/zerver/openapi/zulip.yaml @@ -594,6 +594,25 @@ paths: should update any data structures associated with this user to use this new value as the user's Zulip API email address. + - type: object + additionalProperties: false + description: | + When a user is deactivated or reactivated. + + **Changes**: New in Zulip 8.0 (feature level + 222). Previously the server sent a `realm_user` event with + `op` field set to `remove` when deactivating a user and a + `realm_user` event with `op` field set to `add` when + reactivating a user. + properties: + user_id: + type: integer + description: | + The ID of the user affected by this change. + is_active: + type: boolean + description: | + A boolean describing whether the user account has been deactivated. additionalProperties: false example: { @@ -1048,8 +1067,12 @@ paths: } - type: object description: | - Event sent to all users in a Zulip organization when - a user is deactivated. + No longer sent by modern Zulip servers. + + **Changes**: Deprecated and no longer sent since Zulip 8.0 (feature level 222). + + Previously, this event was sent to all users in a Zulip organization when a + user was deactivated. properties: id: $ref: "#/components/schemas/EventIdSchema" @@ -3715,6 +3738,11 @@ paths: additionalProperties: false description: | Event sent to all users when a bot has been deactivated. + + **Changes**: Deprecated and no longer sent since Zulip 8.0 (feature level 222). + + Previously, this event was sent to all users in a Zulip organization when a + bot was deactivated. properties: id: $ref: "#/components/schemas/EventIdSchema" @@ -17982,6 +18010,15 @@ components: owner_id: nullable: true services: {} + is_active: + type: boolean + description: | + A boolean describing whether the user account has been deactivated. + + **Changes**: New in Zulip 8.0 (feature level 222). Previously + we sent `realm_user` event with `op` field set to `remove` + when deactivating a bot and `realm_user` event with `op` + field set to `add` when reactivating a bot. BasicBotBase: type: object properties: diff --git a/zerver/tests/test_events.py b/zerver/tests/test_events.py index 4821e3ed09..7c9ea8e7fb 100644 --- a/zerver/tests/test_events.py +++ b/zerver/tests/test_events.py @@ -153,7 +153,6 @@ from zerver.lib.event_schema import ( check_reaction_remove, check_realm_bot_add, check_realm_bot_delete, - check_realm_bot_remove, check_realm_bot_update, check_realm_deactivated, check_realm_default_update, @@ -167,7 +166,6 @@ from zerver.lib.event_schema import ( check_realm_update, check_realm_update_dict, check_realm_user_add, - check_realm_user_remove, check_realm_user_update, check_scheduled_message_add, check_scheduled_message_remove, @@ -2751,14 +2749,14 @@ class NormalActionsTest(BaseAction): bot = self.create_bot("test") action = lambda: do_deactivate_user(bot, acting_user=None) events = self.verify_action(action, num_events=2) - check_realm_user_remove("events[0]", events[0]) - check_realm_bot_remove("events[1]", events[1]) + check_realm_user_update("events[0]", events[0], "is_active") + check_realm_bot_update("events[1]", events[1], "is_active") def test_do_deactivate_user(self) -> None: user_profile = self.example_user("cordelia") action = lambda: do_deactivate_user(user_profile, acting_user=None) events = self.verify_action(action, num_events=1) - check_realm_user_remove("events[0]", events[0]) + check_realm_user_update("events[0]", events[0], "is_active") def test_do_reactivate_user(self) -> None: bot = self.create_bot("test") @@ -2768,7 +2766,7 @@ class NormalActionsTest(BaseAction): do_deactivate_user(bot, acting_user=None) action = lambda: do_reactivate_user(bot, acting_user=None) events = self.verify_action(action, num_events=3) - check_realm_bot_add("events[1]", events[1]) + check_realm_bot_update("events[1]", events[1], "is_active") check_subscription_peer_add("events[2]", events[2]) # Test 'peer_add' event for private stream is received only if user is subscribed to it. @@ -2776,7 +2774,7 @@ class NormalActionsTest(BaseAction): self.subscribe(self.example_user("hamlet"), "Test private stream") action = lambda: do_reactivate_user(bot, acting_user=None) events = self.verify_action(action, num_events=4) - check_realm_bot_add("events[1]", events[1]) + check_realm_bot_update("events[1]", events[1], "is_active") check_subscription_peer_add("events[2]", events[2]) check_subscription_peer_add("events[3]", events[3]) @@ -2789,7 +2787,7 @@ class NormalActionsTest(BaseAction): self.user_profile = self.example_user("iago") action = lambda: do_reactivate_user(bot, acting_user=self.example_user("iago")) events = self.verify_action(action, num_events=7) - check_realm_bot_add("events[1]", events[1]) + check_realm_bot_update("events[1]", events[1], "is_active") check_realm_bot_update("events[2]", events[2], "owner_id") check_realm_user_update("events[3]", events[3], "bot_owner_id") check_subscription_peer_remove("events[4]", events[4])