refactor: Only pass stream_id for set_subscribers.

The goal here is to make all our peer_data functions
basically work in id space.  Passing a full `sub`
to these functions is a legacy of when subscriber
info was attached to a full stream "sub" object,
but we don't care about anything sub-related
(color, description, name, etc.) when we are
dealing with subscriptions.

When callers pass in stream_id, you can be more
confident in a quick skim of the code that we're
not mutating anything in the "sub".
This commit is contained in:
Steve Howell
2021-01-13 21:03:25 +00:00
committed by Tim Abbott
parent 6cc880c858
commit 355f44ef13
8 changed files with 19 additions and 20 deletions

View File

@@ -966,13 +966,13 @@ run_test("warn_if_private_stream_is_linked", () => {
};
stream_data.add_sub(test_sub);
peer_data.set_subscribers(test_sub, [1, 2]);
peer_data.set_subscribers(test_sub.stream_id, [1, 2]);
let denmark = {
stream_id: 100,
name: "Denmark",
};
peer_data.set_subscribers(denmark, [1, 2, 3]);
peer_data.set_subscribers(denmark.stream_id, [1, 2, 3]);
function test_noop_case(invite_only) {
compose_state.set_message_type("stream");
@@ -1212,7 +1212,7 @@ run_test("needs_subscribe_warning", () => {
};
stream_data.add_sub(sub);
peer_data.set_subscribers(sub, [bob.user_id, me.user_id]);
peer_data.set_subscribers(sub.stream_id, [bob.user_id, me.user_id]);
blueslip.expect("error", "Unknown user_id in get_by_user_id: 999");
// Test with an invalid user id.

View File

@@ -42,7 +42,7 @@ run_test("set_focused_recipient", () => {
can_access_subscribers: true,
};
stream_data.add_sub(sub);
peer_data.set_subscribers(sub, [me.user_id, alice.user_id]);
peer_data.set_subscribers(sub.stream_id, [me.user_id, alice.user_id]);
set_global("$", (selector) => {
switch (selector) {

View File

@@ -161,7 +161,7 @@ run_test("unsubscribe", () => {
// set up our subscription
stream_data.add_sub(sub);
sub.subscribed = true;
peer_data.set_subscribers(sub, [me.user_id]);
peer_data.set_subscribers(sub.stream_id, [me.user_id]);
// ensure our setup is accurate
assert(stream_data.is_subscribed("devel"));
@@ -215,7 +215,7 @@ run_test("subscribers", () => {
george.user_id,
]);
peer_data.set_subscribers(sub, [me.user_id, fred.user_id, george.user_id]);
peer_data.set_subscribers(sub.stream_id, [me.user_id, fred.user_id, george.user_id]);
stream_data.update_calculated_fields(sub);
assert(stream_data.is_user_subscribed(sub.stream_id, me.user_id));
assert(stream_data.is_user_subscribed(sub.stream_id, fred.user_id));
@@ -224,7 +224,7 @@ run_test("subscribers", () => {
assert.deepEqual(potential_subscriber_ids(), [not_fred.user_id]);
peer_data.set_subscribers(sub, []);
peer_data.set_subscribers(sub.stream_id, []);
const brutus = {
email: "brutus@zulip.com",
@@ -285,7 +285,6 @@ run_test("subscribers", () => {
// Verify defensive code in set_subscribers, where the second parameter
// can be undefined.
peer_data.set_subscribers(sub);
stream_data.add_sub(sub);
peer_data.add_subscriber(sub.stream_id, brutus.user_id);
sub.subscribed = true;
@@ -978,7 +977,7 @@ run_test("filter inactives", () => {
run_test("is_subscriber_subset", () => {
function make_sub(stream_id, user_ids) {
const sub = {stream_id};
peer_data.set_subscribers(sub, user_ids);
peer_data.set_subscribers(sub.stream_id, user_ids);
return sub;
}

View File

@@ -81,14 +81,14 @@ const denmark = {
render_subscribers: true,
should_display_subscription_button: true,
};
peer_data.set_subscribers(denmark, [me.user_id, mark.user_id]);
peer_data.set_subscribers(denmark.stream_id, [me.user_id, mark.user_id]);
const sweden = {
stream_id: 2,
name: "Sweden",
subscribed: false,
};
peer_data.set_subscribers(sweden, [mark.user_id, jill.user_id]);
peer_data.set_subscribers(sweden.stream_id, [mark.user_id, jill.user_id]);
const subs = [denmark, sweden];
for (const sub of subs) {

View File

@@ -56,7 +56,7 @@ run_test("sort_streams", () => {
function process_test_streams() {
for (const test_stream of test_streams) {
peer_data.set_subscribers(test_stream, test_stream.subscribers);
peer_data.set_subscribers(test_stream.stream_id, test_stream.subscribers);
delete test_stream.subscribers;
stream_data.update_calculated_fields(test_stream);
}

View File

@@ -18,9 +18,9 @@ export function clear() {
stream_subscribers = new Map();
}
export function maybe_clear_subscribers(sub) {
if (!stream_subscribers.has(sub.stream_id)) {
set_subscribers(sub, []);
export function maybe_clear_subscribers(stream_id) {
if (!stream_subscribers.has(stream_id)) {
set_subscribers(stream_id, []);
}
}
@@ -94,9 +94,9 @@ export function get_subscribers(stream_id) {
return Array.from(subscribers.keys());
}
export function set_subscribers(sub, user_ids) {
export function set_subscribers(stream_id, user_ids) {
const subscribers = new LazySet(user_ids || []);
stream_subscribers.set(sub.stream_id, subscribers);
stream_subscribers.set(stream_id, subscribers);
}
export function add_subscriber(stream_id, user_id) {

View File

@@ -212,7 +212,7 @@ exports.add_sub = function (sub) {
// We use create_sub_from_server_data at page load.
// We use create_streams for new streams in live-update events.
peer_data.maybe_clear_subscribers(sub);
peer_data.maybe_clear_subscribers(sub.stream_id);
stream_info.set(sub.name, sub);
subs_by_stream_id.set(sub.stream_id, sub);
};
@@ -716,7 +716,7 @@ exports.create_sub_from_server_data = function (attrs) {
...attrs,
};
peer_data.set_subscribers(sub, subscriber_user_ids);
peer_data.set_subscribers(sub.stream_id, subscriber_user_ids);
if (!sub.color) {
sub.color = color_data.pick_color();

View File

@@ -99,7 +99,7 @@ exports.mark_subscribed = function (sub, subscribers, color) {
}
stream_data.subscribe_myself(sub);
if (subscribers) {
peer_data.set_subscribers(sub, subscribers);
peer_data.set_subscribers(sub.stream_id, subscribers);
}
stream_data.update_calculated_fields(sub);