muting: Use stream_id for internal data structures.

This fixes the most core data structures inside of
muting.js.  We still use stream names for incoming
data to set_muted_topics and outgoing data from
get_muted_topics.

This will make us more resilient to stream name changes.
Before, if you were logged on when a stream rename
occured, topics that were muted under that stream would
appear to be unmuted.  (You could fix it with a reload,
but it can be jarring to have a bunch of unread messages
appear in your feed suddenly.)

Fixes #11033
This commit is contained in:
Steve Howell
2018-12-13 21:26:10 +00:00
committed by Tim Abbott
parent 69da22d998
commit a8718c9051
15 changed files with 149 additions and 84 deletions

View File

@@ -396,29 +396,31 @@ run_test('bookend', () => {
run_test('unmuted_messages', () => {
var list = new MessageList({});
var muted_stream_id = 999;
var unmuted = [
{
id: 50,
stream: 'bad',
mentioned: true,
stream_id: muted_stream_id,
mentioned: true, // overrides mute
},
{
id: 60,
stream: 'good',
stream_id: 42,
mentioned: false,
},
];
var muted = [
{
id: 70,
stream: 'bad',
stream_id: muted_stream_id,
mentioned: false,
},
];
with_overrides(function (override) {
override('muting.is_topic_muted', function (stream) {
return stream === 'bad';
override('muting.is_topic_muted', function (stream_id) {
return stream_id === muted_stream_id;
});
// Make sure unmuted_message filters out the "muted" entry,

View File

@@ -128,7 +128,7 @@ run_test('muting enabled', () => {
});
run_test('more muting', () => {
muting.is_topic_muted = function (stream, topic) {
muting.is_topic_muted = function (stream_id, topic) {
return topic === 'muted';
};

View File

@@ -1,4 +1,6 @@
zrequire('muting');
zrequire('stream_data');
set_global('blueslip', global.make_zblueslip());
run_test('edge_cases', () => {
// private messages
@@ -8,40 +10,77 @@ run_test('edge_cases', () => {
assert(!muting.is_topic_muted('nonexistent', undefined));
});
var design = {
stream_id: 100,
name: 'design',
};
var devel = {
stream_id: 101,
name: 'devel',
};
var office = {
stream_id: 102,
name: 'office',
};
var social = {
stream_id: 103,
name: 'social',
};
var unknown = {
stream_id: 999,
name: 'whatever',
};
stream_data.add_sub(design.name, design);
stream_data.add_sub(devel.name, devel);
stream_data.add_sub(office.name, office);
stream_data.add_sub(social.name, social);
run_test('basics', () => {
assert(!muting.is_topic_muted('devel', 'java'));
muting.add_muted_topic('devel', 'java');
assert(muting.is_topic_muted('devel', 'java'));
assert(!muting.is_topic_muted(devel.stream_id, 'java'));
muting.add_muted_topic(devel.stream_id, 'java');
assert(muting.is_topic_muted(devel.stream_id, 'java'));
// test idempotentcy
muting.add_muted_topic('devel', 'java');
assert(muting.is_topic_muted('devel', 'java'));
muting.add_muted_topic(devel.stream_id, 'java');
assert(muting.is_topic_muted(devel.stream_id, 'java'));
muting.remove_muted_topic('devel', 'java');
assert(!muting.is_topic_muted('devel', 'java'));
muting.remove_muted_topic(devel.stream_id, 'java');
assert(!muting.is_topic_muted(devel.stream_id, 'java'));
// test idempotentcy
muting.remove_muted_topic('devel', 'java');
assert(!muting.is_topic_muted('devel', 'java'));
muting.remove_muted_topic(devel.stream_id, 'java');
assert(!muting.is_topic_muted(devel.stream_id, 'java'));
// test unknown stream is harmless too
muting.remove_muted_topic('unknown', 'java');
assert(!muting.is_topic_muted('unknown', 'java'));
muting.remove_muted_topic(unknown.stream_id, 'java');
assert(!muting.is_topic_muted(unknown.stream_id, 'java'));
});
run_test('get_and_set_muted_topics', () => {
assert.deepEqual(muting.get_muted_topics(), []);
muting.add_muted_topic('office', 'gossip');
muting.add_muted_topic('devel', 'java');
muting.add_muted_topic(office.stream_id, 'gossip');
muting.add_muted_topic(devel.stream_id, 'java');
assert.deepEqual(muting.get_muted_topics().sort(), [
['devel', 'java'],
['office', 'gossip'],
]);
blueslip.set_test_data('warn', 'Unknown stream in set_muted_topics: BOGUS STREAM');
muting.set_muted_topics([
['social', 'breakfast'],
['design', 'typography'],
['BOGUS STREAM', 'whatever'],
]);
blueslip.clear_test_data();
assert.deepEqual(muting.get_muted_topics().sort(), [
['design', 'typography'],
['social', 'breakfast'],
@@ -50,11 +89,10 @@ run_test('get_and_set_muted_topics', () => {
run_test('case_insensitivity', () => {
muting.set_muted_topics([]);
assert(!muting.is_topic_muted('SOCial', 'breakfast'));
assert(!muting.is_topic_muted(social.stream_id, 'breakfast'));
muting.set_muted_topics([
['SOCial', 'breakfast'],
]);
assert(muting.is_topic_muted('SOCial', 'breakfast'));
assert(muting.is_topic_muted('social', 'breakfast'));
assert(muting.is_topic_muted('social', 'breakFAST'));
assert(muting.is_topic_muted(social.stream_id, 'breakfast'));
assert(muting.is_topic_muted(social.stream_id, 'breakFAST'));
});

View File

@@ -1,14 +1,21 @@
set_global('$', global.make_zjquery());
zrequire('settings_muting');
zrequire('stream_data');
zrequire('muting');
set_global('muting_ui', {});
var noop = function () {};
var frontend = {
stream_id: 101,
name: 'frontend',
};
stream_data.add_sub('frontend', frontend);
run_test('settings', () => {
muting.add_muted_topic('frontend', 'js');
muting.add_muted_topic(frontend.stream_id, 'js');
var set_up_ui_called = false;
muting_ui.set_up_muted_topics_ui = function (opts) {
assert.deepEqual(opts, [['frontend', 'js']]);

View File

@@ -2,7 +2,6 @@ set_global('$', global.make_zjquery());
set_global('i18n', global.stub_i18n);
set_global('narrow_state', {});
set_global('stream_data', {});
set_global('unread', {});
set_global('muting', {});
set_global('stream_popover', {});
@@ -14,12 +13,18 @@ zrequire('unread');
zrequire('topic_data');
zrequire('topic_list');
var devel = {
stream_id: 555,
name: 'devel',
};
stream_data.add_sub('devel', devel);
run_test('topic_list_build_widget', () => {
var stream_id = 555;
topic_data.reset();
topic_data.add_message({
stream_id: stream_id,
stream_id: devel.stream_id,
topic_name: 'coding',
message_id: 400,
});
@@ -34,13 +39,6 @@ run_test('topic_list_build_widget', () => {
return 3;
};
stream_data.get_sub_by_id = function (stream_id) {
assert.equal(stream_id, 555);
return {
name: 'devel',
};
};
var checked_mutes;
var rendered;
@@ -51,15 +49,15 @@ run_test('topic_list_build_widget', () => {
unread: 3,
is_zero: false,
is_muted: false,
url: '#narrow/stream/devel/subject/coding',
url: '#narrow/stream/555-devel/subject/coding',
};
assert.deepEqual(info, expected);
rendered = true;
return '<topic list item>';
};
muting.is_topic_muted = function (stream_name, topic_name) {
assert.equal(stream_name, 'devel');
muting.is_topic_muted = function (stream_id, topic_name) {
assert.equal(stream_id, devel.stream_id);
assert.equal(topic_name, 'coding');
checked_mutes = true;
return false;
@@ -83,7 +81,7 @@ run_test('topic_list_build_widget', () => {
assert.equal(topic_list.active_stream_id(), undefined);
var widget = topic_list.widget(parent_elem, stream_id);
var widget = topic_list.widget(parent_elem, devel.stream_id);
widget.build_more_topics_section = function () {
return $('<more topics>');
@@ -91,7 +89,7 @@ run_test('topic_list_build_widget', () => {
widget.build();
assert(widget.is_for_stream(stream_id));
assert(widget.is_for_stream(devel.stream_id));
assert.equal(widget.get_parent(), parent_elem);
assert(checked_mutes);

View File

@@ -22,6 +22,14 @@ var me = {
people.add(me);
people.initialize_current_user(me.user_id);
var social = {
stream_id: 200,
name: 'social',
subscribed: true,
in_home_view: true,
};
stream_data.add_sub('social', social);
var zero_counts = {
private_message_count: 0,
home_unread_messages: 0,
@@ -177,25 +185,11 @@ run_test('changing_subjects', () => {
});
run_test('muting', () => {
stream_data.is_subscribed = function () {
return true;
};
stream_data.in_home_view = function () {
return true;
};
unread.declare_bankruptcy();
var stream_id = 101;
var stream_id = social.stream_id;
var unknown_stream_id = 555;
stream_data.get_sub_by_id = function (stream_id) {
if (stream_id === 101) {
return {name: 'social'};
}
};
var message = {
id: 15,
type: 'stream',
@@ -211,7 +205,7 @@ run_test('muting', () => {
assert.equal(unread.num_unread_for_stream(stream_id), 1);
assert.deepEqual(unread.get_msg_ids_for_stream(stream_id), [message.id]);
muting.add_muted_topic('social', 'test_muting');
muting.add_muted_topic(social.stream_id, 'test_muting');
counts = unread.get_counts();
assert.equal(counts.stream_count.get(stream_id), 0);
assert.equal(counts.home_unread_messages, 0);

View File

@@ -166,7 +166,7 @@ MessageListData.prototype = {
unmuted_messages: function (messages) {
return _.reject(messages, function (message) {
return muting.is_topic_muted(message.stream, message.subject) &&
return muting.is_topic_muted(message.stream_id, message.subject) &&
!message.mentioned;
});
},

View File

@@ -2,50 +2,62 @@ var muting = (function () {
var exports = {};
var muted_topics = new Dict({fold_case: true});
var muted_topics = new Dict();
exports.add_muted_topic = function (stream, topic) {
var sub_dict = muted_topics.get(stream);
exports.add_muted_topic = function (stream_id, topic) {
var sub_dict = muted_topics.get(stream_id);
if (!sub_dict) {
sub_dict = new Dict({fold_case: true});
muted_topics.set(stream, sub_dict);
muted_topics.set(stream_id, sub_dict);
}
sub_dict.set(topic, true);
};
exports.remove_muted_topic = function (stream, topic) {
var sub_dict = muted_topics.get(stream);
exports.remove_muted_topic = function (stream_id, topic) {
var sub_dict = muted_topics.get(stream_id);
if (sub_dict) {
sub_dict.del(topic);
}
};
exports.is_topic_muted = function (stream, topic) {
if (stream === undefined) {
exports.is_topic_muted = function (stream_id, topic) {
if (stream_id === undefined) {
return false;
}
var sub_dict = muted_topics.get(stream);
var sub_dict = muted_topics.get(stream_id);
return sub_dict && sub_dict.get(topic);
};
exports.get_muted_topics = function () {
var topics = [];
muted_topics.each(function (sub_dict, stream) {
muted_topics.each(function (sub_dict, stream_id) {
_.each(sub_dict.keys(), function (topic) {
topics.push([stream, topic]);
// TODO: make it so that callees can work w/stream_id
var stream_name = stream_data.maybe_get_stream_name(stream_id);
if (stream_name) {
topics.push([stream_name, topic]);
}
});
});
return topics;
};
exports.set_muted_topics = function (tuples) {
muted_topics = new Dict({fold_case: true});
muted_topics = new Dict();
_.each(tuples, function (tuple) {
var stream = tuple[0];
var stream_name = tuple[0];
var topic = tuple[1];
exports.add_muted_topic(stream, topic);
var stream_id = stream_data.get_stream_id(stream_name);
if (!stream_id) {
blueslip.warn('Unknown stream in set_muted_topics: ' + stream_name);
return;
}
exports.add_muted_topic(stream_id, topic);
});
};

View File

@@ -154,8 +154,15 @@ exports.set_up_muted_topics_ui = function (muted_topics) {
};
exports.mute = function (stream, topic) {
// TODO: have callers pass in stream_id
var stream_id = stream_data.get_stream_id(stream);
if (!stream_id) {
return;
}
stream_popover.hide_topic_popover();
muting.add_muted_topic(stream, topic);
muting.add_muted_topic(stream_id, topic);
unread_ui.update_unread_counts();
exports.rerender();
exports.persist_mute(stream, topic);
@@ -164,11 +171,18 @@ exports.mute = function (stream, topic) {
};
exports.unmute = function (stream, topic) {
// TODO: have callers pass in stream_id
var stream_id = stream_data.get_stream_id(stream);
if (!stream_id) {
return;
}
// we don't run a unmute_notify function because it isn't an issue as much
// if someone accidentally unmutes a stream rather than if they mute it
// and miss out on info.
stream_popover.hide_topic_popover();
muting.remove_muted_topic(stream, topic);
muting.remove_muted_topic(stream_id, topic);
unread_ui.update_unread_counts();
exports.rerender();
exports.persist_unmute(stream, topic);
@@ -177,7 +191,7 @@ exports.unmute = function (stream, topic) {
};
exports.toggle_mute = function (msg) {
if (muting.is_topic_muted(msg.stream, msg.subject)) {
if (muting.is_topic_muted(msg.stream_id, msg.subject)) {
exports.unmute(msg.stream, msg.subject);
} else if (msg.type === 'stream') {
exports.mute(msg.stream, msg.subject);

View File

@@ -446,7 +446,7 @@ exports.message_is_notifiable = function (message) {
}
if (message.type === "stream" &&
muting.is_topic_muted(message.stream, message.subject)) {
muting.is_topic_muted(message.stream_id, message.subject)) {
return false;
}
@@ -568,7 +568,7 @@ exports.get_local_notify_mix_reason = function (message) {
return;
}
if (message.type === "stream" && muting.is_topic_muted(message.stream, message.subject)) {
if (message.type === "stream" && muting.is_topic_muted(message.stream_id, message.subject)) {
return "Sent! Your message was sent to a topic you have muted.";
}

View File

@@ -380,11 +380,11 @@ exports.toggle_actions_popover = function (element, id) {
var can_mute_topic =
message.stream &&
message.subject &&
!muting.is_topic_muted(message.stream, message.subject);
!muting.is_topic_muted(message.stream_id, message.subject);
var can_unmute_topic =
message.stream &&
message.subject &&
muting.is_topic_muted(message.stream, message.subject);
muting.is_topic_muted(message.stream_id, message.subject);
var should_display_edit_history_option = _.any(message.edit_history, function (entry) {
return entry.prev_content !== undefined;

View File

@@ -142,7 +142,7 @@ function build_topic_popover(e) {
popovers.hide_all();
exports.show_streamlist_sidebar();
var is_muted = muting.is_topic_muted(sub.name, topic_name);
var is_muted = muting.is_topic_muted(sub.stream_id, topic_name);
var can_mute_topic = !is_muted;
var can_unmute_topic = is_muted;

View File

@@ -213,7 +213,7 @@ exports.get_next_topic = function (curr_stream, curr_topic) {
var stream_id = stream_data.get_stream_id(stream_name);
var topics = topic_data.get_recent_names(stream_id);
topics = _.reject(topics, function (topic) {
return muting.is_topic_muted(stream_name, topic);
return muting.is_topic_muted(stream_id, topic);
});
return topics;
}

View File

@@ -86,7 +86,7 @@ exports.widget = function (parent_elem, my_stream_id) {
topic_name: topic_name,
unread: num_unread,
is_zero: num_unread === 0,
is_muted: muting.is_topic_muted(my_stream_name, topic_name),
is_muted: muting.is_topic_muted(my_stream_id, topic_name),
url: hash_util.by_stream_topic_uri(my_stream_name, topic_name),
};
var li = $(templates.render('topic_list_item', topic_info));

View File

@@ -298,7 +298,7 @@ exports.unread_topic_counter = (function () {
per_stream_bucketer.each(function (msgs, topic) {
var topic_count = msgs.count();
res.topic_count.get(stream_id).set(topic, topic_count);
if (!muting.is_topic_muted(sub.name, topic)) {
if (!muting.is_topic_muted(stream_id, topic)) {
stream_count += topic_count;
}
});
@@ -350,7 +350,7 @@ exports.unread_topic_counter = (function () {
var sub = stream_data.get_sub_by_id(stream_id);
per_stream_bucketer.each(function (msgs, topic) {
if (sub && !muting.is_topic_muted(sub.name, topic)) {
if (sub && !muting.is_topic_muted(stream_id, topic)) {
stream_count += msgs.count();
}
});
@@ -382,7 +382,7 @@ exports.unread_topic_counter = (function () {
var topic_lists = [];
var sub = stream_data.get_sub_by_id(stream_id);
per_stream_bucketer.each(function (msgs, topic) {
if (sub && !muting.is_topic_muted(sub.name, topic)) {
if (sub && !muting.is_topic_muted(stream_id, topic)) {
topic_lists.push(msgs.members());
}
});