Previously, users found it annoying that the automated "Resolve topic"
notifications triggered an unread for everyone in the stream; this
discouraged some users from using the feature on older threads for
fear of being annoying. We change this to a better default, of only
users who participated in the topic (via either messages or reactions)
being eligible for the new message being unread.
We will likely want to create global and stream-level notifications
settings to control this behavior as a follow-up -- some users, like
me, might prefer the simpler "Always unread" behavior in some streams.
Note that the automated notifications that a topic was resolved will
still result in the topic being moved to the top of the left sidebar.
This would be somewhat difficult to change, since the left sidebar
algorithm just looks at the highest message ID in the topic.
Fixes#19709.
Tests added by Aman Agrawal (amanagr@zulip.com).
In English, compound adjectives should essentially always be
hyphenated. This makes them easier to parse, especially for users who
might not recognize that the words “web public” go together as a
phrase.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
There are three user settings that are integer enums:
`color_scheme`, `demote_inactive_streams` and
`desktop_icon_count_displays`.
Unlike the other user settings, these were using the `content`
keyword instead of the `schema` keyword in their definitions,
which caused them not to be rendered correctly in the api
documentation.
Changes the keyword to `schema` and fixes the indentation
for these three user settings in the two endpoints using
them.
Removes various instances of quotation marks that are not needed,
specifically looking at instances of arrays, e.g. `- "`, in the
OpenAPI documentation.
The target realm was not being passed to create_attachment in
upload_message_file implementations. This was a bug in the edge-case of
cross-realm messages - in particular, causing a bug in the email
gateway:
When an email with an attachment is sent, the message is mirrored to
Zulip with Email Gateway Bot as the message sender and uploader of the
attachment. Due to the realm not being passed to create_attachment, the
Attachment would get created with .realm being the system bot realm,
making the attachment inaccessible under some conditions due to failing
the following condition check (that's expected to pass, provided that
the .realm is set correctly):
```
if (
attachment.is_realm_public
and attachment.realm == user_profile.realm
and user_profile.can_access_public_streams()
):
# Any user in the realm can access realm-public files
return True
```
Fixes the rendering of enums to show strings with quotation marks,
while integers will continue to be rendered without quotation marks.
This allows for an empty string to be passed as an enum value and be
rendered as such in the documentation. Null will be rendered without
quotation marks, like integer values.
Makes `edit_timestamp` and `user_id` required fields for all
`update_message` events.
Adds `rendering_only` as another required field to signal if
events are only updating the rendered content of the message,
which is currently the case for adding inline url previews.
Updates `test_event.py` so that `do_update_message` and
`do_update_embedded_data` refer to the same testing schema
for `update_message` events, and therefore reflect the same
required fields for the `update_message` event.
The OpenAPI definition for `update_message` events is also
updated to reflect the required field and descriptions of
various properties are updated for the addition of the
`rendering_only` property.
Moves details about the rate limit error object and handling to
the OpenAPI documentation description for that common error.
Previously, this information was on the general rest error
handling documentation page without clear connection to the
specific rate limit error.
Fixes a typo in the changelog (feature 36) for that same error
and also fixes a misplaced colon in the description of the error
for missing request parameters.
Adds detailed definition of objects in the `subscription_data` parameter
array for the `/update-subscription-settings` endpoint.
Fixes#20825. Follow-up to #20409.
Formats and moves whether a field of an object in a request
parameter is required or optional to be in the same location
and have the same formatting as the general api parameter
documentation.
Also formats any examples within the object detailed
description to be the same as the general api parameter
documentation.
Follow up to #20409.
Adds a line break before the descriptive text for return
values and events in the api documentation in order to
help with readability of descriptions with multiple
paragraphs of descriptive text.
Adjustments made to the CSS of list items in unordered
lists to visually group the first paragraph of text
to any following paragraphs or unordered lists.
As a consequence:
• Bump minimum supported Python version to 3.7.
• Move Vagrant environment to Debian 10, which has Python 3.7.
• Move CI frontend tests to Debian 10.
• Move production build test to Debian 10.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
regarding -
POST https://yourZulipDomain.zulipchat.com/api/v1/users/me/subscriptions
The definition of the "subscription" parameter didn't include full
information about the parameter. It only said that an array of objects
is passed as a parameter, and relied on description of the parameter
to explain what the object contained. I edited the definition to contain
the full information about the object.
Fixes#20824.
The change to curl_param_value_generators.py warrants a brief
explanation. Stream permission changes now generate a notification
message. Our curl example test for removing a reaction comes after
the two tests for updating the stream permission changes, thus the
hardcoded message ID in that test needs to be incremented by 2 to
account for the two notification messages that now come before it.
This is a part of #20289.
do_make_stream_web_public and do_change_stream_invite_only seem
to contain very similar logic that could just live inside the
do_change_stream_permission function that handles all permission
changes in one place.
queue_client.queues does not list all the queues that exist on the
server (you can’t do that over AMQP); the condition "test_suite" in
queue_client.queues was always false. So the test_suite queue could
accumulate extra messages that broke test_queue_error_json.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This also fixes a warning from
RealmExportTest.test_endpoint_local_uploads: “ResourceWarning:
unclosed file <_io.BufferedReader
name='/srv/zulip/var/…/test-export.tar.gz'>”.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Adds a check for object in parameter type that will render the details
of the object in the parameter description if they are in the object
definition in the OpenAPI documentation.
Fixes#19424.
revoke_invites_generated_by_user should send invites_changed event if it
actually revokes some invitations. This is called in the user
deactivatoin codepath.
Event of type "realm_user", op "remove", emitted by do_deactivate_user
should remove the user id from subscriptions in the state. We weren't
catching this bug, because test_do_deactivate_bot uses a newly created
bot, so no stream subscriptions are affected. The bug shows up if
deactivating e.g. cordelia - thus we want to have two tests instead,
one for testing bot deactivation and one for user deactivation.
We now use recipient_id % 24 for new stream colors
when users have already used all 24 of our canned
colors.
This fix doesn't address the scenario that somebody
dislikes one of our current canned colors, so if a
user continually changes canned color N to some other
color for new streams, their new streams will continue
to include color N (and the user will still need to
change them).
This fix doesn't address the fact that it can be expensive
during bulk-add situations to query for all the colors
that users have already used up.
See https://chat.zulip.org/#narrow/stream/3-backend/topic/assigning.20stream.20colors
for more discussion.
The limit here is purely to prevent breakage in case of a pathological
number of images in a single message; 5 images is entirely possible in
a reasonable message, and causes user confusion when they are not
expended.
Increase the limit to 10 per message.
Django 3.2 expects a list, and Django 4.1 will require one. Fixes
“RemovedInDjango41Warning: Using a boolean value for
requires_system_checks is deprecated. Use '__all__' instead of True,
and [] (an empty list) instead of False.”
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This was deprecated in Django 3.1 for being jQuery-specific, and
removed in Django 4.0. Replicate the jQuery-specific check.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
oneOf with two identical branches (modulo example) is a bug because
oneOf means exclusive or. It’s also a totally inappropriate kludge
for encoding multiple examples. The OpenAPI specification provides a
perfectly good standard way to do that:
https://spec.openapis.org/oas/v3.0.3#example-object
However, we don’t handle that in our OpenAPI documentation generator
yet, so for now just merge the examples.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This was a oneOf with two identical branches modulo example, which is
always a bug because oneOf means exclusive or. But the example for
the first branch did not fit the schema for AddSubscriptionsResponse,
which is a subset of JsonSuccessBase.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Although allOf is often used to indicate inheritance, its semantics
are that of a plain set intersection. The intersection of a nullable
property with a non-nullable property is a non-nullable property.
Therefore, if we want an inherited property to remain nullable, we
need to mark it as such.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
django.utils.translation.ugettext is a deprecated alias of
django.utils.translation.gettext as of Django 3.0, and will be removed
in Django 4.0.
Commit e7ed907cf6 (#18174) fixed this
before, but new instances have been added.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We do not accept heterogeneous arrays containing both user ids and
email addresses.
This also happens to disallow an empty array, which is fine since the
principals parameter should be omitted if the default to the calling
user is desired.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Fixes “DeprecationWarning: 'jinja2.Markup' is deprecated and will be
removed in Jinja 3.1. Import 'markupsafe.Markup' instead.”
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Updates regex in the openapi markdown extension to match api
endpoint names that contain dashes, which is the case for
`zulip-outgoing-webhook` and `rest-error-handling`.
The subscriber list was not updating without a refresh on
reactivating user, because the subscriptions data with the
client was not updated on reactivation.
This commit adds code to send peer_add subscription events
on reactivating the user.
We do not send peer_remove events on deactivating the user,
but the subscriber list is still live-updated because we
have the data of the streams which the deactivated user is
susbcribed to and the clients itself updates the data and UI
on receiving event of deactivation of user, which it is not
possible when reactivating the user.
Fixes#20383.
Leaving old invitations valid, potentially for a very long time, is
clearly unexpected and undesired behavior under normal circumstances. A
user shouldn't be able to e.g. generate a multiuse invite link, get
banned from the organization by being deactivated and then just re-join
using the link they've created for themselves.
do_revoke_user_invite and do_revoke_multi_use_invite were using objects
after their deletion to pass the argument to notify_invites_changed. We
should avoid that. The function was only using the .realm attribute of
the received objects, so it's simpler to make it just take realm as its
argument.
The msg parameter is a string to be displayed when the expected
exception wasn’t raised, not a pattern to match against the raised
exception’s message.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Under the unicodedata distributed with Python 3.6, some Emoji are
classified as `Cn`, and not `So`:
```
$ unicode 1f929 --long
U+1F929 GRINNING FACE WITH STAR EYES
UTF-8: f0 9f a4 a9 UTF-16BE: d83edd29 Decimal: 🤩 Octal: \0374451
🤩
Category: So (Symbol, Other); East Asian width: W (wide)
Unicode block: 1F900..1F9FF; Supplemental Symbols and Pictographs
Bidi: ON (Other Neutrals)
$ python3.6 -c 'import unicodedata; print(unicodedata.category("\U0001f929"))'
Cn
$ python3.7 -c 'import unicodedata; print(unicodedata.category("\U0001f929"))'
So
```
Drop `Cn` from the list of excluded Unicode character classes, and
replace it with an explicit list of the 66 non-characters, which are
invariant.
Co-authored-by: Shlok Patel <shlokcpatel2001@gmail.com>
An explanatory note on the changes in zulip.yaml and
curl_param_value_generators is warranted here. In our automated
tests for our curl examples, the test for the API endpoint that
changes the posting permissions of a stream comes before our
existing curl test for adding message reactions.
Since there is an extra notification message due to the change in
posting permissions, the message IDs used in tests that come after
need to be incremented by 1.
This is a part of #20289.
Prior to this commit, we wrapped all incoming messages from Slack
in backticks. This led to weird formatting errors when an incom-
ing message from Slack contains backticks, to refer to a function
name, for instance.
Moves `flags` field to top part of object description because
it is always included in the event.
If a field is present only for certain types of message updates,
the description begins by stating when the field is present:
"Only present if ...".
These fields are organized by the type of message update:
stream, stream and/or topic, topic, content.
If a field is not present due to a special event, the description
ends by stating when the field is not present:
"Not present if ...".
Adds documentation for fields currently required to be returned
with any `update_message` event.
do_delete_users had two bugs:
1. Creating the replacement dummy users
with active=True
2. Creating the replacement dummy users with email domain set to
realm.uri, which may not be a valid email domain.
Prior commits fixed the bugs, and this migration fixes the pre-existing
objects.
Otherwise the dummy user can be created with an invalid email domain -
e.g. in development environment with the domain
"@http://localhost:9991". get_fake_email_domain exists exactly for
handling these kinds of scenarios.
Stop using `access_user_group_by_id` in notifications codepaths, as it
is meant to be used to check for _write_ access, not read
access (which is not limited). In the notification codepaths, there
are no ACLs to apply, and the ID is known-good; just load it
directly. The `for_mention` flag is removed, as it was not used in the
mention codepaths at all, only the notification ones.
get_remote_server_by_uuid (called in validate_api_key) raises
ValidationError when given an invalid UUID due to how Django handles
UUIDField. We don't want that exception and prefer the ordinary
DoesNotExist exception to be raised.
APNs payloads nest the zulip-custom data further than the top level,
as Android notifications do. This led to APNs data silently never
being truncated; this case was not caught in tests because the mocks
provided the wrong data for the APNs structure.
Adjust to look in the appropriate place within the APNs data, and
truncate that.
This replaces the temporary (and testless) fix in
24b1439e93 with a more permanent
fix.
Instead of checking if the user is a bot just before
sending the notifications, we now just don't enqueue
notifications for bots. This is done by sending a list
of bot IDs to the event_queue code, just like other
lists which are used for creating NotificationData objects.
Credit @andersk for the test code in `test_notification_data.py`.
As explained in the comments in the code, just doing UUID(string) and
catching ValueError is not enough, because the uuid library sometimes
tries to modify the string to convert it into a valid UUID:
>>> a = '18cedb98-5222-5f34-50a9-fc418e1ba972'
>>> uuid.UUID(a, version=4)
UUID('18cedb98-5222-4f34-90a9-fc418e1ba972')
This diff looks slightly noisy, but the main chunk of
code that we moved here has the same logic as before,
and it just gets realm_id from MentionBackend now, instead
of having our markdown processor have to supply it.
We basically want MentionData to be the gatekeeper of
mention data, and then we delegate backend tasks to
MentionBackend.
Soon we will add a cache to MentionBacked, which will
justify this change a bit more.
We now make it mandatory to pass in the Realm object.
If this function was ever called with None, I am scared
to know what the expected results were at the time of
writing.
It's slightly annoying to plumb Optional[MentionBackend]
down the stack, but it's a one-time change.
I tried to make the cache code relatively unobtrusive
for the single-message use case.
We should be able to eliminate redundant stream queries
using similar techniques.
I considered caching at the level of rendering the message
itself, but this involves nearly as much plumbing, and
you have to account for the fact that several users on
your realm may have distinct default languages (French,
Spanish, Russian, etc.), so you would not eliminate as
many query hops. Also, if multiple streams were involved,
users would get slightly different messages based on
their prior subscriptions.
When our handlers specifically reference self.md.zulip_db_data,
we now use an explicit type.
We probably want a more robust solution here, such as a semgrep
rule.
We now serialize still_url as None for non-animated emojis,
instead of omitting the field. The webapp does proper checks
for falsiness here. The mobile app does not yet use the field
(to my knowledge).
We bump the API version here. More discussion here:
https://chat.zulip.org/#narrow/stream/378-api-design/topic/still_url/near/1302573
Appending to bytes in a loop leads to a quadratic slowdown since
Python doesn’t optimize this for bytes like it does for str.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
While accepting an invitation from a user, there was no condition in
place to check if the user sending the invitation was now
now-deactivated.
Skip sending notifications about newly-joined users to users who are
now disabled.
Fixes#18569.
We don't have to go to the database to get the Recipient
fields for `user_profile.recipient`.
See also 85ed6f332a from a little
over a year ago--it's very similar.
The bug here probably didn't come up too much in
practice, but if we were adding a user to multiple
streams when they already had used all N available
colors, all the new streams would be assigned the same
color, since the size of used_colors would stay at N,
thwarting our little modulo-len hackery.
It's not a terrible bug, since users can obviously
customize their stream colors as they see fit.
Usually when we are adding a user to multiple streams,
the users are fairly new, and thus don't have many
existing streams, so I have never heard this bug
reported in the field.
Anyway, assigning the colors in bulk seems to make more
sense, and I added some tests.
For the situations where all the colors have already
been used, I didn't put a ton of thought into exactly
which repeated colors we want to choose; instead, I
just ensure they're different modulo 24. It's possible
that we should just have more than 24 canned colors, or
we should just assign the same default color every time
and let users change it themselves (once they've gone
beyond the 24, to be clear). Or maybe we can just do
something smarter here. I don't have enough time for a
deep dive on this issue.
Part of our codepath for subscribing users involves
fetching the users' existing subscriptions to make sure
we can do things like properly report to the clients
that the users were already subscribed. This codepath
used to be coupled to code that helped users maintain
unique stream colors.
Suppose you are creating a new stream, and you are
importing users from an older stream with 15k
subscribers, and each of your users is subscribed to
about 20 streams.
The prior code, instead of filtering on recipient_id,
would literally look at every subscription for every
user, which was kind of crazy if you didn't understand
the pick-stream-color complications.
Before this commit, we would fetch 300k rows with 15
columns each (granted, all but one of the columns are
bool/int). That's a total of 4.5 million tiny objects
that we had to glom into Django ORM objects and slice
and dice.
After this commit, we would fetch exactly zero rows
for the are-they-already-subscribed logic.
Yes, ZERO.
If we were to mistakenly try to re-add the same 15k
subscribers to the new stream (under the new code), we
will now fetch 15k Sub rows instead of 300k.
It is worth looking at the prior commit. We go through
great pains to ensure that users get new stream colors
when we invite them to a stream, and we still fetch a
bunch of data for that. Instead of 4.5 million cells,
it's more like 600k cells (2 columns per row), and it's
less than that insofar as some users may only
have 24 distinct colors among their many streams.
It's a lot of work.
This commit sets us up for the next commit, which will
save us a very expensive query.
If you are adding 15k users to a stream, and each user
has about 20 existing streams, then we need to retrieve
300k rows from the database to figure out which stream
colors they already have. We don't need all the extra
fields from Subscription, so now we get just the two
values we need for making a color map.
In the next commit we'll eliminate the other use case
for the big query, and I will explain in greater
depth how splitting out the color-picking code can
be a huge win. It is possible that some product decisions
could make this codepath easier. We could also do some
engineering specific to stream colors, such as caching
which colors users have already used.
This does cost us an extra round trip to the database.
Having the `wildcard_mentions_notify` setting turned on does
not necessarily mean that the user will receive notification
for that message. There is more nuance to this, as explained
in the updated comment.
We recently ran into a payload in production that didn't contain
an event type at all. A payload where we can't figure out the event
type is quite rare. Instead of letting these payloads run amok, we
should raise a more informative exception for such unusual payloads.
If we encounter too many of these, then we can choose to conduct a
deeper investigation on a case-by-case basis.
With some changes by Tim Abbott.
Given that these values are uuids, it's better to use UUIDField which is
meant for exactly that, rather than an arbitrary CharField.
This requires modifying some tests to use valid uuids.
We avoid repeating the same calculations over and
over again for the same stream.
This helps, but the real bottleneck in this function
is that send_event usually takes at least a millisecond,
and that adds up quickly if you're doing something
like subscribing 5k users to a new stream.
GIF files can be `.GIF`, and also we determine the file format by
inspecting the image data, so there's no reason to have this
assertion.
(The code for serving still images does not rely on the file being a
GIF.)
Have kept process_new_human_user out of
the atomic block because it involves many
different operations and also sends events.
Tried enclosing event in on_commit but that
would need many changes in the tests, so have
skipped it for now.
Updates testing helpers in `event_schema.py` for `do_update_message` so
that all stream message fields are present in any edits / updates to
stream messages. Adds verfication tests of events returned from private
message edits and from stream message content-only and topic-only edits.
Updates the `update_message` event type to always include a `stream_id`
field when the message being edited is a stream message. This change
aligns with the current definition of the `\get-events` endpoint
in the OpenAPI documentation.
It is better to press on, than stop halfway through due to a user
whose email no longer works. The exception is already logged, which
is sufficient here, as this is generally run interactively.
These fundamentally tested send_email, not build_email, and thus
belong in TestSendEmail, not TestBuildEmail. They also duplicated the
code in test_send_email_exceptions; reuse it.
This allows verify_uploads to use the database
as the authoritative source for what attachments
we need to look for when we're verifying the
images got exported properly, while still
also verifying attachment.json is correct.
It is better for the verifying code to just explicitly
ensure that the exported file bytes match the bytes
in the test image. This introduces a tiny bit more
of I/O.
It's easier to read the code without the intermediate
full_data dictionary that obscures where the files live.
We also avoid some unnecessary file i/o in the tests.
We do a sanity check for every table
that gets written to user.json as part of
the single-user export.
If we add more tables to the single-user export,
the test that I modified here will now ask
the author to add a new checker function, which
means we should always have at least a basic
sanity check for every exported table as long
as we stay in this new paradigm.
We also remove a little bit of old code that
became redundant.
This replaces the TERMS_OF_SERVICE and PRIVACY_POLICY settings with
just a POLICIES_DIRECTORY setting, in order to support settings (like
Zulip Cloud) where there's more policies than just those two.
With minor changes by Eeshan Garg.
We do s/TOS/TERMS_OF_SERVICE/ on the name, and while we're at it,
remove the assumed zerver/ namespace for the template, which isn't
correct -- Zulip Cloud related content should be in the corporate/
directory.
We now complain if a test author sends a stream message
that does not result in the sender getting a
UserMessage row for the message.
This is basically 100% equivalent to complaining that
the author failed to subscribe the sender to the stream
as part of the test setup, as far as I can tell, so the
AssertionError instructs the author to subscribe the
sender to the stream.
We exempt bots from this check, although it is
plausible we should only exempt the system bots like
the notification bot.
I considered auto-subscribing the sender to the stream,
but that can be a little more expensive than the
current check, and we generally want test setup to be
explicit.
If there is some legitimate way than a subscribed human
sender can't get a UserMessage, then we probably want
an explicit test for that, or we may want to change the
backend to just write a UserMessage row in that
hypothetical situation.
For most tests, including almost all the ones fixed
here, the author just wants their test setup to
realistically reflect normal operation, and often devs
may not realize that Cordelia is not subscribed to
Denmark or not realize that Hamlet is not subscribed to
Scotland.
Some of us don't remember our Shakespeare from high
school, and our stream subscriptions don't even
necessarily reflect which countries the Bard placed his
characters in.
There may also be some legitimate use case where an
author wants to simulate sending a message to an
unsubscribed stream, but for those edge cases, they can
always set allow_unsubscribed_sender to True.
These variables can be unset if the `os.path.exists` check fails.
That should be rare, since we've previously checked the files do
exist before getting here.
While races here are unlikely, it is most correct to enforce this
invariant at the database layer, and having a database-level
constraint makes the models file a bit more readable.
These are not considered to be "personal"
info, even if you upload them, so we
don't export them.
Generally the only folks who upload
these are admins, who can easily get
them in other ways. In fact, anybody
can get these via the app.
We now ensure that all message ids are sorted BEFORE
we split them into batches.
We now do a few extra "slim" queries to get message
ids up front.
But, now, when we divide them into batches, we no
longer run 2 or 3 different complicated queries in
a loop. We just basically hydrate our message ids,
so `write_message_partials` should be easy to reason
about.
This change also means that for tiny realms with
< 1000 messages you will always have just one
json file, since we aggregate the ids from the
queries before batching.
This accomplishes a few things:
* It extracts `chunkify` rather than having us
clumsily track chunking-related stuff in a
big loop that is doing other stuff.
* It makes it so that all message ids
in message-000001.json < message-000002.json.
* It makes it easier for us to customize
the messages we send to a single user
(coming soon).
BTW we probably have a slicker version of chunkify
somewhere in our codebase, but I couldn't remember
where.
Following b3c58f454f, we want to clean up
old topics that may contain the disallowed characters. The Message table
is large, so we go in batches, making sure we limit topic fetches and
UPDATE query to no more than BATCH_SIZE Message rows per query.
Now all file writes go through our three
helper functions, and we consistently
write a single log message after the file
gets written.
I killed off write_message_exports, since
all but one of its callers can call
write_table_data, which automatically
sorts data. In particular, our Message
and UserMessage data will now be sorted
by ids.
This probably just postpones the list creation until
Django builds the "IN" query, but semantically it's
good to work in sets where we don't have any
meaningful ordering of the list that gets used.
The immediate benefit of this is stronger mypy
checks (avoiding the ugly union caused by message
files).
The subsequent commit will add sorting.
We have test coverage on all these lines insofar
as if you comment out the lines, tests will
explode (i.e. more than superficial line
coverage).
The distinction here wasn't super meaningful
due to the way we order our "elif" statements,
but we want to reserver "normal_parent" for the
majority of use cases, where you simply tell
the Config what the "foreign_key" is.
For realm-wide exports, there is no reason to query
inefficiently against a list of modified users.
We move the Config out of the common child configs.
Even though Django usually treats foo__in
and foo_id__in identically for filters where
foo is a ForeignKey type, we want to insist
on somewhat more consistent syntax, because
we have the odd combo of type and type_id
in Recipient, where type_id is kinda like a
foreign key, but not a ForeignKey.
So we assert for now that all our include_rows
values end in "_id__in".
Zulip shows two guides on How to reply, first one by
the welcome bot and second one is intro_reply hotspot.
To simply and avoid redundancy, intro_reply hotspot is
removed.
Fixes#20482.
Force postgres to give reactions in ID order - which
is generally chronological order. Results in frontend
displaying reactions in said order.
Fixes#20060.
In many of our stream notification messages, we make use of the
same silent user mention syntax, the template for which was always
hardcoded. This commit adds a helper function that all relevant
callers can call to get the right syntax when mentioning users.
Thanks to Tim Abbott for this suggestion!
Removed existing empty narrow divs from app/home.html and created
a new javascript module to dynamically load empty narrow messages
using handlebar template.
Fixes#18797
The original intention of this was to prevent coding
errors with realm getters that don't, um, filter
on realm.
Unfortunately, you can still write a broken realm getter
that forgets to filter on realm, but which returns a
Set, and the new safeguards won't see any difference.
We could make all the getters return sorted lists
instead, but that's for another day.
This code does serve another purpose, which is to
prevet egregious bugs in the import itself.
The diff here is ugly, but to summarize:
BEFORE IMPORT:
define get_user_id
define get_huddle_hashes
AFTER IMPORT AND MAKING GETTERS:
check realm id
define assert_realm_values
verify emoji codes
check huddle hashes
We don't have automated test coverage on this yet,
but below are the results from manual testing.
Note that we include the realm icon and logo even
though they were not created by Cordelia.
./manage.py export_single_user cordelia@zulip.com
$ (cd /tmp/zulip-export-4v3mo802/ && find .)
.
./emoji
./emoji/2
./emoji/2/emoji
./emoji/2/emoji/images
./emoji/2/emoji/images/3.jpg
./emoji/records.json
./messages-000001.json
./realm_icons
./realm_icons/2
./realm_icons/2/night_logo.original
./realm_icons/2/night_logo.png
./realm_icons/2/icon.png
./realm_icons/2/icon.original
./realm_icons/records.json
./avatars
./avatars/2
./avatars/2/c5125af0447f4d66ce34c1b32eac75ac27ebe0e7.original
./avatars/2/c5125af0447f4d66ce34c1b32eac75ac27ebe0e7.png
./avatars/records.json
./uploads
./uploads/2
./uploads/2/68
./uploads/2/68/xyEkC5dTIp8m42_6HJ3kBfdt
./uploads/2/68/xyEkC5dTIp8m42_6HJ3kBfdt/denver.jpg
./uploads/2/96
./uploads/2/96/ol5WE6RTUntvuPDSpJUrYTim
./uploads/2/96/ol5WE6RTUntvuPDSpJUrYTim/denver.jpg
./uploads/records.json
./user.json
There are tactical reasons to remove this assertion.
Basically, the reason it's safe to remove is that it's
been around a long time and we would have seen this
operationally. Also, the check to make sure that the
S3 filename thingy matches the avatar hash is a much
stronger check.
We will soon restore a stronger version of this check
that applies to all of our asset types (emojis/avatars/etc.).
This makes it easier to read the calling code and see
the big picture of how the four asset types are
organized.
I also handle uploads first, to be similar to the local
code.
This code is well tested--you can modify any of the callers
to pass in a wrong value of `object_key` and get a failing
test.
The comment explains in more detail, but basically we'd skip
exercising a bit of code in the signup code path if there were no
messages in the last week, resulting in the query count not matching.
"help" command occurs in the command list in
initial pms or when bot doesn't understand the message. It doesn't
occur when the bot is respoding to the "help" command itself.
This commit adds code to check whether a user is allowed to use
wildcard mention in a large stream or not while editing a message
based on the realm settings.
Previously this was only checked while sending message, thus user
was easily able to use wildcard mention by first sending a normal
message and then using a wildcard mention by editing it.
1. The initial welcome message now contains less detail.
2. The bot now responds to these commands: "apps", "edit profile",
"dark mode", "light mode", "streams", "topics", "message formatting",
"keyboard shortcuts" and "help" - the bot still responds if there are
slight variations in these commands.
3. Tests have been made to check if bot responds to the advertised
commands (with variations) and gives a negative message if it doesn't
understand the message.
With substantial tweaks by tabbott.
Fixes#19900.
A confirmation link takes a user to the check_prereg_key_and_redirect
endpoint, before getting redirected to POST to /accounts/register/. The
problem was that validation was happening in the check_prereg_key_and_redirect
part and not in /accounts/register/ - meaning that one could submit an
expired confirmation key and be able to register.
We fix this by moving validation into /accouts/register/.
Migrates the `/update-subscription-settings` api endpoint to the
`ignored_parameters_unsupported` model, which is also currently used
by `/update-settings` and `update-realm-user-settings-defaults`.
This change is a step towards preparing for an eventual migration to
have all endpoints return an `ignored_parameters_unsupported` block.
Previously the `/update-subscription-settings` endpoint returned a
copy of the data object sent in the request.
Fixes#15307.
django-scim2 doesn't order the rows when fetching them in reponse to a
query using the filter syntax. We ensure that ORDER BY id is always
appended to the SQL queries.
We add the following tables to the user export:
AlertWord
CustomProfileFieldValue
RealmAuditLog
Service
UserActivity
UserActivityInterval
UserCount
UserGroup
UserHotspot
UserPresence
UserTopic
Except for UserCount, we achieve this by sharing
code with the realm export via
add_user_profile_child_configs.
UserCount is handled slightly differently than realm
exports due to which key we trigger off.
It's possible that RealmAuditLog is incomplete for
single users, since we may also want rows where they
are the acting_user. This commit finds rows where
they are the modified_user. For non-admins I believe
it's rarely the case that they are the actor, and
they will tend to be the modified user if the two
fields are different at all. For admins it's
arguable we want to see both changes they enacted
as well as changes that affected them.
This commit switches the BigBlueButton integration
to use SHA256 instead of SHA1 as BigBlueButton supports
it and scalelite does now, too.
Fixes#19966.
This commit changes web_public_streams_enabled to return False if
realm.enable_spectator_access is False. This is added so that
creating web-public streams is not allowed if enable_spectator_access
is False.
Python's behaviour on `sys.exit` is to wait for all non-daemon threads
to exit. In the context of the missedmessage_emails worker, if any
work is pending, a non-daemon Timer thread exists, which is waiting
for 5 seconds. As soon as that thread is serviced, it sets up another
5-second Timer, a process which repeats until all
ScheduledMessageNotificationEmail records have been handled. This
likely takes two minutes, but may theoretically take up to a week
until the thread exits, and thus sys.exit can complete.
Supervisor only gives the process 30 seconds to shut down, so
something else must prevent this endless Timer.
When `stop` is called, take the lock so we can mutate the timer.
However, since `stop` may have been called from a signal handler, our
thread may _already_ have the lock. As Python provides no way to know
if our thread is the one which has the lock, make the lock a
re-entrant one, allowing us to always try to take it.
With the lock in hand, cancel any outstanding timers. A race exists
where the timer may not be able to be canceled because it has
finished, maybe_send_batched_emails has been called, and is itself
blocked on the lock. Handle this case by timing out the thread join
in `stop()`, and signal the running thread to exit by unsetting the
timer event, which will be detected once it claims the lock.
Special characters, including `\r`, `\n`, and more esoteric codepoints
like non-characters, can negatively affect rendering and UI behaviour.
Check for, and prevent making new messages with, characters in the
Unicode categories of `Cc` (control characters), `Cs`, (surrogates),
and `Cn` (unassigned, non-characters).
Fixes#20128.
This commit replaces "dark mode" and "light mode" with "dark theme"
and "light theme" in the message returned and shown in a little
popup in the UI, when color scheme settings are changed through
slash commands.
Adds `wildcard_mentions_notify` as a property that can be updated
by the endpoint and removes mention of potential `null` value in
the return object because it is not possible.
Also cleans up the documentation of `in_home_view` legacy property
and updates the return object description to better reflect what
is actually returned.
Since spectators can't access personal profile settings and
can't view profile for other users. Hence, we don't send realm
custom profile field data and user's profile data to spectators.
Fixes#20301.
Enable spectator access for test `zulip` realm in developement
setup.
Add option in `do_create_realm` to configure
`enable_spectator_access` field of `Realm`.
We restrict access of messages from web public streams if
anonymous login is disabled via `enable_spectator_access`.
Display of `Anonymous login` button is now controlled by
the value of `enable_spectator_access`.
Admins can toggle `enable_spectator_access` via org settings in UI.
If null is a potential value of data type for a return value or
parameter in the API endpoint, then it is rendered as an option.
This currently relies on the 'nullable' setting in the OpenAPI spec
that was removed in the 3.1.0 release. If/when the OpenAPI version
is updated, then how the `data_type` for parameters and return values
is rendered will need to be reworked.
Fixes#20264.
It turns out these were just wrong. We fix a few things:
* Sort the list of settings so that it's possible to compare with reality.
* Deleted additional fields that don't actually exist.
* Fixed various fields missing past feature level updates.
Neither of these fields use the `realm/update_dict` event type; they
use `realm/update`; we've attempted to clarify that in the previous
commit.
That reality means we don't have automated testing for these values,
and that meant that typos like these could slip through.
The user id is a very useful piece of information that the mobile
client should have access to - instead of only getting the email. This
makes it much simpler to impleent clients that might be robust to
changes in email address.
RabbitMQ clients have a setting called prefetch[1], which controls how
many un-acknowledged events the server forwards to the local queue in
the client. The default is 0; this means that when clients first
connect, the server must send them every message in the queue.
This itself may cause unbounded memory usage in the client, but also
has other detrimental effects. While the client is attempting to
process the head of the queue, it may be unable to read from the TCP
socket at the rate that the server is sending to it -- filling the TCP
buffers, and causing the server's writes to block. If the server
blocks for more than 30 seconds, it times out the send, and closes the
connection with:
```
closing AMQP connection <0.30902.126> (127.0.0.1:53870 -> 127.0.0.1:5672):
{writer,send_failed,{error,timeout}}
```
This is https://github.com/pika/pika/issues/753#issuecomment-318119222.
Set a prefetch limit of 100 messages, or the batch size, to better
handle queues which start with large numbers of outstanding events.
Setting prefetch=1 causes significant performance degradation in the
no-op queue worker, to 30% of the prefetch=0 performance. Setting
prefetch=100 achieves 90% of the prefetch=0 performance, and higher
values offer only minor gains above that. For batch workers, their
performance is not notably degraded by prefetch equal to their batch
size, and they cannot function on smaller prefetches than their batch
size.
We also set a 100-count prefetch on Tornado workers, as they are
potentially susceptible to the same effect.
[1] https://www.rabbitmq.com/confirms.html#channel-qos-prefetch
The `current_queue_size` key in the queue monitoring stats file was
the local queue size, not the global queue size -- d5a6b0f99a
renamed the function, but did not adjust the queue monitoring JSON,
despite the last use of it having been removed in cd9b194d88.
The function is still used to mark "we emptied our queue," and it
remains a reasonable metric for that.
TOR users are legitimate users of the system; however, that system can
also be used for abuse -- specifically, by evading IP-based
rate-limiting.
For the purposes of IP-based rate-limiting, add a
RATE_LIMIT_TOR_TOGETHER flag, defaulting to false, which lumps all
requests from TOR exit nodes into the same bucket. This may allow a
TOR user to deny other TOR users access to the find-my-account and
new-realm endpoints, but this is a low cost for cutting off a
significant potential abuse vector.
If enabled, the list of TOR exit nodes is fetched from their public
endpoint once per hour, via a cron job, and cached on disk. Django
processes load this data from disk, and cache it in memcached.
Requests are spared from the burden of checking disk on failure via a
circuitbreaker, which trips of there are two failures in a row, and
only begins trying again after 10 minutes.
Unhandled exceptions propagating to process_queue were not caught there,
causing improper logging - errors didn't land in errors.log as expected.
Exceptions should be caught and explicitly logged by the process_queue
logger. Exceptions occurring during consuming events are caught and
handled inside the worker's logic - however those that happen while
setting up the worker were not addressed at all, and that's the core bug
we mean to address here.
Furthermore, in multi-threaded mode we want the autoreload mechanism to
be working - which it doesn't without catching the exceptions. The
correct approach is to - again - catch the exception, log it and then
send SIGUSR1 signal to trigger exit and autoreload.
I believe that this migration with the default of atomic=True will
fail when trying to convert the field to PositiveIntegerField if there
were any 0 values present in the database when the migration began.
The fix is to have each of the steps be their own transaction.
A SIGTERM can show up at any point in the ioloop, even in places which
are not prepared to handle it. This results in the process ignoring
the `sys.exit` which the SIGTERM handler calls, with an uncaught
SystemExit exception:
```
2021-11-09 15:37:49.368 ERR [tornado.application:9803] Uncaught exception
Traceback (most recent call last):
File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/tornado/http1connection.py", line 238, in _read_message
delegate.finish()
File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/tornado/httpserver.py", line 314, in finish
self.delegate.finish()
File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/tornado/routing.py", line 251, in finish
self.delegate.finish()
File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/tornado/web.py", line 2097, in finish
self.execute()
File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/tornado/web.py", line 2130, in execute
**self.path_kwargs)
File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/tornado/gen.py", line 307, in wrapper
yielded = next(result)
File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/tornado/web.py", line 1510, in _execute
result = method(*self.path_args, **self.path_kwargs)
File "/home/zulip/deployments/2021-11-08-05-10-23/zerver/tornado/handlers.py", line 150, in get
request = self.convert_tornado_request_to_django_request()
File "/home/zulip/deployments/2021-11-08-05-10-23/zerver/tornado/handlers.py", line 113, in convert_tornado_request_to_django_request
request = WSGIRequest(environ)
File "/home/zulip/deployments/2021-11-08-05-10-23/zulip-py3-venv/lib/python3.6/site-packages/django/core/handlers/wsgi.py", line 66, in __init__
script_name = get_script_name(environ)
File "/home/zulip/deployments/2021-11-08-05-10-23/zerver/tornado/event_queue.py", line 611, in <lambda>
signal.signal(signal.SIGTERM, lambda signum, stack: sys.exit(1))
SystemExit: 1
```
Supervisor then terminates the process with a SIGKILL, which results
in dropping data held in the tornado process, as it does not dump its
queue.
The only command which is safe to run in the signal handler is
`ioloop.add_callback_from_signal`, which schedules the callback to run
during the course of the normal ioloop. This callbacks does an
orderly shutdown of the server and the ioloop before exiting.
This commit ensures that all markdown fixtures have unique
test names by rewriting the names of some of them and adding
a test in `test_markdown.py`.
Earlier this was over-writing the value for same keys in
`load_markdown_tests` in `test_markdown.py`.
Race conditions in stream unsubscription may lead to multiple
back-to-back SUBSCRIPTION_DEACTIVATED RealmAuditLog entries for the
same stream. The current logic constructs duplicate UserMessage
entries for such, which then later fail to insert.
Keep a set of message-ids that have been prep'd to be inserted, so
that we don't duplicate them if there is a duplicated
SUBSCRIPTION_DEACTIVATED row. This also renames the `message` local
variable, which otherwise overrode the `message` argument of a
different type.
Previously, our codebase contained links to various versions of the
Django docs, eg https://docs.djangoproject.com/en/1.8/ref/
request-response/#django.http.HttpRequest and https://
docs.djangoproject.com/en/2.2/ref/settings/#std:setting-SERVER_EMAIL
opening a link to a doc with an outdated Django version would show a
warning "This document is for an insecure version of Django that is no
longer supported. Please upgrade to a newer release!".
Most of these links are inside comments.
Following the replacement of these links in our docs, this commit uses
a search with the regex "docs.djangoproject.com/en/([0-9].[0-9]*)/"
and replaces all matches with "docs.djangoproject.com/en/3.2/".
All the new links in this commit have been generated by the above
replace and each link has then been manually checked to ensure that
(1) the page still exists and has not been moved to a new location
(and it has been found that no page has been moved like this), (2)
that the anchor that we're linking to has not been changed (and it has
been found that no anchor has been changed like this).
One comment where we mentioned a Django version in text before linking
to a page for that version has also been changed, the comment
mentioned the specific version when a change happened, and the history
is no longer relevant to us.
This resolves the issues reported in #20108, major chunk of which were
due to the incomplete support for importing the livechat streams/messages
in the tool. So, it's best not to import any livechat streams/messages for
now until a complete support for importing the same is developed.
The decorator form is clearer by being more explicit; additionally,
the api_by_user rate-limit only currently used in one place, and makes
it difficult to test per-user rate-limits that are more specific.
Both `create_realm_by_ip` and `find_account_by_ip` send emails to
arbitrary email addresses, and as such can be used to spam users.
Lump their IP rate limits into the same bucket; most legitimate users
will likely not be using both of these endpoints at similar times.
The rate is set at 5 in 30 minutes, the more quickly-restrictive of
the two previous rates.
The existing test did no verify that the rate limit only applied to
127.0.0.1, and that other IPs were unaffected. For safety, add an
explicit test of this.
The only use case of rate_limit_rule which does not clear the
RateLimitedIPAddr history is test_hit_ratelimits_as_remote_server,
which is not made any worse by clearing out the IP history for a
non-existent `api_by_remote_server` domain.
Since `prefers_web_public_view` key in session is only
relevant to users without an account, this key should no longer
be present in the user's session object.
Fixes#19907
For export realm following changes have been made:
- `./manage.py export --upload` would delete `.tar.gz` and unpacked dir
- `./manage.py export` would only delete `unpacked dir`
Besides, we have removed `--delete-after-upload` as we have set it as
the default.
Fixes#20081
If realm is web_public, spectators can now view avatar of other
users.
There is a special exception we had to introduce in rest model to
allow `/avatar` type of urls for `anonymous` access, because they
don't have the /api/v1 prefix.
Fixes#19838.
This commit adds functionality to import messages from the
Discussions having direct channels as their parent. As we don't
have topics in the PMs, the messages are imported in interleaved
form in the imported direct channels/PMs.
This was completely unsupported earlier and would have resulted in
an error.
This commit updates the error message returned when the maximum
invite limit for the day. We update the error returned by API to
only mention that the limit is reached and add the suggestion
to use multi-use link or contact support in the message shown
in webapp.
Add `escape_navigates_to_default_view` as a bool setting in
UserBaseSettings model and implement it as a checkbox that toggles
the hotkey implementation of escape to the default view in the
advanced user display settings.
With /help/ documentation edits from Alya Abbott.
Fixes#20043.
We always use delivery_email to generate gravatar_url, but in
test_admin_api_hide_emails we were passing email to get_gravatar_url
and matched with the avatar_url field of the fetched user object.
The tests were passing because the email_address_is_realm_public
was using old realm object and thus email field was incorrectly
set to delivery_email even when email_address_visibility was set
to EMAIL_ADDRESS_VISIBILITY_ADMINS.
This commit fixes the test to pass delivery_email to get_gravatar_url.
We create RealmUserDefault object for internal realm just
for consistency. The code in migration does so but it
was missed to add the code when creating new internal realm.
Not proxying these requests through camo is a security concern.
Furthermore, on the desktop client, any embed image which is hosted on
a server with an expired or otherwise invalid certificate will trigger
a blocking modal window with no clear source and a confusing error
message; see zulip/zulip-desktop#1119.
Rewrite all `message_embed_image` URLs through camo, if it is enabled.
We add discussion id and url in the comments and highlighted title to
the body of disscussion message to make it more meaningful and accessible.
Fixes#19938.
We aim to use Zulip topics thoughtfully in displaying messages from
discussions, as well as linking to the discussion in every message so
that it's easy to view them.
Fixes#19938.
Supporting URL percent-encoded bytes is possible using `%%20`, but this
is not necessarily very understandable to end-users, even those that
understand percent encoding.
Allow `%20` in linkifier URL format strings, and transform them into
`%%20` in the pattern just before they are applied in markdown
translation. Care must be taken here, such that already-escaped `%`s
are not escaped an extra time.
We do this before rendering, and not before storage, as
a simplification; the JS-side linkifier at present only understands
`%(foo)s` and thus needs no changes, and to avoid an un-escaping pass
before showing in the admin UI.
User-supplied custom realm filter has had some sort of regex-based
validation of the format URL since their introduction in
d7e1e4a2c0 -- and this has always been
in addition to the URLValidator. The URLValidator is the one which
does the security-relevant work of validating that the schema is
reasonable, and that the overall shape of the URL is well-formed. The
regex has served primarily to arbitrary limit the characters that can
appear in the URL, in the mistaken name of safety.
Adjust the regex, such that its only purpose is to verify that the
usages of `%` characters in the URL are reasonable, and leave the URL
validation to the URLValidator, which can do a far better job. This
includes broadening the support to include `%%` as an escape
character; this is likely such a niche case as to be unnecessary, but
costs little.
Fixes#16013.
og:image is supposed to be an absolute URL, but some sites incorrectly
provide a relative URL. In this case, it makes more sense to
interpret it relative to the full page URL after redirects, rather
than relative to just the domain part of the page URL before
redirects.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We don't yet have a do_reactivate_stream function, but we reserve a
number since:
1. It'll likely be added in the future.
2. For now, we can restore archived stream with some manual intervention
in the Django shell, and for that we'll want to create an appropriate
RealmAuditLog entry.
Removes the `/day` and `/night` options from the typeahead menu while
still allowing the commands to be used. Typing `/day` and `/night`
will now suggest `/light` and `/dark`, respectively. Also changes the
`Dark mode` and `Light mode` popups that appear after using the
corresponding command.
Fixes#18318.
Created a schema for the ignored_parameters_unsupported that is
returned by the /settings and /realm/user_settings_defaults endpoints
and removed the duplicated text in the api documentation.
Also cleaned up some small errors in the /realm/user_settings_default
definition and sidebar link /api/update-realm-user-settings-defaults.
Fixes#19674
Since 3853285241, PushNotificationsWorker uses the aioapns library
to send Apple push notifications. This introduces an asyncio event
loop into this worker process, which, if unlucky, can respond poorly
when a SIGALRM is introduced to it:
```
[asyncio] Task exception was never retrieved
future: <Task finished coro=<send_apple_push_notification.<locals>.attempt_send() done, defined at /path/to/zerver/lib/push_notifications.py:166> exception=WorkerTimeoutException(30, 1)>
Traceback (most recent call last):
File "/path/to/zerver/lib/push_notifications.py", line 169, in attempt_send
result = await apns_context.apns.send_notification(request)
File "/path/to/zulip-py3-venv/lib/python3.6/site-packages/aioapns/client.py", line 57, in send_notification
response = await self.pool.send_notification(request)
File "/path/to/zulip-py3-venv/lib/python3.6/site-packages/aioapns/connection.py", line 407, in send_notification
response = await connection.send_notification(request)
File "/path/to/zulip-py3-venv/lib/python3.6/site-packages/aioapns/connection.py", line 189, in send_notification
data = json.dumps(request.message, ensure_ascii=False).encode()
File "/usr/lib/python3.6/json/__init__.py", line 238, in dumps
**kw).encode(obj)
File "/usr/lib/python3.6/json/encoder.py", line 199, in encode
chunks = self.iterencode(o, _one_shot=True)
File "/usr/lib/python3.6/json/encoder.py", line 257, in iterencode
return _iterencode(o, 0)
File "/path/to/zerver/worker/queue_processors.py", line 353, in timer_expired
raise WorkerTimeoutException(limit, len(events))
zerver.worker.queue_processors.WorkerTimeoutException: Timed out after 30 seconds processing 1 events
```
...which subsequently leads to the worker failing to make any progress
on the queue.
Remove the timeout on the worker. This may result in failing to make
forward progress if Apple/Google take overly long handling requests,
but is likely preferable to failing to make forward progress if _one_
request takes too long and gets unlucky with when the signal comes
through.
This makes logging more consistent between FCM and APNs codepaths, and
makes clear which user-ids are for local users, and which are opaque
integers namespaced from some remote zulip server.
Being able to determine how many distinct users are getting push
notifications per remote host is useful, as is the distribution of
device counts. This parallels the log line in
handle_push_notification for push notifications from local realms,
handled via the event queue.
We should use more selective query for UserGroupMembership
objects in tests for checking adding and removing members.
This is done by checking the membership counts for the
particular user group only.
This will help in keeping the tests more understandable
after we add members to the role-based system groups,
since that would create a lot of membership objects.
We make the UserGroup queries in user group creation and
deletion tests more selective by fitering the user groups
which belong to the realm and not the one included in
lear realm, etc.
This will help us to keep the tests more understandable
when the counts of UserGroup increases due to addition of
system groups. There is no need to consider system groups
of other realms in these tests.
It is confusing to have the plan type constants not be namespaced
by the thing they represent. We already have a namespacing
convention in place for constants, so we should use it for
Realm.plan_type as well.
* Remove unnecessary json_validator for full_name parameter.
* Update frontend to pass the right parameter.
* Update documentation and note the change.
Fixes#18409.
`rendered_content` in historical messages may be empty; examining the
history of them may thus require diff'ing two empty strings, which
itself produces an empty string.
Use `lxml.html.fragment_fromstring` to be able to successfully parse
these, rather than 500.
Part of #19559.
As detailed in the comments, the default behavior is undesirable for us
because we can't really predict all possibilities of exceptions that may
be raised - and thus putting str(e) in the http response is potentially
insecure as it may leak some unexpected sensitive information that was
in the exception.
As a hypothetical example - KeyError resulting from some buggy
some_dict[secret_string] call would leak information. Though of course
we aim to never write code like that.
We pass allow_realm_admin as True to access_stream_by_id for
`GET users/{user_id}/subscriptions{stream_id}` endpoint
because we want to allow non-subscribed admins to get
subscription status in private streams.
Fixes#19077.
Tim Abbott
Aman Agrawal
Mateusz Mandera
Anders Kaseorg
jai2201
Lauryn Menard
Sahil Batra
Sharif Naas
Eeshan Garg
Palash
Alex Vandiver
Steve Howell
BIKI DAS
Abhijeet Prasad Bodas
parth
Shlok Patel
odunybrad
Nikhil Maske
Nipunn Koorapati
S-Abhishek
Alya Abbott
strifel
akshatdalton
YashRE42
Priyansh Garg
Chris Bobbe
Suyash Vardhan Mathur
RISHABH SIDANA
Jonny Tran
rht
Gaurav Pandey
Vishnu KS