Commit Graph

6 Commits

Author SHA1 Message Date
Abhijeet Prasad Bodas 7f9c3bb4d0 starred messages: Fix "unstar all in topic" is incomplete.
Currently, when there are some old starred messages
in a topic, it is possible that some of them won't be
unstarred on doing "Unstar all messages in topic".
This happens when those messages haven't been fetched
yet from the server, and we have no way to verify if
they actually belong to the topic.

This commit fixes that by changing this mechanism to
first fetch all starred messages in the topic from
the server, and then send their IDs back to the backend
to unstar them.

While doing this, we assume that the user does not
have more than 1000 starred messages in that topic.

Note that, we still depend on the local data to
decide whether or not the "Unstar all messages in
topic" option should be shown in the topic popover.
A method similar to the above cannot be used here, because
making server requests before opening the popover
could visually slow down the popover opening.

Using local data for the topic popover would probably
not be a big problem, because users would want to
unstar all messages in a topic probably after noticing
that there are a lot of them, meaning there was at least
one starred message from that topic which was already
fetched, which is sufficient for us to conclude that
we need to show the option in the topic popover.

Fixes #17790
2021-04-29 16:43:39 -07:00
Steve Howell 45d806d9aa refactor: Extract message_helper.process_new_message.
We move the message_store.add_message_metadata function
(and all its dependencies) into a new module called
message_helper and rename the function to process_new_message.
(It does a bit more than adding message metadata, such
as updating our message store.)

We also have a "protected" interface now in message_store,
so that message_helper can access the message store:

    update_message_cache
    get_cached_message

Because update_message_cache is identical to
the former create_mock_message, we just renamed it
in the tests.

Most callers should use these functions:

    message_helper.process_new_message (setting)
    message_store.get (getting)

It's slightly annoying that the setter interface
is in a different module than the getter interface,
but that's how you break a bunch of dependencies.

We also extract the tiny message_user_ids class:

    user_ids()
    add_user_ids()

All the code moves here are pretty trivial, and
the code that was moved maintains 100% line
coverage.

The module name `message_helper` is not ideal, but it's a single
function and it'll save time to just do the topology change now and
leave thinking through the right name to later.
2021-03-29 14:53:57 -07:00
Anders Kaseorg 38ffd47b90 js: Convert static/js/page_params.js to ES6 module.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-03-26 10:17:56 -07:00
Anders Kaseorg 84f4229194 node_tests: Remove unnecessary zrequire calls for ES6 modules.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-03-26 10:17:56 -07:00
Abhijeet Prasad Bodas 42aea49784 left sidebar: Add support to unstar all messages in topic.
This adds support for unstarring all (starred)
messages from a particular topic, from the topic
popover.

The earlier implementation of this in #16898
was reverted in bc55aa6a01 (#17429)
because it had two problems-

1. The crash reported in bc55aa6a01
was due to message_store returning undefined. This happens
when the message itself hasn't been fetched from the server
yet, but we know that the message is starred from the ids
in `page_params` in `starred_messages.js`.
This commit handles this case explicitly.
Note that, we simply ignore those messages which
we haven't fetched, and because of this, it may
happen that we don't unstar some messages from that
topic. The correct implementation for this would
be to ask the backend for starred IDs in a topic.

2. The earlier implementation actually unstarred **all**
messages. This was because it grabbed the topic and stream_id
from the topic popover `data` attributes, after the topic
popover had been closed. This passed `undefined`, which
the function then interpreted as an action to unstar all
messages.
With this commit, we use the confirm_dialog widget,
which eliminates the need to store this data in the DOM.
2021-03-23 00:17:15 -07:00
Abhijeet Prasad Bodas 91234b2a86 node tests: Add tests for starred_messages.js. 2021-03-18 15:55:39 -07:00