paulmataruso/zulip
1
0
Fork 0
mirror of https://github.com/zulip/zulip.git synced 2025-10-23 04:52:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

streams: Live update UI correctly when archiving streams.

Browse Source 
Previously, "delete" event was sent for both archiving streams
and when user lost access to a stream. So, when user lost
access to a stream, the UI was live updated like the stream
was archived, which was not correct. But now "update" event
is sent when a stream is archived, so the webapp code is
changed accordingly to live-update the UI for both cases
correctly.

As a result, some of the changes in 43932cd6aa and a29b6485d6
are reverted as "update" event is sent when archiving and "delete"
event is sent only when a user loses access to a stream as before.
This commit is contained in:
Sahil Batra
2025-04-01 12:13:16 +05:30
committed by Tim Abbott
parent 7c470f0161
commit 9d15a11331
8 changed files with 159 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
web/src/server_events_dispatch.js
View File

@@ -30,7 +30,6 @@ import * as message_events from "./message_events.ts";
import * as message_lists from "./message_lists.ts";
import * as message_live_update from "./message_live_update.ts";
import * as message_view from "./message_view.ts";
import * as message_view_header from "./message_view_header.ts";
import * as muted_users_ui from "./muted_users_ui.ts";
import * as narrow_state from "./narrow_state.ts";
import * as narrow_title from "./narrow_title.ts";
@@ -647,13 +646,11 @@ export function dispatch_normal_event(event) {
break;
case "delete":
for (const stream_id of event.stream_ids) {
const sub = sub_store.get(stream_id);
const is_subscribed = sub.subscribed;
const was_subscribed = sub_store.get(stream_id).subscribed;
const is_narrowed_to_stream = narrow_state.narrowed_to_stream_id(stream_id);
stream_data.delete_sub(stream_id);
stream_settings_ui.update_settings_for_archived(sub);
message_view_header.maybe_rerender_title_area_for_stream(stream_id);
if (is_subscribed) {
stream_settings_ui.remove_stream(stream_id);
if (was_subscribed) {
stream_list.remove_sidebar_row(stream_id);
if (stream_id === compose_state.selected_recipient_id) {
compose_state.set_selected_recipient_id("");
@@ -663,15 +660,12 @@ export function dispatch_normal_event(event) {
settings_streams.update_default_streams_table();
stream_data.remove_default_stream(stream_id);
if (realm.realm_new_stream_announcements_stream_id === stream_id) {
realm.realm_new_stream_announcements_stream_id = -1;
settings_org.sync_realm_settings("new_stream_announcements_stream_id");
}
if (realm.realm_signup_announcements_stream_id === stream_id) {
realm.realm_signup_announcements_stream_id = -1;
settings_org.sync_realm_settings("signup_announcements_stream_id");
}
if (realm.realm_zulip_update_announcements_stream_id === stream_id) {
realm.realm_zulip_update_announcements_stream_id = -1;
settings_org.sync_realm_settings(
"zulip_update_announcements_stream_id",
);
@@ -681,7 +675,6 @@ export function dispatch_normal_event(event) {
message_lists.current.update_trailing_bookend(true);
}
}
message_live_update.rerender_messages_view();
stream_list.update_subscribe_to_more_streams_link();
break;
default:

17
web/src/stream_data.ts
View File

@@ -110,6 +110,11 @@ class BinaryDict<T> {
this.trues.delete(k);
this.falses.set(k, v);
}
delete(k: number): void {
this.trues.delete(k);
this.falses.delete(k);
}
}
// The stream_info variable maps stream ids to stream properties objects
@@ -303,7 +308,7 @@ export function slug_to_stream_id(slug: string): number | undefined {
return undefined;
}
export function delete_sub(stream_id: number): void {
export function mark_archived(stream_id: number): void {
const sub = get_sub_by_id(stream_id);
if (sub === undefined || !stream_info.get(stream_id)) {
blueslip.warn("Failed to archive stream " + stream_id.toString());
@@ -312,6 +317,16 @@ export function delete_sub(stream_id: number): void {
sub.is_archived = true;
}
export function delete_sub(stream_id: number): void {
if (!stream_info.get(stream_id)) {
blueslip.warn("Failed to archive stream " + stream_id.toString());
return;
}
sub_store.delete_sub(stream_id);
stream_info.delete(stream_id);
}
export function get_non_default_stream_names(): {name: string; unique_id: number}[] {
let subs = [...stream_info.values()];
subs = subs.filter(

28
web/src/stream_events.ts
View File

@@ -9,10 +9,12 @@ import * as blueslip from "./blueslip.ts";
import * as browser_history from "./browser_history.ts";
import * as color_data from "./color_data.ts";
import * as compose_recipient from "./compose_recipient.ts";
import * as compose_state from "./compose_state.ts";
import * as dialog_widget from "./dialog_widget.ts";
import * as hash_util from "./hash_util.ts";
import {$t, $t_html} from "./i18n.ts";
import * as message_lists from "./message_lists.ts";
import * as message_live_update from "./message_live_update.ts";
import * as message_view from "./message_view.ts";
import * as message_view_header from "./message_view_header.ts";
import * as narrow_state from "./narrow_state.ts";
@@ -21,6 +23,7 @@ import * as peer_data from "./peer_data.ts";
import * as people from "./people.ts";
import * as recent_view_ui from "./recent_view_ui.ts";
import * as settings_notifications from "./settings_notifications.ts";
import * as settings_streams from "./settings_streams.ts";
import {realm} from "./state_data.ts";
import * as stream_color_events from "./stream_color_events.ts";
import * as stream_create from "./stream_create.ts";
@@ -177,6 +180,31 @@ export function update_property<P extends keyof UpdatableStreamProperties>(
update_stream_setting(sub, value, "is_recently_active");
stream_list.update_streams_sidebar();
},
is_archived(value) {
if (!value) {
// We currently do not live-update when unarchiving stream.
return;
}
const is_subscribed = sub.subscribed;
const is_narrowed_to_stream = narrow_state.narrowed_to_stream_id(stream_id);
stream_data.mark_archived(stream_id);
stream_settings_ui.update_settings_for_archived(sub);
message_view_header.maybe_rerender_title_area_for_stream(stream_id);
if (is_subscribed) {
stream_list.remove_sidebar_row(stream_id);
if (stream_id === compose_state.selected_recipient_id) {
compose_state.set_selected_recipient_id("");
compose_recipient.on_compose_select_recipient_update();
}
}
settings_streams.update_default_streams_table();
stream_data.remove_default_stream(stream_id);
if (is_narrowed_to_stream) {
assert(message_lists.current !== undefined);
message_lists.current.update_trailing_bookend(true);
}
message_live_update.rerender_messages_view();
},
};
if (Object.hasOwn(updaters, property) && updaters[property] !== undefined) {

15
web/src/stream_settings_ui.ts
View File

@@ -315,6 +315,21 @@ export function add_sub_to_table(sub: StreamSubscription): void {
update_empty_left_panel_message();
}
export function remove_stream(stream_id: number): void {
if (!overlays.streams_open()) {
return;
}
// It is possible that row is empty when we deactivate a
// stream, but we let jQuery silently handle that.
const $row = stream_ui_updates.row_for_stream_id(stream_id);
$row.remove();
update_empty_left_panel_message();
if (hash_parser.is_editing_stream(stream_id)) {
stream_edit.open_edit_panel_empty();
}
}
export function update_settings_for_subscribed(slim_sub: StreamSubscription): void {
const sub = stream_settings_data.get_sub_for_settings(slim_sub);
stream_ui_updates.update_add_subscriptions_elements(sub);

4
web/src/sub_store.ts
View File

@@ -63,6 +63,10 @@ export function clear(): void {
subs_by_stream_id.clear();
}
export function delete_sub(stream_id: number): void {
subs_by_stream_id.delete(stream_id);
}
export function add_hydrated_sub(stream_id: number, sub: StreamSubscription): void {
// The only code that should call this directly is
// in stream_data.js. Grep there to find callers.

41
web/tests/dispatch_subs.test.cjs
View File

@@ -13,8 +13,6 @@ const test_user = events.test_user;
const compose_recipient = mock_esm("../src/compose_recipient");
const message_lists = mock_esm("../src/message_lists");
const message_live_update = mock_esm("../src/message_live_update");
const message_view_header = mock_esm("../src/message_view_header");
const narrow_state = mock_esm("../src/narrow_state");
const overlays = mock_esm("../src/overlays");
const settings_org = mock_esm("../src/settings_org");
@@ -225,17 +223,22 @@ test("stream delete (normal)", ({override}) => {
bookend_updates += 1;
});
const removed_stream_ids = [];
override(stream_settings_ui, "remove_stream", (stream_id) => {
removed_stream_ids.push(stream_id);
});
let removed_sidebar_rows = 0;
override(stream_list, "remove_sidebar_row", () => {
removed_sidebar_rows += 1;
});
override(stream_settings_ui, "update_settings_for_archived", noop);
override(stream_list, "update_subscribe_to_more_streams_link", noop);
override(message_live_update, "rerender_messages_view", noop);
override(message_view_header, "maybe_rerender_title_area_for_stream", noop);
dispatch(event);
assert.deepEqual(removed_stream_ids, [event.stream_ids[0], event.stream_ids[1]]);
// We should possibly be able to make a single call to
// update_trailing_bookend, but we currently do it for each stream.
assert.equal(bookend_updates, 2);
@@ -263,6 +266,12 @@ test("stream delete (special streams)", ({override}) => {
stream_data.subscribe_myself(devel_sub);
const removed_stream_ids = [];
override(stream_settings_ui, "remove_stream", (stream_id) => {
removed_stream_ids.push(stream_id);
});
// sanity check data
assert.equal(event.stream_ids.length, 2);
override(realm, "realm_new_stream_announcements_stream_id", event.stream_ids[0]);
@@ -270,19 +279,18 @@ test("stream delete (special streams)", ({override}) => {
override(realm, "realm_zulip_update_announcements_stream_id", event.stream_ids[0]);
override(settings_org, "sync_realm_settings", noop);
override(stream_settings_ui, "update_settings_for_archived", noop);
override(settings_streams, "update_default_streams_table", noop);
override(message_lists.current, "update_trailing_bookend", noop);
override(stream_list, "remove_sidebar_row", noop);
override(stream_list, "update_subscribe_to_more_streams_link", noop);
override(message_live_update, "rerender_messages_view", noop);
override(message_view_header, "maybe_rerender_title_area_for_stream", noop);
dispatch(event);
assert.equal(realm.realm_new_stream_announcements_stream_id, -1);
assert.equal(realm.realm_signup_announcements_stream_id, -1);
assert.equal(realm.realm_zulip_update_announcements_stream_id, -1);
assert.deepEqual(removed_stream_ids, [event.stream_ids[0], event.stream_ids[1]]);
assert.equal(realm.realm_new_stream_announcements_stream_id, event.stream_ids[0]);
assert.equal(realm.realm_signup_announcements_stream_id, event.stream_ids[1]);
assert.equal(realm.realm_zulip_update_announcements_stream_id, event.stream_ids[0]);
});
test("stream delete (stream is selected in compose)", ({override}) => {
@@ -308,6 +316,12 @@ test("stream delete (stream is selected in compose)", ({override}) => {
stream_data.subscribe_myself(devel_sub);
compose_state.set_stream_id(event.stream_ids[0]);
const removed_stream_ids = [];
override(stream_settings_ui, "remove_stream", (stream_id) => {
removed_stream_ids.push(stream_id);
});
override(settings_streams, "update_default_streams_table", noop);
narrow_state.narrowed_to_stream_id = () => true;
@@ -321,13 +335,12 @@ test("stream delete (stream is selected in compose)", ({override}) => {
override(stream_list, "remove_sidebar_row", () => {
removed_sidebar_rows += 1;
});
override(stream_settings_ui, "update_settings_for_archived", noop);
override(stream_list, "update_subscribe_to_more_streams_link", noop);
override(message_live_update, "rerender_messages_view", noop);
override(message_view_header, "maybe_rerender_title_area_for_stream", noop);
dispatch(event);
assert.deepEqual(removed_stream_ids, [event.stream_ids[0], event.stream_ids[1]]);
assert.equal(compose_state.stream_name(), "");
// We should possibly be able to make a single call to

26
web/tests/stream_data.test.cjs
View File

@@ -708,6 +708,28 @@ test("default_stream_names", () => {
});
test("delete_sub", () => {
const canada = {
stream_id: 101,
name: "Canada",
subscribed: true,
};
stream_data.add_sub(canada);
assert.ok(stream_data.is_subscribed(canada.stream_id));
assert.equal(stream_data.get_sub("Canada").stream_id, canada.stream_id);
assert.equal(sub_store.get(canada.stream_id).name, "Canada");
stream_data.delete_sub(canada.stream_id);
assert.ok(!stream_data.is_subscribed(canada.stream_id));
assert.ok(!stream_data.get_sub("Canada"));
assert.ok(!sub_store.get(canada.stream_id));
blueslip.expect("warn", "Failed to archive stream 99999");
stream_data.delete_sub(99999);
});
test("mark_archived", () => {
const canada = {
is_archived: false,
stream_id: 101,
@@ -724,7 +746,7 @@ test("delete_sub", () => {
assert.equal(sub_store.get(canada.stream_id).name, "Canada");
assert.equal(stream_data.is_stream_archived(canada.stream_id), false);
stream_data.delete_sub(canada.stream_id);
stream_data.mark_archived(canada.stream_id);
assert.ok(stream_data.is_stream_archived(canada.stream_id));
assert.ok(stream_data.is_subscribed(canada.stream_id));
assert.ok(stream_data.get_sub("Canada"));
@@ -733,7 +755,7 @@ test("delete_sub", () => {
assert.equal(stream_data.get_archived_subs().length, archived_subs.length + 1);
blueslip.expect("warn", "Failed to archive stream 99999");
stream_data.delete_sub(99999);
stream_data.mark_archived(99999);
});
test("notifications", ({override}) => {

42
web/tests/stream_events.test.cjs
View File

@@ -13,6 +13,8 @@ const browser_history = mock_esm("../src/browser_history");
const color_data = mock_esm("../src/color_data");
const compose_recipient = mock_esm("../src/compose_recipient");
const dialog_widget = mock_esm("../src/dialog_widget");
const message_live_update = mock_esm("../src/message_live_update");
const settings_streams = mock_esm("../src/settings_streams");
const stream_color_events = mock_esm("../src/stream_color_events");
const stream_list = mock_esm("../src/stream_list");
const stream_muting = mock_esm("../src/stream_muting");
@@ -46,6 +48,7 @@ const user_profile = mock_esm("../src/user_profile");
const {Filter} = zrequire("../src/filter");
const activity_ui = zrequire("activity_ui");
const {buddy_list} = zrequire("buddy_list");
const compose_state = zrequire("compose_state");
const narrow_state = zrequire("narrow_state");
const peer_data = zrequire("peer_data");
const people = zrequire("people");
@@ -313,6 +316,45 @@ test("update_property", ({override}) => {
assert.equal(args.sub, sub);
}
// Test archiving stream
{
stream_data.subscribe_myself(sub);
const stub = make_stub();
override(stream_settings_ui, "update_settings_for_archived", stub.f);
override(settings_streams, "update_default_streams_table", noop);
override(message_live_update, "rerender_messages_view", noop);
narrow_to_frontend();
let bookend_updates = 0;
override(message_lists.current, "update_trailing_bookend", () => {
bookend_updates += 1;
});
let removed_sidebar_rows = 0;
override(stream_list, "remove_sidebar_row", () => {
removed_sidebar_rows += 1;
});
compose_state.set_stream_id(stream_id);
stream_events.update_property(stream_id, "is_archived", true);
assert.ok(stream_data.is_stream_archived(stream_id));
assert.ok(stream_data.is_subscribed(stream_id));
const args = stub.get_args("sub");
assert.equal(args.sub.stream_id, stream_id);
assert.equal(bookend_updates, 1);
assert.equal(removed_sidebar_rows, 1);
}
// We do not live update unarchiving stream, but we test this for coverage.
{
stream_events.update_property(stream_id, "is_archived", false);
}
// Test deprecated properties for coverage.
{
stream_events.update_property(stream_id, "stream_post_policy", 2);