mirror of https://github.com/zulip/zulip.git
Fix recent regressions with All Messages (muted topics).
My recent refactoring that split out MessageListData introduced a nasty bug where we were putting muted messages into the "All Messages" view even though the underlying list was correctly filtering them, so the symptoms were two-fold: - muted messages cluttered up your feed - replying to the message caused a traceback (since it wasn't actually in the underlying data structure) This has to do with what MessageListData.add_messages() was passing back to MessageList to orchestrate drawing in MessageListView. I think what happened here is I got this working kind of sloppily but correctly for the non-muting case and then got in the weeds of some other stuff. Not my finest moment. The actual correct code here is simple enough. We triage top, interior, and bottom, and then the respective methods that put the data into the data structure return the filtered lists (i.e. not muted) and put them into the info structure. Fixes #9656
This commit is contained in:
parent
f625d4d237
commit
a361e2b993
|
@ -144,7 +144,16 @@ run_test('more muting', () => {
|
|||
{id: 8},
|
||||
];
|
||||
|
||||
mld.add_messages(orig_messages);
|
||||
const orig_info = mld.add_messages(orig_messages);
|
||||
|
||||
assert.deepEqual(orig_info, {
|
||||
top_messages: [],
|
||||
interior_messages: [],
|
||||
bottom_messages: [
|
||||
{id: 4},
|
||||
{id: 8},
|
||||
],
|
||||
});
|
||||
|
||||
assert.deepEqual(
|
||||
_.pluck(mld._all_items, 'id'),
|
||||
|
@ -165,7 +174,8 @@ run_test('more muting', () => {
|
|||
{id: 9, subject: 'muted'},
|
||||
{id: 10},
|
||||
];
|
||||
mld.add_messages(more_messages);
|
||||
|
||||
const more_info = mld.add_messages(more_messages);
|
||||
|
||||
assert.deepEqual(
|
||||
_.pluck(mld._all_items, 'id'),
|
||||
|
@ -177,6 +187,18 @@ run_test('more muting', () => {
|
|||
[2, 4, 6, 8, 10]
|
||||
);
|
||||
|
||||
assert.deepEqual(more_info, {
|
||||
top_messages: [
|
||||
{id: 2},
|
||||
],
|
||||
interior_messages: [
|
||||
{id: 6},
|
||||
],
|
||||
bottom_messages: [
|
||||
{id: 10},
|
||||
],
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
run_test('errors', () => {
|
||||
|
|
|
@ -254,19 +254,8 @@ MessageListData.prototype = {
|
|||
});
|
||||
}
|
||||
|
||||
// We return probably more info than our actual callers
|
||||
// need, but this is useful for tests and not actually
|
||||
// expensive.
|
||||
var info = {
|
||||
top_messages: top_messages,
|
||||
bottom_messages: bottom_messages,
|
||||
interior_messages: interior_messages,
|
||||
};
|
||||
|
||||
if (interior_messages.length > 0) {
|
||||
var all_messages = top_messages.concat(interior_messages).concat(bottom_messages);
|
||||
self.add_anywhere(all_messages);
|
||||
return info;
|
||||
interior_messages = self.add_anywhere(interior_messages);
|
||||
}
|
||||
|
||||
if (top_messages.length > 0) {
|
||||
|
@ -277,6 +266,12 @@ MessageListData.prototype = {
|
|||
bottom_messages = self.append(bottom_messages);
|
||||
}
|
||||
|
||||
var info = {
|
||||
top_messages: top_messages,
|
||||
bottom_messages: bottom_messages,
|
||||
interior_messages: interior_messages,
|
||||
};
|
||||
|
||||
return info;
|
||||
},
|
||||
|
||||
|
@ -294,6 +289,7 @@ MessageListData.prototype = {
|
|||
this._items = viewable_messages.concat(this._items);
|
||||
|
||||
} else {
|
||||
viewable_messages = messages;
|
||||
this._items = messages.concat(this._items);
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue