message lists: Exclude 1:1 PMs with muted users.

* We hide 1:1 PMs from and to muted users throughout
the UI, because doing so will not lead to loss of
conversational context. The "to" part is also important,
because the last few messages sent to a user before
muting them would probably be asking them to stop
spamming.

* After this change, we will need to do filtering for either
user or topic muting in pretty much all narrows, so we need
to keep the `_all_items` list in MessageListData always
up-to-date.

* A further commit will relax this and make it possible to
view these messages only when in a `pm-with/muted_user`
narrow.
This commit is contained in:
Abhijeet Prasad Bodas
2021-05-08 02:04:38 +05:30
committed by Tim Abbott
parent e64e5936ce
commit a59f545136
5 changed files with 103 additions and 50 deletions

View File

@@ -114,6 +114,14 @@ run_test("muting", () => {
{id: 1, type: "stream", stream_id: 1, topic: "muted"}, {id: 1, type: "stream", stream_id: 1, topic: "muted"},
{id: 2, type: "stream", stream_id: 1, topic: "whatever"}, {id: 2, type: "stream", stream_id: 1, topic: "whatever"},
{id: 3, type: "stream", stream_id: 1, topic: "muted", mentioned: true}, // mentions override muting {id: 3, type: "stream", stream_id: 1, topic: "muted", mentioned: true}, // mentions override muting
// 10 = muted user, 9 = non-muted user, 11 = you
{id: 4, type: "private", to_user_ids: "9,10,11", sender_id: 10}, // muted to huddle
{id: 5, type: "private", to_user_ids: "9,10,11", sender_id: 9}, // non-muted to huddle
{id: 6, type: "private", to_user_ids: "11", sender_id: 10}, // muted to 1:1 PM
{id: 7, type: "private", to_user_ids: "11", sender_id: 9}, // non-muted to 1:1 PM
{id: 8, type: "private", to_user_ids: "10", sender_id: 11}, // 1:1 PM to muted
{id: 9, type: "private", to_user_ids: "9", sender_id: 11}, // 1:1 PM to non-muted
]; ];
// `messages_filtered_for_topic_mutes` should skip filtering // `messages_filtered_for_topic_mutes` should skip filtering
@@ -132,42 +140,84 @@ run_test("muting", () => {
}, },
); );
// Test actual behaviour of `messages_filtered_for_topic_mutes` // Test actual behaviour of `messages_filtered_for_*` methods.
mld.excludes_muted_topics = true; mld.excludes_muted_topics = true;
muting.add_muted_topic(1, "muted"); muting.add_muted_topic(1, "muted");
const res = mld.messages_filtered_for_topic_mutes(msgs); const res = mld.messages_filtered_for_topic_mutes(msgs);
assert.deepEqual(res, [ assert.deepEqual(res, [
{id: 2, type: "stream", stream_id: 1, topic: "whatever"}, {id: 2, type: "stream", stream_id: 1, topic: "whatever"},
{id: 3, type: "stream", stream_id: 1, topic: "muted", mentioned: true}, // mentions override muting {id: 3, type: "stream", stream_id: 1, topic: "muted", mentioned: true}, // mentions override muting
// `messages_filtered_for_topic_mutes` does not affect private messages
{id: 4, type: "private", to_user_ids: "9,10,11", sender_id: 10},
{id: 5, type: "private", to_user_ids: "9,10,11", sender_id: 9},
{id: 6, type: "private", to_user_ids: "11", sender_id: 10},
{id: 7, type: "private", to_user_ids: "11", sender_id: 9},
{id: 8, type: "private", to_user_ids: "10", sender_id: 11},
{id: 9, type: "private", to_user_ids: "9", sender_id: 11},
]); ]);
muting.add_muted_user(10);
const res_user = mld.messages_filtered_for_user_mutes(msgs);
assert.deepEqual(res_user, [
// `messages_filtered_for_user_mutes` does not affect stream messages
{id: 1, type: "stream", stream_id: 1, topic: "muted"},
{id: 2, type: "stream", stream_id: 1, topic: "whatever"},
{id: 3, type: "stream", stream_id: 1, topic: "muted", mentioned: true},
{id: 4, type: "private", to_user_ids: "9,10,11", sender_id: 10}, // muted to huddle
{id: 5, type: "private", to_user_ids: "9,10,11", sender_id: 9}, // non-muted to huddle
{id: 7, type: "private", to_user_ids: "11", sender_id: 9}, // non-muted to 1:1 PM
{id: 9, type: "private", to_user_ids: "9", sender_id: 11}, // 1:1 PM to non-muted
]);
// Output filtered based on both topic and user muting.
mld._all_items = msgs;
const filtered_messages = mld.unmuted_messages(mld._all_items);
assert.deepEqual(filtered_messages, [
{id: 2, type: "stream", stream_id: 1, topic: "whatever"},
{id: 3, type: "stream", stream_id: 1, topic: "muted", mentioned: true},
{id: 4, type: "private", to_user_ids: "9,10,11", sender_id: 10},
{id: 5, type: "private", to_user_ids: "9,10,11", sender_id: 9},
{id: 7, type: "private", to_user_ids: "11", sender_id: 9},
{id: 9, type: "private", to_user_ids: "9", sender_id: 11},
]);
// Also verify that, the correct set of messages is stored in `_items`
// once we update the list for muting.
mld.update_items_for_muting();
assert.deepEqual(filtered_messages, mld._items);
// MessageListData methods should always attempt to filter messages, // MessageListData methods should always attempt to filter messages,
// and update `_all_items` when `excludes_muted_topics` is true. // and keep `_all_items` up-to-date.
mld = new MessageListData({ mld = new MessageListData({
excludes_muted_topics: true, excludes_muted_topics: true,
}); });
assert.deepEqual(mld._all_items, []); assert.deepEqual(mld._all_items, []);
let messages_filtered_for_topic_mutes_calls = 0; let unmuted_messages_calls = 0;
mld.messages_filtered_for_topic_mutes = (messages) => { mld.unmuted_messages = (messages) => {
messages_filtered_for_topic_mutes_calls = messages_filtered_for_topic_mutes_calls + 1; unmuted_messages_calls = unmuted_messages_calls + 1;
return messages; return messages;
}; };
mld.add_anywhere([{id: 10}]); mld.add_anywhere([{id: 10}, {id: 20}]);
assert.equal(messages_filtered_for_topic_mutes_calls, 1); assert.equal(unmuted_messages_calls, 1);
assert_msg_ids(mld._all_items, [10]); assert_msg_ids(mld._all_items, [10, 20]);
mld.prepend([{id: 9}]); mld.prepend([{id: 9}, {id: 19}]);
assert.equal(messages_filtered_for_topic_mutes_calls, 2); assert.equal(unmuted_messages_calls, 2);
assert_msg_ids(mld._all_items, [9, 10]); assert_msg_ids(mld._all_items, [9, 19, 10, 20]);
mld.append([{id: 11}]); mld.append([{id: 11}, {id: 21}]);
assert.equal(messages_filtered_for_topic_mutes_calls, 3); assert.equal(unmuted_messages_calls, 3);
assert_msg_ids(mld._all_items, [9, 10, 11]); assert_msg_ids(mld._all_items, [9, 19, 10, 20, 11, 21]);
mld.remove([9]); mld.remove([9]);
assert_msg_ids(mld._all_items, [10, 11]); assert_msg_ids(mld._all_items, [19, 10, 20, 11, 21]);
mld.reorder_messages(20);
assert_msg_ids(mld._all_items, [10, 11, 19, 20, 21]);
mld.clear(); mld.clear();
assert_msg_ids(mld._all_items, []); assert_msg_ids(mld._all_items, []);

View File

@@ -179,9 +179,9 @@ run_test("is private with no target", () => {
}, },
has_found_newest: true, has_found_newest: true,
all_messages: [ all_messages: [
{id: 450, type: "private"}, {id: 450, type: "private", to_user_ids: "1,2"},
{id: 500, type: "private"}, {id: 500, type: "private", to_user_ids: "1,2"},
{id: 550, type: "private"}, {id: 550, type: "private", to_user_ids: "1,2"},
], ],
expected_id_info: { expected_id_info: {
target_id: undefined, target_id: undefined,
@@ -244,9 +244,9 @@ run_test("is:private with target and no unreads", () => {
empty: false, empty: false,
all_messages: [ all_messages: [
{id: 350}, {id: 350},
{id: 400, type: "private"}, {id: 400, type: "private", to_user_ids: "1,2"},
{id: 450, type: "private"}, {id: 450, type: "private", to_user_ids: "1,2"},
{id: 500, type: "private"}, {id: 500, type: "private", to_user_ids: "1,2"},
], ],
expected_id_info: { expected_id_info: {
target_id: 450, target_id: 450,

View File

@@ -395,6 +395,9 @@ export class MessageList {
// to do this is in the message_events.js code path for // to do this is in the message_events.js code path for
// processing topic edits, since that's the only place we'll // processing topic edits, since that's the only place we'll
// call this frequently anyway. // call this frequently anyway.
//
// But in any case, we need to rerender the list for user muting,
// to make sure only the right messages are hidden.
this.rerender(); this.rerender();
} }

View File

@@ -11,9 +11,7 @@ import * as util from "./util";
export class MessageListData { export class MessageListData {
constructor({excludes_muted_topics, filter = new Filter()}) { constructor({excludes_muted_topics, filter = new Filter()}) {
this.excludes_muted_topics = excludes_muted_topics; this.excludes_muted_topics = excludes_muted_topics;
if (this.excludes_muted_topics) { this._all_items = [];
this._all_items = [];
}
this._items = []; this._items = [];
this._hash = new Map(); this._hash = new Map();
this._local_only = new Set(); this._local_only = new Set();
@@ -109,10 +107,7 @@ export class MessageListData {
} }
clear() { clear() {
if (this.excludes_muted_topics) { this._all_items = [];
this._all_items = [];
}
this._items = []; this._items = [];
this._hash.clear(); this._hash.clear();
} }
@@ -182,14 +177,29 @@ export class MessageListData {
}); });
} }
messages_filtered_for_user_mutes(messages) {
return messages.filter((message) => {
if (message.type !== "private") {
return true;
}
const recipients = util.extract_pm_recipients(message.to_user_ids);
if (recipients.length > 1) {
// Huddle message
return true;
}
const recipient_id = Number.parseInt(recipients[0], 10);
return !muting.is_user_muted(recipient_id) && !muting.is_user_muted(message.sender_id);
});
}
unmuted_messages(messages) { unmuted_messages(messages) {
return this.messages_filtered_for_topic_mutes(messages); return this.messages_filtered_for_topic_mutes(
this.messages_filtered_for_user_mutes(messages),
);
} }
update_items_for_muting() { update_items_for_muting() {
if (!this.excludes_muted_topics) {
return;
}
this._items = this.unmuted_messages(this._all_items); this._items = this.unmuted_messages(this._all_items);
} }
@@ -264,10 +274,8 @@ export class MessageListData {
const viewable_messages = this.unmuted_messages(messages); const viewable_messages = this.unmuted_messages(messages);
if (this.excludes_muted_topics) { this._all_items = messages.concat(this._all_items);
this._all_items = messages.concat(this._all_items); this._all_items.sort((a, b) => a.id - b.id);
this._all_items.sort((a, b) => a.id - b.id);
}
this._items = viewable_messages.concat(this._items); this._items = viewable_messages.concat(this._items);
this._items.sort((a, b) => a.id - b.id); this._items.sort((a, b) => a.id - b.id);
@@ -280,9 +288,7 @@ export class MessageListData {
// Caller should have already filtered // Caller should have already filtered
const viewable_messages = this.unmuted_messages(messages); const viewable_messages = this.unmuted_messages(messages);
if (this.excludes_muted_topics) { this._all_items = this._all_items.concat(messages);
this._all_items = this._all_items.concat(messages);
}
this._items = this._items.concat(viewable_messages); this._items = this._items.concat(viewable_messages);
this._add_to_hash(messages); this._add_to_hash(messages);
@@ -293,9 +299,7 @@ export class MessageListData {
// Caller should have already filtered // Caller should have already filtered
const viewable_messages = this.unmuted_messages(messages); const viewable_messages = this.unmuted_messages(messages);
if (this.excludes_muted_topics) { this._all_items = messages.concat(this._all_items);
this._all_items = messages.concat(this._all_items);
}
this._items = viewable_messages.concat(this._items); this._items = viewable_messages.concat(this._items);
this._add_to_hash(messages); this._add_to_hash(messages);
@@ -310,9 +314,7 @@ export class MessageListData {
} }
this._items = this._items.filter((msg) => !msg_ids_to_remove.has(msg.id)); this._items = this._items.filter((msg) => !msg_ids_to_remove.has(msg.id));
if (this.excludes_muted_topics) { this._all_items = this._all_items.filter((msg) => !msg_ids_to_remove.has(msg.id));
this._all_items = this._all_items.filter((msg) => !msg_ids_to_remove.has(msg.id));
}
} }
// Returns messages from the given message list in the specified range, inclusive // Returns messages from the given message list in the specified range, inclusive
@@ -501,9 +503,7 @@ export class MessageListData {
) { ) {
blueslip.debug("Changed message ID from server caused out-of-order list, reordering"); blueslip.debug("Changed message ID from server caused out-of-order list, reordering");
this._items.sort(message_sort_func); this._items.sort(message_sort_func);
if (this.excludes_muted_topics) { this._all_items.sort(message_sort_func);
this._all_items.sort(message_sort_func);
}
return true; return true;
} }

View File

@@ -157,9 +157,9 @@ export function unmute_user(user_id) {
} }
export function rerender_for_muted_user() { export function rerender_for_muted_user() {
message_lists.current.rerender(); message_lists.current.update_muting_and_rerender();
if (message_lists.current !== message_lists.home) { if (message_lists.current !== message_lists.home) {
message_lists.home.rerender(); message_lists.home.update_muting_and_rerender();
} }
if (overlays.settings_open() && settings_muted_users.loaded) { if (overlays.settings_open() && settings_muted_users.loaded) {