From a29b6485d65cce6254d2d683f93eaaee9d732c11 Mon Sep 17 00:00:00 2001 From: sanchi-t Date: Tue, 2 Jul 2024 00:49:22 +0530 Subject: [PATCH] delete_sub: Do not remove archived stream when deactivated. Stream is simply marked as `archived: true` instead of removing the stream from `sub_store` and `stream_info`. A check in `subscribe_myself` is added before subscribing to a stream. --- web/src/server_events_dispatch.js | 2 ++ web/src/stream_data.ts | 11 ++++++++--- web/tests/dispatch_subs.test.js | 12 ++++++++++++ web/tests/lib/events.js | 2 ++ web/tests/stream_data.test.js | 14 +++++++++++--- 5 files changed, 35 insertions(+), 6 deletions(-) diff --git a/web/src/server_events_dispatch.js b/web/src/server_events_dispatch.js index c1944fa358..bd6392d931 100644 --- a/web/src/server_events_dispatch.js +++ b/web/src/server_events_dispatch.js @@ -604,6 +604,7 @@ export function dispatch_normal_event(event) { ); stream_data.delete_sub(stream.stream_id); stream_settings_ui.remove_stream(stream.stream_id); + message_view_header.maybe_rerender_title_area_for_stream(stream); if (was_subscribed) { stream_list.remove_sidebar_row(stream.stream_id); if (stream.stream_id === compose_state.selected_recipient_id) { @@ -632,6 +633,7 @@ 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: diff --git a/web/src/stream_data.ts b/web/src/stream_data.ts index 985d580b72..9d220adb16 100644 --- a/web/src/stream_data.ts +++ b/web/src/stream_data.ts @@ -143,6 +143,10 @@ export function rename_sub(sub: StreamSubscription, new_name: string): void { } export function subscribe_myself(sub: StreamSubscription): void { + if (sub.is_archived) { + blueslip.warn("Can't subscribe to an archived stream."); + return; + } const user_id = people.my_current_user_id(); peer_data.add_subscriber(sub.stream_id, user_id); sub.subscribed = true; @@ -297,12 +301,13 @@ export function slug_to_stream_id(slug: string): number | undefined { } export function delete_sub(stream_id: number): void { - if (!stream_info.get(stream_id)) { + 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()); return; } - sub_store.delete_sub(stream_id); - stream_info.delete(stream_id); + sub.is_archived = true; + stream_info.set_false(stream_id, sub); } export function get_non_default_stream_names(): {name: string; unique_id: number}[] { diff --git a/web/tests/dispatch_subs.test.js b/web/tests/dispatch_subs.test.js index 4063a9ac59..52814359cb 100644 --- a/web/tests/dispatch_subs.test.js +++ b/web/tests/dispatch_subs.test.js @@ -13,6 +13,8 @@ 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"); @@ -223,6 +225,8 @@ test("stream delete (normal)", ({override}) => { removed_sidebar_rows += 1; }); 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); @@ -239,9 +243,12 @@ test("stream delete (special streams)", ({override}) => { const event = event_fixtures.stream__delete; for (const stream of event.streams) { + stream.is_archived = false; stream_data.add_sub(stream); } + stream_data.subscribe_myself(event.streams[0]); + // sanity check data assert.equal(event.streams.length, 2); override(realm, "realm_new_stream_announcements_stream_id", event.streams[0].stream_id); @@ -254,6 +261,8 @@ test("stream delete (special streams)", ({override}) => { 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); @@ -268,6 +277,7 @@ test("stream delete (stream is selected in compose)", ({override}) => { const event = event_fixtures.stream__delete; for (const stream of event.streams) { + stream.is_archived = false; stream_data.add_sub(stream); } @@ -294,6 +304,8 @@ test("stream delete (stream is selected in compose)", ({override}) => { removed_sidebar_rows += 1; }); 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); diff --git a/web/tests/lib/events.js b/web/tests/lib/events.js index d0e2ef179d..21940c2662 100644 --- a/web/tests/lib/events.js +++ b/web/tests/lib/events.js @@ -40,6 +40,7 @@ const fake_now = 1596713966; exports.test_streams = { devel: { + is_archived: false, name: "devel", description: ":devel fun:", rendered_description: "devel fun", @@ -56,6 +57,7 @@ exports.test_streams = { can_remove_subscribers_group: 2, }, test: { + is_archived: false, name: "test", description: "test desc", rendered_description: "test desc", diff --git a/web/tests/stream_data.test.js b/web/tests/stream_data.test.js index b1cad1c581..ff5dc40647 100644 --- a/web/tests/stream_data.test.js +++ b/web/tests/stream_data.test.js @@ -579,24 +579,32 @@ test("default_stream_names", () => { test("delete_sub", () => { const canada = { + is_archived: false, stream_id: 101, name: "Canada", subscribed: true, }; stream_data.add_sub(canada); + const num_subscribed_subs = stream_data.num_subscribed_subs(); 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"); + assert.equal(stream_data.is_stream_archived(canada.stream_id), false); 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)); + 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")); + assert.ok(sub_store.get(canada.stream_id)); + assert.equal(stream_data.num_subscribed_subs(), num_subscribed_subs - 1); blueslip.expect("warn", "Failed to archive stream 99999"); stream_data.delete_sub(99999); + + blueslip.expect("warn", "Can't subscribe to an archived stream."); + stream_data.subscribe_myself(canada); }); test("notifications", ({override}) => {