Mirror
/
zulip
mirror of https://github.com/zulip/zulip.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

js: Break cyclic dependency between modules related to `stream_data`. 

Moved `stream_data.is_active` to `stream_sort.has_recent_activity` to
remove the cyclic dependency between `stream_data` and `stream_topic_history`.
This commit is contained in:
Lalit 2023-04-21 23:32:54 +05:30 committed by Tim Abbott
parent 40cea58f87
commit 2996e8e722
12 changed files with 225 additions and 191 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
web/src/server_events_dispatch.js
View File

@ -63,6 +63,7 @@ import * as stream_data from "./stream_data";
import * as stream_events from "./stream_events";
import * as stream_list from "./stream_list";
import * as stream_settings_ui from "./stream_settings_ui";
import * as stream_sort from "./stream_sort";
import * as stream_topic_history from "./stream_topic_history";
import * as stream_ui_updates from "./stream_ui_updates";
import * as sub_store from "./sub_store";
@ -668,7 +669,7 @@ export function dispatch_normal_event(event) {
}
if (event.property === "demote_inactive_streams") {
stream_list.update_streams_sidebar();
stream_data.set_filter_out_inactives();
stream_sort.set_filter_out_inactives();
}
if (event.property === "user_list_style") {
settings_display.report_user_list_style_change(

43
web/src/stream_data.js
View File

@ -6,7 +6,6 @@ import {page_params} from "./page_params";
import * as peer_data from "./peer_data";
import * as people from "./people";
import * as settings_config from "./settings_config";
import * as stream_topic_history from "./stream_topic_history";
import * as sub_store from "./sub_store";
import * as user_groups from "./user_groups";
import {user_settings} from "./user_settings";
@ -98,7 +97,6 @@ class BinaryDict {
// The stream_info variable maps stream names to stream properties objects
// Call clear_subscriptions() to initialize it.
let stream_info;
let filter_out_inactives = false;
const stream_ids_by_name = new FoldDict();
const default_stream_ids = new Set();
@ -170,45 +168,6 @@ export function clear_subscriptions() {
clear_subscriptions();
export function set_filter_out_inactives() {
if (
user_settings.demote_inactive_streams ===
settings_config.demote_inactive_streams_values.automatic.code
) {
filter_out_inactives = num_subscribed_subs() >= 30;
} else if (
user_settings.demote_inactive_streams ===
settings_config.demote_inactive_streams_values.always.code
) {
filter_out_inactives = true;
} else {
filter_out_inactives = false;
}
}
// for testing:
export function is_filtering_inactives() {
return filter_out_inactives;
}
export function is_active(sub) {
if (!filter_out_inactives || sub.pin_to_top) {
// If users don't want to filter inactive streams
// to the bottom, we respect that setting and don't
// treat any streams as dormant.
//
// Currently this setting is automatically determined
// by the number of streams. See the callers
// to set_filter_out_inactives.
return true;
}
return stream_topic_history.stream_has_topics(sub.stream_id) || sub.newly_subscribed;
}
export function is_muted_active(sub) {
return sub.is_muted && is_active(sub);
}
export function rename_sub(sub, new_name) {
const old_name = sub.name;
@ -889,8 +848,6 @@ export function initialize(params) {
populate_subscriptions(subscriptions, true, true);
populate_subscriptions(unsubscribed, false, true);
populate_subscriptions(never_subscribed, false, false);
set_filter_out_inactives();
}
export function remove_default_stream(stream_id) {

2
web/src/stream_list.js
View File

@ -374,7 +374,7 @@ class StreamSidebarRow {
}
update_whether_active() {
if (stream_data.is_active(this.sub) || this.sub.pin_to_top === true) {
if (stream_sort.has_recent_activity(this.sub) || this.sub.pin_to_top === true) {
this.$list_item.removeClass("inactive_stream");
} else {
this.$list_item.addClass("inactive_stream");

55
web/src/stream_sort.js
View File

@ -1,5 +1,8 @@
import * as settings_config from "./settings_config";
import * as stream_data from "./stream_data";
import * as stream_topic_history from "./stream_topic_history";
import * as sub_store from "./sub_store";
import {user_settings} from "./user_settings";
import * as util from "./util";
let previous_pinned;
@ -9,6 +12,13 @@ let previous_muted_active;
let previous_muted_pinned;
let all_streams = [];
// Because we need to check whether we are filtering inactive streams
// in a loop over all streams to render the left sidebar, and the
// definition of demote_inactive_streams involves how many streams
// there are, we maintain this variable as a cache of the calculation
// to avoid making left sidebar rendering a quadratic operation.
let filter_out_inactives = false;
export function get_streams() {
const sorted_streams = all_streams.map((stream_id) =>
stream_data.maybe_get_stream_name(stream_id),
@ -26,6 +36,45 @@ function compare_function(a, b) {
return util.strcmp(stream_name_a, stream_name_b);
}
export function set_filter_out_inactives() {
if (
user_settings.demote_inactive_streams ===
settings_config.demote_inactive_streams_values.automatic.code
) {
filter_out_inactives = stream_data.num_subscribed_subs() >= 30;
} else if (
user_settings.demote_inactive_streams ===
settings_config.demote_inactive_streams_values.always.code
) {
filter_out_inactives = true;
} else {
filter_out_inactives = false;
}
}
// Exported for access by unit tests.
export function is_filtering_inactives() {
return filter_out_inactives;
}
export function has_recent_activity(sub) {
if (!filter_out_inactives || sub.pin_to_top) {
// If users don't want to filter inactive streams
// to the bottom, we respect that setting and don't
// treat any streams as dormant.
//
// Currently this setting is automatically determined
// by the number of streams. See the callers
// to set_filter_out_inactives.
return true;
}
return stream_topic_history.stream_has_topics(sub.stream_id) || sub.newly_subscribed;
}
export function has_recent_activity_but_muted(sub) {
return has_recent_activity(sub) && sub.is_muted;
}
export function sort_groups(streams, search_term) {
const stream_id_to_name = (stream) => sub_store.get(stream).name;
// Use -, _ and / as word separators apart from the default space character
@ -38,7 +87,7 @@ export function sort_groups(streams, search_term) {
);
function is_normal(sub) {
return stream_data.is_active(sub);
return has_recent_activity(sub);
}
const pinned_streams = [];
@ -138,3 +187,7 @@ export function next_stream_id(stream_id) {
return maybe_get_stream_id(i + 1);
}
export function initialize() {
set_filter_out_inactives();
}

5
web/src/typeahead_helper.js
View File

@ -11,6 +11,7 @@ import * as people from "./people";
import * as pm_conversations from "./pm_conversations";
import * as recent_senders from "./recent_senders";
import * as stream_data from "./stream_data";
import * as stream_sort from "./stream_sort";
import * as user_groups from "./user_groups";
import * as user_status from "./user_status";
import * as util from "./util";
@ -430,8 +431,8 @@ function activity_score(sub) {
if (sub.pin_to_top) {
stream_score += 2;
}
// Note: A pinned stream may accumulate a 3rd point if it is active
if (stream_data.is_active(sub)) {
// Note: A pinned stream may accumulate a 3rd point if it has recent activity.
if (stream_sort.has_recent_activity(sub)) {
stream_score += 1;
}
}

2
web/src/ui_init.js
View File

@ -94,6 +94,7 @@ import * as stream_edit from "./stream_edit";
import * as stream_edit_subscribers from "./stream_edit_subscribers";
import * as stream_list from "./stream_list";
import * as stream_settings_ui from "./stream_settings_ui";
import * as stream_sort from "./stream_sort";
import * as timerender from "./timerender";
import * as tippyjs from "./tippyjs";
import * as topic_list from "./topic_list";
@ -639,6 +640,7 @@ export function initialize_everything() {
stream_settings_ui.initialize();
user_group_settings_ui.initialize();
stream_list.initialize();
stream_sort.initialize();
condense.initialize();
spoilers.initialize();
lightbox.initialize();

7
web/tests/composebox_typeahead.test.js
View File

@ -40,6 +40,7 @@ const people = zrequire("people");
const user_groups = zrequire("user_groups");
const stream_bar = zrequire("stream_bar");
const stream_data = zrequire("stream_data");
const stream_sort = zrequire("stream_sort");
const compose = zrequire("compose");
const compose_pm_pill = zrequire("compose_pm_pill");
const compose_ui = zrequire("compose_ui");
@ -1062,7 +1063,8 @@ test("initialize", ({override, override_rewire, mock_template}) => {
"demote_inactive_streams",
settings_config.demote_inactive_streams_values.never.code,
);
stream_data.set_filter_out_inactives();
stream_sort.set_filter_out_inactives();
fake_this = {completing: "stream", token: "s"};
actual_value = sort_items(fake_this, [sweden_stream, serbia_stream]);
expected_value = [sweden_stream, serbia_stream];
@ -1073,7 +1075,8 @@ test("initialize", ({override, override_rewire, mock_template}) => {
"demote_inactive_streams",
settings_config.demote_inactive_streams_values.always.code,
);
stream_data.set_filter_out_inactives();
stream_sort.set_filter_out_inactives();
actual_value = sort_items(fake_this, [sweden_stream, serbia_stream]);
expected_value = [sweden_stream, serbia_stream];
assert.deepEqual(actual_value, expected_value);

3
web/tests/dispatch.test.js
View File

@ -63,6 +63,7 @@ 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_sort = mock_esm("../src/stream_sort");
const stream_topic_history = mock_esm("../src/stream_topic_history");
const submessage = mock_esm("../src/submessage");
mock_esm("../src/top_left_corner", {
@ -917,7 +918,7 @@ run_test("user_settings", ({override}) => {
{
const stub = make_stub();
event = event_fixtures.user_settings__demote_inactive_streams;
override(stream_data, "set_filter_out_inactives", noop);
override(stream_sort, "set_filter_out_inactives", noop);
override(stream_list, "update_streams_sidebar", stub.f);
user_settings.demote_inactive_streams = 1;
dispatch(event);

130
web/tests/stream_data.test.js
View File

@ -2,8 +2,6 @@
const {strict: assert} = require("assert");
const _ = require("lodash");
const {zrequire} = require("./lib/namespace");
const {run_test} = require("./lib/test");
const blueslip = require("./lib/zblueslip");
@ -14,13 +12,11 @@ const {page_params, user_settings} = require("./lib/zpage_params");
page_params.development_environment = true;
const color_data = zrequire("color_data");
const stream_topic_history = zrequire("stream_topic_history");
const peer_data = zrequire("peer_data");
const people = zrequire("people");
const sub_store = zrequire("sub_store");
const stream_data = zrequire("stream_data");
const stream_settings_data = zrequire("stream_settings_data");
const settings_config = zrequire("settings_config");
const user_groups = zrequire("user_groups");
const me = {
@ -36,10 +32,6 @@ const test_user = {
};
// set up user data
function contains_sub(subs, sub) {
return subs.some((s) => s.name === sub.name);
}
const admins_group = {
name: "Admins",
id: 1,
@ -251,97 +243,6 @@ test("renames", () => {
assert.equal(actual_id, 42);
});
test("is_active", () => {
let sub;
user_settings.demote_inactive_streams =
settings_config.demote_inactive_streams_values.automatic.code;
stream_data.set_filter_out_inactives();
sub = {name: "pets", subscribed: false, stream_id: 111};
stream_data.add_sub(sub);
assert.ok(stream_data.is_active(sub));
stream_data.subscribe_myself(sub);
assert.ok(stream_data.is_active(sub));
assert.ok(contains_sub(stream_data.subscribed_subs(), sub));
assert.ok(!contains_sub(stream_data.unsubscribed_subs(), sub));
stream_data.unsubscribe_myself(sub);
assert.ok(stream_data.is_active(sub));
sub.pin_to_top = true;
assert.ok(stream_data.is_active(sub));
sub.pin_to_top = false;
const opts = {
stream_id: 222,
message_id: 108,
topic_name: "topic2",
};
stream_topic_history.add_message(opts);
assert.ok(stream_data.is_active(sub));
user_settings.demote_inactive_streams =
settings_config.demote_inactive_streams_values.always.code;
stream_data.set_filter_out_inactives();
sub = {name: "pets", subscribed: false, stream_id: 111};
stream_data.add_sub(sub);
assert.ok(!stream_data.is_active(sub));
sub.pin_to_top = true;
assert.ok(stream_data.is_active(sub));
sub.pin_to_top = false;
stream_data.subscribe_myself(sub);
assert.ok(stream_data.is_active(sub));
stream_data.unsubscribe_myself(sub);
assert.ok(!stream_data.is_active(sub));
sub = {name: "lunch", subscribed: false, stream_id: 222};
stream_data.add_sub(sub);
assert.ok(stream_data.is_active(sub));
stream_topic_history.add_message(opts);
assert.ok(stream_data.is_active(sub));
user_settings.demote_inactive_streams =
settings_config.demote_inactive_streams_values.never.code;
stream_data.set_filter_out_inactives();
sub = {name: "pets", subscribed: false, stream_id: 111};
stream_data.add_sub(sub);
assert.ok(stream_data.is_active(sub));
stream_data.subscribe_myself(sub);
assert.ok(stream_data.is_active(sub));
stream_data.unsubscribe_myself(sub);
assert.ok(stream_data.is_active(sub));
sub.pin_to_top = true;
assert.ok(stream_data.is_active(sub));
stream_topic_history.add_message(opts);
assert.ok(stream_data.is_active(sub));
});
test("is_muted_active", () => {
const sub = {name: "cats", subscribed: true, stream_id: 111, is_muted: true};
stream_data.add_sub(sub);
assert.ok(stream_data.is_muted_active(sub));
});
test("admin_options", () => {
function make_sub() {
const sub = {
@ -870,11 +771,9 @@ test("initialize", () => {
stream_data.initialize(get_params());
}
user_settings.demote_inactive_streams = 1;
page_params.realm_notifications_stream_id = -1;
initialize();
assert.ok(!stream_data.is_filtering_inactives());
const stream_names = new Set(stream_data.get_streams_for_admin().map((elem) => elem.name));
assert.ok(stream_names.has("subscriptions"));
@ -899,35 +798,6 @@ test("initialize", () => {
assert.equal(stream_data.get_notifications_stream(), "foo");
});
test("filter inactives", () => {
user_settings.demote_inactive_streams =
settings_config.demote_inactive_streams_values.automatic.code;
const params = {};
params.unsubscribed = [];
params.never_subscribed = [];
params.subscriptions = [];
params.realm_default_streams = [];
stream_data.initialize(params);
assert.ok(!stream_data.is_filtering_inactives());
_.times(30, (i) => {
const name = "random" + i.toString();
const stream_id = 100 + i;
const sub = {
name,
subscribed: true,
newly_subscribed: false,
stream_id,
};
stream_data.add_sub(sub);
});
stream_data.initialize(params);
assert.ok(stream_data.is_filtering_inactives());
});
test("edge_cases", () => {
const bad_stream_ids = [555555, 99999];

16
web/tests/stream_list.test.js
View File

@ -200,11 +200,11 @@ test_ui("create_sidebar_row", ({override_rewire, mock_template}) => {
assert.ok(!$social_li.hasClass("out_of_home_view"));
const row = stream_list.stream_sidebar.get_row(stream_id);
override_rewire(stream_data, "is_active", () => true);
override_rewire(stream_sort, "has_recent_activity", () => true);
row.update_whether_active();
assert.ok(!$social_li.hasClass("inactive_stream"));
override_rewire(stream_data, "is_active", () => false);
override_rewire(stream_sort, "has_recent_activity", () => false);
row.update_whether_active();
assert.ok($social_li.hasClass("inactive_stream"));
@ -229,16 +229,16 @@ test_ui("pinned_streams_never_inactive", ({override_rewire, mock_template}) => {
const $social_sidebar = $("<social-sidebar-row-stub>");
let stream_id = social.stream_id;
let row = stream_list.stream_sidebar.get_row(stream_id);
override_rewire(stream_data, "is_active", () => false);
override_rewire(stream_sort, "has_recent_activity", () => false);
stream_list.build_stream_list();
assert.ok($social_sidebar.hasClass("inactive_stream"));
override_rewire(stream_data, "is_active", () => true);
override_rewire(stream_sort, "has_recent_activity", () => true);
row.update_whether_active();
assert.ok(!$social_sidebar.hasClass("inactive_stream"));
override_rewire(stream_data, "is_active", () => false);
override_rewire(stream_sort, "has_recent_activity", () => false);
row.update_whether_active();
assert.ok($social_sidebar.hasClass("inactive_stream"));
@ -246,7 +246,7 @@ test_ui("pinned_streams_never_inactive", ({override_rewire, mock_template}) => {
const $devel_sidebar = $("<devel-sidebar-row-stub>");
stream_id = devel.stream_id;
row = stream_list.stream_sidebar.get_row(stream_id);
override_rewire(stream_data, "is_active", () => false);
override_rewire(stream_sort, "has_recent_activity", () => false);
stream_list.build_stream_list();
assert.ok(!$devel_sidebar.hasClass("inactive_stream"));
@ -499,7 +499,7 @@ test_ui("sort_streams", ({override_rewire, mock_template}) => {
initialize_stream_data();
override_rewire(stream_data, "is_active", (sub) => sub.name !== "cars");
override_rewire(stream_sort, "has_recent_activity", (sub) => sub.name !== "cars");
let appended_elems;
$("#stream_filters").append = (elems) => {
@ -587,7 +587,7 @@ test_ui("separators_only_pinned_and_dormant", ({override_rewire, mock_template})
};
add_row(DenmarkSub);
override_rewire(stream_data, "is_active", (sub) => sub.name !== "Denmark");
override_rewire(stream_sort, "has_recent_activity", (sub) => sub.name !== "Denmark");
let appended_elems;
$("#stream_filters").append = (elems) => {

146
web/tests/stream_sort.test.js
View File

@ -2,11 +2,27 @@
const {strict: assert} = require("assert");
const _ = require("lodash");
const {zrequire} = require("./lib/namespace");
const {run_test} = require("./lib/test");
const {user_settings} = require("./lib/zpage_params");
const people = zrequire("people");
const stream_data = zrequire("stream_data");
const stream_topic_history = zrequire("stream_topic_history");
const stream_sort = zrequire("stream_sort");
const settings_config = zrequire("settings_config");
function contains_sub(subs, sub) {
return subs.some((s) => s.name === sub.name);
}
const me = {
email: "me@zulip.com",
full_name: "Current User",
user_id: 100,
};
const scalene = {
subscribed: true,
@ -94,7 +110,7 @@ test("basics", ({override_rewire}) => {
stream_data.add_sub(muted_active);
stream_data.add_sub(muted_pinned);
override_rewire(stream_data, "is_active", (sub) => sub.name !== "pneumonia");
override_rewire(stream_sort, "has_recent_activity", (sub) => sub.name !== "pneumonia");
// Test sorting into categories/alphabetized
let sorted = sort_groups("");
@ -175,3 +191,131 @@ test("basics", ({override_rewire}) => {
assert.deepEqual(sorted.normal_streams, [stream_hyphen_underscore_slash.stream_id]);
assert.deepEqual(sorted.dormant_streams, []);
});
test("has_recent_activity", () => {
people.init();
people.add_active_user(me);
people.initialize_current_user(me.user_id);
let sub;
user_settings.demote_inactive_streams =
settings_config.demote_inactive_streams_values.automatic.code;
stream_sort.set_filter_out_inactives();
sub = {name: "pets", subscribed: false, stream_id: 111};
stream_data.add_sub(sub);
assert.ok(stream_sort.has_recent_activity(sub));
stream_data.subscribe_myself(sub);
assert.ok(stream_sort.has_recent_activity(sub));
assert.ok(contains_sub(stream_data.subscribed_subs(), sub));
assert.ok(!contains_sub(stream_data.unsubscribed_subs(), sub));
stream_data.unsubscribe_myself(sub);
assert.ok(stream_sort.has_recent_activity(sub));
sub.pin_to_top = true;
assert.ok(stream_sort.has_recent_activity(sub));
sub.pin_to_top = false;
const opts = {
stream_id: 222,
message_id: 108,
topic_name: "topic2",
};
stream_topic_history.add_message(opts);
assert.ok(stream_sort.has_recent_activity(sub));
user_settings.demote_inactive_streams =
settings_config.demote_inactive_streams_values.always.code;
stream_sort.set_filter_out_inactives();
sub = {name: "pets", subscribed: false, stream_id: 111};
stream_data.add_sub(sub);
assert.ok(!stream_sort.has_recent_activity(sub));
sub.pin_to_top = true;
assert.ok(stream_sort.has_recent_activity(sub));
sub.pin_to_top = false;
stream_data.subscribe_myself(sub);
assert.ok(stream_sort.has_recent_activity(sub));
stream_data.unsubscribe_myself(sub);
assert.ok(!stream_sort.has_recent_activity(sub));
sub = {name: "lunch", subscribed: false, stream_id: 222};
stream_data.add_sub(sub);
assert.ok(stream_sort.has_recent_activity(sub));
stream_topic_history.add_message(opts);
assert.ok(stream_sort.has_recent_activity(sub));
user_settings.demote_inactive_streams =
settings_config.demote_inactive_streams_values.never.code;
stream_sort.set_filter_out_inactives();
sub = {name: "pets", subscribed: false, stream_id: 111};
stream_data.add_sub(sub);
assert.ok(stream_sort.has_recent_activity(sub));
stream_data.subscribe_myself(sub);
assert.ok(stream_sort.has_recent_activity(sub));
stream_data.unsubscribe_myself(sub);
assert.ok(stream_sort.has_recent_activity(sub));
sub.pin_to_top = true;
assert.ok(stream_sort.has_recent_activity(sub));
stream_topic_history.add_message(opts);
assert.ok(stream_sort.has_recent_activity(sub));
});
test("has_recent_activity_but_muted", () => {
const sub = {name: "cats", subscribed: true, stream_id: 111, is_muted: true};
stream_data.add_sub(sub);
assert.ok(stream_sort.has_recent_activity_but_muted(sub));
});
test("filter inactives", () => {
user_settings.demote_inactive_streams =
settings_config.demote_inactive_streams_values.automatic.code;
assert.ok(!stream_sort.is_filtering_inactives());
_.times(30, (i) => {
const name = "random" + i.toString();
const stream_id = 100 + i;
const sub = {
name,
subscribed: true,
newly_subscribed: false,
stream_id,
};
stream_data.add_sub(sub);
});
stream_sort.set_filter_out_inactives();
assert.ok(stream_sort.is_filtering_inactives());
});
test("initialize", () => {
user_settings.demote_inactive_streams = 1;
stream_sort.initialize();
assert.ok(!stream_sort.is_filtering_inactives());
});

4
web/tests/typeahead_helper.test.js
View File

@ -15,6 +15,7 @@ const recent_senders = zrequire("recent_senders");
const peer_data = zrequire("peer_data");
const people = zrequire("people");
const stream_data = zrequire("stream_data");
const stream_sort = zrequire("stream_sort");
const compose_state = zrequire("compose_state");
const emoji = zrequire("emoji");
const pygments_data = zrequire("../generated/pygments_data.json");
@ -156,7 +157,8 @@ test("sort_streams", ({override}) => {
"demote_inactive_streams",
settings_config.demote_inactive_streams_values.always.code,
);
stream_data.set_filter_out_inactives();
stream_sort.set_filter_out_inactives();
override(
stream_topic_history,
"stream_has_topics",