This locks the message row while a reaction is being added/removed,
which will handle race conditions caused by deleting the message
at the same time.
We make sure that events work happens outside the transaction,
so that in case there's some problem with the queue processor, the
locks aren't held for too long.
As a nice side-effect, we also handle race conditions from double
adding reactions, because once the message is locked, a duplicate
request will wait till the earlier transaction commits, and hence
will not throw `IntegrityErrors`s (rather, will be handled in our
safety check in the /views code itself), which earlier had to be
handled explicitly.
This is will make it easier to systematically use Django's
`capturOnCommitCallbacks` in tests outside of the main
`test_events` file which involve assertions on events.
Convert this function that absolutely makes a stream web public.
We already have do_change_stream_invite_only to convert
streams to public and private streams.
We also update all the fields that should be set when a stream
is made web public.
Previously, if a user subscribed to a stream with
history_public_to_subscribers, and then was looking at old messages in
the stream, they would not get live-updates for that stream, because
of the structure in how notify_reaction_update only looked at
UserMessage rows (we had a previous workaround involving the
`historical` field in `UserMessage` which had already made it work if
the user themselves added the reaction).
We fix this by including all subscribers with history access in the
set of recipients for update events.
Fixes a bug that was confused with #16942.
Zulip converts :) to the 1F642 Unicode emoji and promotes the same emoji
in the popular section of the emoji picker.
Previously Zulip has labeled 1F642 as "slight smile". While that name
conforms to the Unicode standard (which describes the code point as
SLIGHTLY SMILING FACE), it didn't match our use case of the emoji.
If a user types :) or selects the first smile in the emoji picker they
probably mean to express a regular "smile" and not a "slight smile",
which raises the question why they are only smiling slightly.
This commit relabels 1F642 as 😄 and our previous 😄 263A as
:smiling_face:. Note that 263A looks different in our three supported
emoji sets, so it is not suited to be our "default smile".
This change does not require a migration since our emoji system stores
both unicode points and names and handles name changes transparently.
This fixes up some complex helpers that may
have had some value before f-strings come along,
but they mostly obscured the logic for
people reading the tests.
We still keep really simple helpers for the
common cases, but there are no optional
parameters for them.
One goal of this fix is to remove the
short_name concept, and we just explicitly
set senders everywhere we need them.
We also now have each test just explicitly set
its reaction_type.
For cases where we have custom message ids
or senders, we just inline the simple call
to api_post.
Fixes#2665.
Regenerated by tabbott with `lint --fix` after a rebase and change in
parameters.
Note from tabbott: In a few cases, this converts technical debt in the
form of unsorted imports into different technical debt in the form of
our largest files having very long, ugly import sequences at the
start. I expect this change will increase pressure for us to split
those files, which isn't a bad thing.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Automatically generated by the following script, based on the output
of lint with flake8-comma:
import re
import sys
last_filename = None
last_row = None
lines = []
for msg in sys.stdin:
m = re.match(
r"\x1b\[35mflake8 \|\x1b\[0m \x1b\[1;31m(.+):(\d+):(\d+): (\w+)", msg
)
if m:
filename, row_str, col_str, err = m.groups()
row, col = int(row_str), int(col_str)
if filename == last_filename:
assert last_row != row
else:
if last_filename is not None:
with open(last_filename, "w") as f:
f.writelines(lines)
with open(filename) as f:
lines = f.readlines()
last_filename = filename
last_row = row
line = lines[row - 1]
if err in ["C812", "C815"]:
lines[row - 1] = line[: col - 1] + "," + line[col - 1 :]
elif err in ["C819"]:
assert line[col - 2] == ","
lines[row - 1] = line[: col - 2] + line[col - 1 :].lstrip(" ")
if last_filename is not None:
with open(last_filename, "w") as f:
f.writelines(lines)
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This changes add_reaction in zerver.views.reactions to allow
calling POST ../messages/{message_id}/reactions api endpoint with
emoji_name only, even in the case of a custom emoji.
Generated by autopep8, with the setup.cfg configuration from #14532.
I’m not sure why pycodestyle didn’t already flag these.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Generated by `pyupgrade --py3-plus --keep-percent-format` on all our
Python code except `zthumbor` and `zulip-ec2-configure-interfaces`,
followed by manual indentation fixes.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
We try to use the correct variation of `email`
or `delivery_email`, even though in some
databases they are the same.
(To find the differences, I temporarily hacked
populate_db to use different values for email
and delivery_email, and reduced email visibility
in the zulip realm to admins only.)
In places where we want the "normal" realm
behavior of showing emails (and having `email`
be the same as `delivery_email`), we use
the new `reset_emails_in_zulip_realm` helper.
A couple random things:
- I fixed any error messages that were leaking
the wrong email
- a test that claimed to rely on the order
of emails no longer does (we sort user_ids
instead)
- we now use user_ids in some place where we used
to use emails
- for IRC mirrors I just punted and used
`reset_emails_in_zulip_realm` in most places
- for MIT-related tests, I didn't fix email
vs. delivery_email unless it was obvious
I also explicitly reset the realm to a "normal"
realm for a couple tests that I frankly just didn't
have the energy to debug. (Also, we do want some
coverage on the normal case, even though it is
"easier" for tests to pass if you mix up `email`
and `delivery_email`.)
In particular, I just reset data for the analytics
and corporate tests.
This reduces query counts in some cases, since
we no longer need to look up the user again. In
particular, it reduces some noise when we
count queries for O(N)-related tests.
The query count is usually reduced by 2 per
API call. We no longer need to look up Realm
and UserProfile. In most cases we are saving
these lookups for the whole tests, since we
usually already have the `user` objects for
other reasons. In a few places we are simply
moving where that query happens within the
test.
In some places I shorten names like `test_user`
or `user_profile` to just be `user`.
This commit mostly makes our tests less
noisy, since emails are no longer an important
detail of sending messages (they're not even
really used in the API).
It also sets us up to have more scrutiny
on delivery_email/email in the future
for things that actually matter. (This is
a prep commit for something along those
lines, kind of hard to explain the full
plan.)
The original/legacy emoji reactions endpoints made use of HTTP PUT and
didn't have an API that could correctly handle situations where the
emoji names change over time. We stopped using the legacy endpoints
some time ago, so we can remove them now.
This requires straightforward updates to older tests that were still
written against the legacy API.
Fixes#12940.
MigrationsTestCase is intentionally omitted from this, since migrations
tests are different in their nature and so whatever setUp()
ZulipTestCase may do in the future, MigrationsTestCase may not
necessarily want to replicate.
Given that we allow adding emoji reactions by only using the
emoji_name, we should offer the same possibility for removing
reactions to make the experience for API clients not require looking
up emoji codes.
Since this is an additional optional parameter, this also preserves
backward compatibility.
Complete, correct implementations of Zulip's emoji reactions API need
to send both emoji_code and emoji_name in order to add a reaction;
this is important for corner cases around clicking on a reaction in a
message that was first reacted to a year ago, when the emoji
name->code mappings have changed for the given code point in the
intervening time.
However, for folks building tools using the Zulip API, that corner
case is not particularly common; as a result, it makes sense to offer
an interface that allows adding a reaction by only specifying the
emoji name.
This is why the only field that needs to be required is emoji_name,
which can now be mapped to a single emoji. Both fields will be
necessary when "voting" an old reaction, but since we stil allow
specifying the two of them, these changes offer retrocompatibility.
This commit migrates realm emoji to be addressed by their `id` rather
than their name. This fixes a long standing issue which was causing
an error on uploading an emoji with same name as a deactivated realm
emoji.
Fixes: #6977.
Till now, we had been storing realm emoji's name in emoji code field
in reactions' model. This commit migrates it to store realm emoji's id.
It is a part of effort to migrate realm emojis to be referenced by their
id and not by name.
Inorder to provide more explicit error messages I have merged the
`emoji_code_is_valid()` and `emoji_name_is_valid()` functions into
`check_emoji_code_consistency()` and `check_emoji_name_consistency()`
respectively.
This endpoint will allow us to add/delete emoji reactions whose emoji
got renamed during various emoji infra changes. This was also a
required change for realm emoji migration.
This commit was tweaked significantly by tabbott for greater clarity
(with no changes to the actual logic).
On receiving a request for deleting a reaction, just check if such
a reaction exists or not. If it exists then just delete the reaction
otherwise send an error message that such a reaction doesn't exist.
It doesn't make sense to check whether an emoji name is valid or not.
Abhijeet Prasad Bodas
Aman Agrawal
Anders Kaseorg
Mateusz Mandera
LoopThrough-i-j
Gittenburg
Steve Howell
Anders Kaseorg
akashaviator
Steve Howell
Gloria Elston
Mateusz Mandera
Anders Kaseorg
Yago González
Aditya Bansal
Harshit Bansal
Tim Abbott
Alena Volkova
rht