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
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.
This reverts commit 6fba17599f (#16898).
@chrisbobbe reported this crash:
Uncaught TypeError: Cannot read property 'stream_id' of undefined
at starred_messages.js:43
at Array.filter (<anonymous>)
at Object.e.get_topic_starred_msg_ids (starred_messages.js:40)
at stream_popover.js:221
at HTMLSpanElement.<anonymous> (stream_popover.js:358)
at HTMLUListElement.dispatch (jquery.js:5429)
at HTMLUListElement.v.handle (jquery.js:5233)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Adds a "unstar messages in topic foo" option to the topic sidebar
popover, if there are any starred messages in that topic, known
to the frontend.
Altered existing "unstar all messages" confirmation modal to mention
the topic name, in the case that it was opened by the topic sidebar
codepath.
This is just a v1, and will not unstar old messages from that
topic, if they have not been fetched by the frontend.
Fixes#12194
Co-authored-by: Abhijeet Bodas <abhijeetbodas2001@gmail.com>
ES and TypeScript modules are strict by default and don’t need this
directive. ESLint will remind us to add it to new CommonJS files and
remove it from ES and TypeScript modules.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Prettier would do this anyway, but it’s separated out for a more
reviewable diff. Generated by ESLint.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Prettier would do this anyway, but it’s separated out for a more
reviewable diff. Generated by ESLint.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Prettier would do this anyway, but it’s separated out for a more
reviewable diff. Generated by ESLint.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit was originally automatically generated using `tools/lint
--only=eslint --fix`. It was then modified by tabbott to contain only
changes to a set of files that are unlikely to result in significant
merge conflicts with any open pull request, excluding about 20 files.
His plan is to merge the remaining changes with more precise care,
potentially involving merging parts of conflicting pull requests
before running the `eslint --fix` operation.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
With webpack, variables declared in each file are already file-local
(Global variables need to be explicitly exported), so these IIFEs are
no longer needed.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Send at most 1k message ids in a single read flag request to avoid locking
large number of rows in server database in a single request and avoid long
processing time.
Fixes#11956.
In this commit, I've added a feature to unstar all the starred
messages. This is useful, e.g., for folks who are using starred
messages to keep track of things they should come back when next at
their desktop.
The event flow is the standard one for a feature with a confirmation modal:
(1) User clicks on unstar all messages.
(2) We display a confirmation modal; if the user confirms, we send a
request to the backend to clear all starred messages.
(3) The events system sends that UI update back to us, removing the
stars from the UI.
Fixes#11401.
Apparently, we didn't have one of these, and thus had a moderate
number of generally very old violations in the codebase. Fix this and
clear the ones that exist..
In 47aaa73f96, we fixed one issue, which
is that server_events_dispatch.js was calling `update_starred` with
the wrong arguments, but created a new one (toggle_starred wasn't
updated) and missed another (which is that we weren't ever updating
message.starred, and thus if you toggle a message's star-state in one
browser, and then tried to toggle it back in a second, it would feel
like the click didn't work, because it was trying to toggle
e.g. off->on a second time).
This commit prepares the frontend code to be consumed by webpack.
It is a hack: In theory, modules should be declaring and importing the
modules they depend on and the globals they expose directly.
However, that requires significant per-module work, which we don't
really want to block moving our toolchain to webpack on.
So we expose the modules by setting window.varName = varName; as
needed in the js files.
Fixes#8965.
Mark_message(s)_as_read is used in marking a message as having been
read by the browser, rename it to notify_server_message(s)_read to
avoid any confusion.
We were queuing up individual messages to be flagged as
read on the server before this change, and we used debounce
to avoid sending individual POSTs, but this created delays
that were ripe for race conditions.
Now we batch them in the caller and use throttle instead.
This now prevents us from slamming the server with lots of
individual requests, without as many opportunities for races.
(Note that we still have some possibility of race conditions,
but they should be rare now, and other commits will address
some of the other contributors to read/unread glitches.)
In the JS code, we now use `message.unread` universally as
the indicator of whether a message is unread, rather than
the `message.flags` array that gets passed down to us
from the server.
In particular, we use the unread flag for filtering when
you search.
A lot of this commit is just removing logic to add/remove
"read" from `message.flags` and updating tests.
We also explicitly set `message.unread` to `false` inside of
`unread.mark_as_read()` and no longer have `unread.set_flag()`.
(Some of the callers to `unread.set_flag` were also calling
`unread.mark_as_read`, which was updating the `message`
object, so now we just have `unread.mark_as_read` update
the `message` object. And then unread_ops.mark_all_as_read()
was already calling unread.declare_bankruptcy().)
We now make direct calls to `send_flags_update` for
seting collapsed/uncollapsed, rather than going through
the `batched_updaters` mechanism.
Also the methods in `message_flags` to set
collapsed/uncollapsed now take a single message.
We now do all of the main logic for starring/unstarring
a message in `message_flags.toggle_starred`:
* mark the message as read (just in case)
* update the UI (i.e. the green star in the message)
* update the server
The calling code in both the click handler and the hotkey
handler remains simple--they just handle minor details like
finding the message and clearing popovers.
For updating the server, we now call the new
`send_flag_update` helper.
And we continue to delegate some of the logic to
`ui.update_starred`, but we remove some code there that's
now pushed up to `message_flags.toggle_starred`.
This change should be mostly transparent to users, but it
does remove some inconsistent behaviors between the click
handler and the hotkey handler. Before this change, the
click handler was more aggressive about updating the UI
and marking the message as read. For people using the "*"
key to star/unstar, they probably would only have noticed
different behavior on a slow connection or in an edge
case scenario where only half of the message was onscreen.
More importantly, by simplifying how we talk to the server,
this eliminated up to a one-second lag due to the debounce
logic in the batch_updater code. The complicated debounce
logic is only really needed for batch-updating "read"
messages, and it was overkill and sluggish for starring
messages.
Last but not least, we add defensive code for the local
echo case. (Users have to wait till the message gets acked
to star it.)
Abhijeet Prasad Bodas
Anders Kaseorg
Abhishek-Balaji
Anders Kaseorg
Mohit Gupta
ruchit2801
Tim Abbott
Joshua Pan
Armaan Ahluwalia
Steve Howell
Shubham Padia
Steve Howell
Daw-Ran Liou
Rafid Aslam
Ashish