message_list_data: Store message_list_data objects created locally.

[amanagr]

For #15131

@timabbott here is the list of events for which we will have to update the message_list_data objects we have stored locally:

• muted_topics
• muted_users
• stream update/create/delete events
• delete_message
• update_message events (stream/topic change)

Does the approach look good to you? I implemented what an update would look like for muted_topics.

[timabbott]

I don't think this will be a reliable key, since I'm not sure the order will be consistent. I wonder if we can use the "narrow URL" generated by hash_util as a unique string for a view -- we'd need to take some care to just have a fixed sort order in which operators are emitted into those URLs (E.g. "stream" always before "topic", which I think we might already do? Not sure).

[amanagr]

We use the same operators to create the hash. The order in the hash is the same as in the operators list.

See hash_util.operators_to_hash():

        for (const elem of operators) {
            // Support legacy tuples.
            const operator = elem.operator;
            const operand = elem.operand;

            const sign = elem.negated ? "-" : "";
            hash +=
                "/" +
                sign +
                internal_url.encodeHashComponent(operator) +
                "/" +
                encode_operand(operator, operand);
        }

[timabbott]

This call somewhat duplicates the all_messages_data.remove(message_ids); block at the top of this function.

I think a better approach here might be (1) To add an attribute on MessageListData or something for whether the list is currently rendered, and (2) replace the loop at the top of this function with something conceptually like (I've not thought about whether the generic loop function is the right interface):

message_list_data.for_each((list_data) => list_data.remove(message_ids));

And (3) we'd extend that .remove() function to, if it's rendered, call list.remove_and_rerender() instead.

I'm not sure what the cleanest approach would be, but the overall goal would be we'd be refactoring functions like this to just have a "Update all the message lists" block. I suppose that could end up looking like this; might be cleaner than the idea I suggested above:

message_list_data.for_each((list_data) => {
    if (list_data.is_currently_rendered) {
        list_data.list.remove_and_rerender(message_ids);
    } else {
        list_data.remove(message_ids));
    }
}

[amanagr]

Sounds good!

[zulipbot]

Heads up @amanagr, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.