Before, the message reactions section along with the add reaction button
was being rendered for every message even when there were no reactions
present - this led to additional DOM cost.
This commit adds the message reactions section only when there is
at least a single reaction on the message, and follows up with a cleanup
of the message reactions section when there are no reactions.
Fixes#31137.
Co-authored-by: Anmol-dev45 <basnetanmol2020@gmail.com>
This prevents a deadlock between the thumbnailing worker and message
sending, as follows:
1. A user uploads an image, making Attachment and ImageAttachment
rows, as well as enqueuing a job in the thumbnailing queue.
2. Message sending starts a transaction, creates the Message row,
and calls `do_claim_attachments`, which edits the Attachment row
of the upload (implicitly locking it).
3. The thumbnailing worker starts a transaction, locks the
ImageAttachment row for its image, thumbnails it, and then
attempts to `select_for_update()` the message objects (joined to
the Attachments table) to find the ones which link to the
attachment in question. This query blocks, since "a locking
clause without a table list affects all tables used in the
statement"[^1] and the message-send request already has a write
lock on the Attachments row in question.
4. The message-send request attempts to re-fetch the ImageAttachment
row inside the transaction, which tries to pull a lock on it.
5. Deadlock, because the message-send request has the Attachment
lock, and waits for the ImageAttachment lock; the thumbnailing
worker has the ImageAttachment lock, and waits for the Attachment
lock.
We break this deadlock by limiting the
`update_message_rendered_content` `select_for_update` to only take
the lock on the Message table, and not also the Attachments table --
no changes will be made to the Attachments, so no lock is necessary
there. This allows the thumbnailing worker to successfully pull the
empty list of messages (since the message-send request has not
commits its transaction, and thus the Message row is not visible
yet), and release its ImageAttachment lock so that the message-send
request can proceed.
[^1]: https://www.postgresql.org/docs/current/sql-select.html#SQL-FOR-UPDATE-SHARE
Instead of storing setting pill widgets for new groups
in group_setting_widget_map, we just use variable in the
user_group_create file to store the widget.
This helps in accessing the widget with the key having
"new_group_" as prefix which we want to avoid as a pattern.
Note that the classes and IDs in templates still use
"new_group_" prefix.
This commit refactors get_group_setting_widget_value function
to accept pill widget as parameter instead of setting name.
This is a prep commit for not needing to store the widgets for
group creation form in settings_components.group_setting_widget_map.
This commit refactors set_group_setting_widget_value function
to accept pill widget as parameter instead of setting name.
This is a prep commit for not needing to store the widgets for
group creation form in settings_components.group_setting_widget_map.
This better simulates the Slack API, which is important, since some
integrations check this response and decide whether the Slack endpoint
is working based on what they receive.
The existing text says to post a GitHub comment "saying that you'd
like to work on" the issue. A lot of new contributors,
understandably, take that literally -- they just say they'd like to
work on the issue, with no further information.
In particular they don't give any evidence that they've taken the
steps we prescribe in the preceding section, of figuring out what code
is involved and how they'll approach the problem before they claim it.
When I reply asking for that information, very often they haven't done
those steps... while sometimes they have, and just hadn't put together
from context that that would be a good thing to communicate.
So spell that out explicitly. Hopefully this will elicit smoother
communication from the contributors who have done that work; and
for those who haven't, hopefully it'll help redirect them to go back
and do it.
Also expand on the instruction not to spam.
Previously [{operator: "topic", operand: "one\xa0two"}] would be
unparsed to "topic:one\xa0two" which parses as [{operator: "topic",
operand: "one"}, {operator: "search", operand: "two"}], leading to
exceptions in the search pill system.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Previously, the "stream_topic_history" used to store unacked float
message ids as well in its "max_message_id" of stream and
"message_id" of topic histories.
This commit updates it to rather store only the acked message ids
here, and rather use the "echo_state" module so as to look up
for unacked messages in case of looking for recent topics, or
max message id in functions.
Previously, when `insert_local_message` was called, the
data structures in the `echo_state` are updated with the new
local messages after calling `insert_new_message`. This would
update the stream sidebar before even updating the `echo_state`
with the new local messages.
This commit introduces `track_local_message`, which basically
updates the `echo_state` data structures with the local
message before actually updating the stream sidebar.
This is a preparatory commit to update stream_topic_history
to only contain acked message ids.
The narrow_state module has a function that returns what this needs.
This does remove a log statement, but I don't think it was useful; we
don't need to or have a useful way to colorize a channel that doesn't
exist.
This commit refactors the `ensure_channel_topic_terms` of filter.ts.
Previously, this method used to add channel and topic terms, with
operands as placeholders in case the `with` narrow doesn't have
channel-topic terms.
This commit updates it to rather correct the narrow with the right
terms in case the channel-topic terms are missing in the `with`
narrow, but leave it as it is in case the channel-topic terms are
present, so that it can later be corrected if the channel-topic
terms are not pointing to the right conversation.
Previously, no custom styling was being applied to the enter send
choice options, which led to uneven styling from the other popover
options, as well as the outline ring being cut-off from the edges of
the popover. This commit fixes these issues by adding custom styling
for the outline ring when the enter send choice options are focused.
These files are necessary for the protocol to verify that the file
upload was completed successfully. Rather than delete them, we update
their StorageClass if it is non-STANDARD.
Because the main indexes on end_time either don't include realm_id or
do include subgroup, passing an explicit subgroup=None for
single-realm queries to read CountStats that don't use the subgroups
feature greatly improves the query plans.
This was causing a bug where the participants weren't necessarily
all getting rendered, specifically when there were many subscribers
to a channel, because we render users in batches as buddy list
scrolls, and those users would only show up after some scrolling.
This fix makes sure we always load the participants first.
We create an unnamed user group with just the group creator as it's
member when trying to set the default. The pattern I've followed across
most of the acting_user additions is to just put the user declared
somewhere before the check_add_user_group and see if the test passes.
If it does not, then I'll look at what kind of user it needs to be set
to `acting_user`.
We also add the exception for the group creator to be able to edit their
group in this commit. This exception was added in the backend in earlier
commits.
This commit does not add the logic of using this setting to actually
check the permission on the backend. That will be done in a later
commit.
Only owners can modify this setting, but we will add that logic in a
later commit in order to keep changes in this commit minimal.
Adding the setting breaks the frontend, since the frontend tries to find
a dropdown widget for the setting automatically. To avoid this, we've
added a small temporary if statement to `settings_org.js`.
Although, most lists where we insert this setting follow an unofficial
alphabetical order, `can_manage_all_groups` has been bunched together
with `can_create_groups` since keeping those similar settings together
would be nicer when checking any code related to creating/managing a
user group.
We will not remove `user_group_edit_policy` yet. That will be removed
once we have introduced a user group setting to manage edit permissions
to groups.
We might introduce a generic testing function similar to
do_test_changing_settings_by_owners_only later, but not right now, since
there is only 1 setting at the moment needing that test.
This commit does not add the logic of using this setting to actually
check the permission on the backend. That will be done in a later
commit.
Adding the setting breaks the frontend, since the frontend tries to find
a dropdown widget for the setting automatically. To avoid this, we've
added a small temporary if statement to `settings_org.js`.
Shubham Padia
Sayam Samal
Anders Kaseorg
Alex Vandiver
Sahil Batra
Pierre Carru
Greg Price
Aman Agrawal
Lauryn Menard
roanster007
Tim Abbott
evykassirer
opmkumar