stream_settings: Add Not subscribed tab to stream settings.

This commit adds a new tab to the stream settings overlay called
"Not subscribed" which lists all the streams the user is not
subscribed to. This tab is disabled if the user is a guest.

Introduced a new variable called "show_not_subscribed" to replicate
a similar model to how the subscribed tab is working. Currently,
there are only two tabs: "subscribed" and "all streams" so we can
use an if-else condition.

Refactored the 'update_stream_row_in_settings_tab' function inside
stream_ui_updates.js to include the case of a 'Not-subscribed' tab,
so that the tab is immediately updated in any add/remove subscription
event.

Fixed and added the node tests for these changes.
This commit is contained in:
sujal shah 2024-05-08 21:30:20 +05:30 committed by Tim Abbott
parent d7936fe576
commit 6deb165ef6
6 changed files with 196 additions and 56 deletions

View File

@ -3,6 +3,9 @@ import type {ElementHandle, Page} from "puppeteer";
import * as common from "./lib/common"; import * as common from "./lib/common";
async function test_subscription_button(page: Page): Promise<void> { async function test_subscription_button(page: Page): Promise<void> {
const all_stream_selector = "[data-tab-key='all-streams']";
await page.waitForSelector(all_stream_selector, {visible: true});
await page.click(all_stream_selector);
const stream_selector = "[data-stream-name='Venice']"; const stream_selector = "[data-stream-name='Venice']";
const button_selector = `${stream_selector} .sub_unsub_button`; const button_selector = `${stream_selector} .sub_unsub_button`;
const subscribed_selector = `${button_selector}.checked`; const subscribed_selector = `${button_selector}.checked`;

View File

@ -198,7 +198,7 @@ export function channels_settings_edit_url(
} }
export function channels_settings_section_url(section = "subscribed"): string { export function channels_settings_section_url(section = "subscribed"): string {
const valid_section_values = new Set(["new", "subscribed", "all"]); const valid_section_values = new Set(["new", "subscribed", "all", "notsubscribed"]);
if (!valid_section_values.has(section)) { if (!valid_section_values.has(section)) {
blueslip.warn("invalid section for channels settings: " + section); blueslip.warn("invalid section for channels settings: " + section);
return "#channels/subscribed"; return "#channels/subscribed";

View File

@ -43,12 +43,22 @@ export function setup_subscriptions_tab_hash(tab_key_value) {
if ($("#subscription_overlay .right").hasClass("show")) { if ($("#subscription_overlay .right").hasClass("show")) {
return; return;
} }
if (tab_key_value === "all-streams") { switch (tab_key_value) {
browser_history.update("#channels/all"); case "all-streams": {
} else if (tab_key_value === "subscribed") { browser_history.update("#channels/all");
browser_history.update("#channels/subscribed"); break;
} else { }
blueslip.debug("Unknown tab_key_value: " + tab_key_value); case "subscribed": {
browser_history.update("#channels/subscribed");
break;
}
case "not-subscribed": {
browser_history.update("#channels/notsubscribed");
break;
}
default: {
blueslip.debug("Unknown tab_key_value: " + tab_key_value);
}
} }
} }

View File

@ -353,6 +353,11 @@ function triage_stream(left_panel_params, sub) {
return "rejected"; return "rejected";
} }
if (left_panel_params.show_not_subscribed && sub.subscribed) {
// reject subscribed streams
return "rejected";
}
const search_terms = search_util.get_search_terms(left_panel_params.input); const search_terms = search_util.get_search_terms(left_panel_params.input);
function match(attr) { function match(attr) {
@ -508,6 +513,7 @@ export function get_left_panel_params() {
const params = { const params = {
input, input,
show_subscribed: stream_ui_updates.show_subscribed, show_subscribed: stream_ui_updates.show_subscribed,
show_not_subscribed: stream_ui_updates.show_not_subscribed,
sort_order, sort_order,
}; };
return params; return params;
@ -523,10 +529,23 @@ export function switch_stream_tab(tab_name) {
use `toggler.goto`. use `toggler.goto`.
*/ */
if (tab_name === "all-streams") { switch (tab_name) {
stream_ui_updates.set_show_subscribed(false); case "all-streams": {
} else if (tab_name === "subscribed") { stream_ui_updates.set_show_subscribed(false);
stream_ui_updates.set_show_subscribed(true); stream_ui_updates.set_show_not_subscribed(false);
break;
}
case "subscribed": {
stream_ui_updates.set_show_subscribed(true);
stream_ui_updates.set_show_not_subscribed(false);
break;
}
case "not-subscribed": {
stream_ui_updates.set_show_subscribed(false);
stream_ui_updates.set_show_not_subscribed(true);
break;
}
// No default
} }
redraw_left_panel(); redraw_left_panel();
@ -602,6 +621,7 @@ export function setup_page(callback) {
child_wants_focus: true, child_wants_focus: true,
values: [ values: [
{label: $t({defaultMessage: "Subscribed"}), key: "subscribed"}, {label: $t({defaultMessage: "Subscribed"}), key: "subscribed"},
{label: $t({defaultMessage: "Not subscribed"}), key: "not-subscribed"},
{label: $t({defaultMessage: "All channels"}), key: "all-streams"}, {label: $t({defaultMessage: "All channels"}), key: "all-streams"},
], ],
callback(_value, key) { callback(_value, key) {
@ -615,6 +635,7 @@ export function setup_page(callback) {
} }
if (current_user.is_guest) { if (current_user.is_guest) {
toggler.disable_tab("all-streams"); toggler.disable_tab("all-streams");
toggler.disable_tab("not-subscribed");
} }
// show the "Stream settings" header by default. // show the "Stream settings" header by default.
@ -744,6 +765,12 @@ export function change_state(section, left_side_tab, right_side_tab) {
return; return;
} }
if (section === "notsubscribed") {
toggler.goto("not-subscribed");
stream_edit.empty_right_panel();
return;
}
// if the section is a valid number. // if the section is a valid number.
if (/\d+/.test(section)) { if (/\d+/.test(section)) {
const stream_id = Number.parseInt(section, 10); const stream_id = Number.parseInt(section, 10);
@ -846,9 +873,13 @@ export function toggle_view(event) {
const stream_filter_tab = active_data.$tabs.first().text(); const stream_filter_tab = active_data.$tabs.first().text();
if (event === "right_arrow" && stream_filter_tab === "Subscribed") { if (event === "right_arrow" && stream_filter_tab === "Subscribed") {
toggler.goto("not-subscribed");
} else if (event === "right_arrow" && stream_filter_tab === "Not subscribed") {
toggler.goto("all-streams"); toggler.goto("all-streams");
} else if (event === "left_arrow" && stream_filter_tab === "All channels") { } else if (event === "left_arrow" && stream_filter_tab === "Not subscribed") {
toggler.goto("subscribed"); toggler.goto("subscribed");
} else if (event === "left_arrow" && stream_filter_tab === "All channels") {
toggler.goto("not-subscribed");
} }
} }

View File

@ -29,6 +29,7 @@ function settings_button_for_sub(sub) {
} }
export let show_subscribed = true; export let show_subscribed = true;
export let show_not_subscribed = false;
export function is_subscribed_stream_tab_active() { export function is_subscribed_stream_tab_active() {
// Returns true if "Subscribed" tab in stream settings is open // Returns true if "Subscribed" tab in stream settings is open
@ -36,10 +37,20 @@ export function is_subscribed_stream_tab_active() {
return show_subscribed; return show_subscribed;
} }
export function is_not_subscribed_stream_tab_active() {
// Returns true if "not-subscribed" tab in stream settings is open
// otherwise false.
return show_not_subscribed;
}
export function set_show_subscribed(value) { export function set_show_subscribed(value) {
show_subscribed = value; show_subscribed = value;
} }
export function set_show_not_subscribed(value) {
show_not_subscribed = value;
}
export function update_web_public_stream_privacy_option_state($container) { export function update_web_public_stream_privacy_option_state($container) {
const $web_public_stream_elem = $container.find( const $web_public_stream_elem = $container.find(
`input[value='${CSS.escape( `input[value='${CSS.escape(
@ -324,15 +335,20 @@ export function update_stream_row_in_settings_tab(sub) {
// This is in the left panel. // This is in the left panel.
// This function display/hide stream row in stream settings tab, // This function display/hide stream row in stream settings tab,
// used to display immediate effect of add/removal subscription event. // used to display immediate effect of add/removal subscription event.
// If user is subscribed to stream, it will show sub row under // If user is subscribed or unsubscribed to stream, it will show sub or unsub
// "Subscribed" tab, otherwise if stream is not public hide // row under "Subscribed" or "Not subscribed" (only if the stream is public) tab, otherwise
// stream row under tab. // if stream is not public hide stream row under tab.
if (is_subscribed_stream_tab_active()) {
const $sub_row = row_for_stream_id(sub.stream_id); if (is_subscribed_stream_tab_active() || is_not_subscribed_stream_tab_active()) {
if (sub.subscribed) { const $row = row_for_stream_id(sub.stream_id);
$sub_row.removeClass("notdisplayed");
if (
(is_subscribed_stream_tab_active() && sub.subscribed) ||
(is_not_subscribed_stream_tab_active() && !sub.subscribed)
) {
$row.removeClass("notdisplayed");
} else if (sub.invite_only || current_user.is_guest) { } else if (sub.invite_only || current_user.is_guest) {
$sub_row.addClass("notdisplayed"); $row.addClass("notdisplayed");
} }
} }
} }

View File

@ -105,8 +105,47 @@ run_test("redraw_left_panel", ({mock_template}) => {
date_created: 1691057093, date_created: 1691057093,
creator_id: null, creator_id: null,
}; };
const abcd = {
elem: "abcd",
subscribed: false,
name: "Abcd",
stream_id: 106,
description: "India town",
subscribers: [1, 2, 3],
stream_weekly_traffic: 0,
color: "red",
can_remove_subscribers_group: admins_group.id,
date_created: 1691057093,
creator_id: null,
};
const utopia = {
elem: "utopia",
subscribed: false,
name: "Utopia",
stream_id: 107,
description: "movie",
subscribers: [1, 2, 3, 4],
stream_weekly_traffic: 8,
color: "red",
can_remove_subscribers_group: admins_group.id,
date_created: 1691057093,
creator_id: null,
};
const jerry = {
elem: "jerry",
subscribed: false,
name: "Jerry",
stream_id: 108,
description: "cat",
subscribers: [1],
stream_weekly_traffic: 4,
color: "red",
can_remove_subscribers_group: admins_group.id,
date_created: 1691057093,
creator_id: null,
};
const sub_row_data = [denmark, poland, pomona, cpp, zzyzx]; const sub_row_data = [denmark, poland, pomona, cpp, zzyzx, abcd, utopia, jerry];
for (const sub of sub_row_data) { for (const sub of sub_row_data) {
stream_data.create_sub_from_server_data(sub); stream_data.create_sub_from_server_data(sub);
@ -154,65 +193,106 @@ run_test("redraw_left_panel", ({mock_template}) => {
} }
// Search with single keyword // Search with single keyword
test_filter({input: "Po", show_subscribed: false}, [poland, pomona]); test_filter({input: "Po", show_subscribed: false, show_not_subscribed: false}, [
poland,
pomona,
]);
assert.ok(ui_called); assert.ok(ui_called);
// The denmark row is active, even though it's not displayed. // The denmark row is active, even though it's not displayed.
assert.ok($denmark_row.hasClass("active")); assert.ok($denmark_row.hasClass("active"));
// Search with multiple keywords // Search with multiple keywords
test_filter({input: "Denmark, Pol", show_subscribed: false}, [denmark, poland]); test_filter({input: "Denmark, Pol", show_subscribed: false, show_not_subscribed: false}, [
test_filter({input: "Den, Pol", show_subscribed: false}, [denmark, poland]); denmark,
poland,
]);
test_filter({input: "Den, Pol", show_subscribed: false, show_not_subscribed: false}, [
denmark,
poland,
]);
// Search is case-insensitive // Search is case-insensitive
test_filter({input: "po", show_subscribed: false}, [poland, pomona]); test_filter({input: "po", show_subscribed: false, show_not_subscribed: false}, [
poland,
pomona,
]);
// Search handles unusual characters like C++ // Search handles unusual characters like C++
test_filter({input: "c++", show_subscribed: false}, [cpp]); test_filter({input: "c++", show_subscribed: false, show_not_subscribed: false}, [cpp]);
// Search subscribed streams only // Search subscribed streams only
test_filter({input: "d", show_subscribed: true}, [poland]); test_filter({input: "d", show_subscribed: true, show_not_subscribed: false}, [poland]);
// Search unsubscribed streams only
test_filter({input: "d", show_subscribed: false, show_not_subscribed: true}, [abcd, denmark]);
// Search terms match stream description // Search terms match stream description
test_filter({input: "Co", show_subscribed: false}, [denmark, pomona]); test_filter({input: "Co", show_subscribed: false, show_not_subscribed: false}, [
denmark,
pomona,
]);
// Search names AND descriptions // Search names AND descriptions
test_filter({input: "Mon", show_subscribed: false}, [pomona, poland]); test_filter({input: "Mon", show_subscribed: false, show_not_subscribed: false}, [
pomona,
poland,
]);
// Explicitly order streams by name // Explicitly order streams by name
test_filter({input: "", show_subscribed: false, sort_order: "by-stream-name"}, [ test_filter(
cpp, {
denmark, input: "",
poland, show_subscribed: false,
pomona, show_not_subscribed: false,
zzyzx, sort_order: "by-stream-name",
]); },
[abcd, cpp, denmark, jerry, poland, pomona, utopia, zzyzx],
);
// Order streams by subscriber count // Order streams by subscriber count
test_filter({input: "", show_subscribed: false, sort_order: "by-subscriber-count"}, [ test_filter(
poland, {
cpp, input: "",
zzyzx, show_subscribed: false,
denmark, show_not_subscribed: false,
pomona, sort_order: "by-subscriber-count",
]); },
[utopia, abcd, poland, cpp, zzyzx, denmark, jerry, pomona],
);
// Order streams by weekly traffic // Order streams by weekly traffic
test_filter({input: "", show_subscribed: false, sort_order: "by-weekly-traffic"}, [ test_filter(
poland, {
cpp, input: "",
zzyzx, show_subscribed: false,
pomona, show_not_subscribed: false,
denmark, sort_order: "by-weekly-traffic",
]); },
[poland, utopia, cpp, zzyzx, jerry, abcd, pomona, denmark],
);
// Sort for subscribed only. // Sort for subscribed only.
test_filter({input: "", show_subscribed: true, sort_order: "by-subscriber-count"}, [ test_filter(
poland, {
cpp, input: "",
zzyzx, show_subscribed: true,
pomona, show_not_subscribed: false,
]); sort_order: "by-subscriber-count",
},
[poland, cpp, zzyzx, pomona],
);
// Sort for unsubscribed only.
test_filter(
{
input: "",
show_subscribed: false,
show_not_subscribed: true,
sort_order: "by-subscriber-count",
},
[utopia, abcd, denmark, jerry],
);
// active stream-row is not included in results // active stream-row is not included in results
$(".stream-row-denmark").addClass("active"); $(".stream-row-denmark").addClass("active");