The IntegrityError shows up in the database logs, which looks
unnecessarily concerning. Use `ON CONFLICT IGNORE` to mark this as
expected, especially since the return value is never used.
This commit adds a 'skip-delay' option to the
'send_zulip_update_announcements' management command.
It will be useful for self-hosted servers after 9.0 upgrade to
avoid the 24 hour delay to receive update messages after group
DM is sent to the admins.
One can run the management command with the --skip-delay flag
to immediately send the update messages.
When an organization (without open ability for anyone to join) invites a
guest user, the invitation prompts allows them to choose whether the
guest should be added to default streams or not. This is useful, because
since we don't have per-role default streams configs, they may want
default streams to be for full Members.
SCIM provisioning doesn't have this control, since a newly provisioned
user gets created via a direct do_create_user call, thus adding them to
the organization's default streams, with no workaround possible aside of
just getting rid of default streams in the organization.
To make provisioning guests in such an organization usable, we add a
simple config option to create them with no streams. It's configured by
adding
```
"create_guests_without_streams": True
```
to the config dict in settings.SCIM_CONFIG.
Creating a bot with a name that is already in use
will raise an error. However, by deactivating
the existing bot, creating a new bot with the
same name, and then reactivating the original bot,
it is possible to have multiple bots with the same name.
To fix this, we check if the bot name is already
in use in the active bots list. If it is,
an error will be raised, prompting either the
name of the existing bot to be changed or
the bot to be deactivated.
Co-authored-by: Sujal Shah <sujalshah28092004@gmail.com>
Adds "channel" to the `stream_wildcards` frozenset for stream
wildcard notifications on the backend/server.
Updates frontend/web-app to handle "channel" as the other stream
wildcards are handled in the typeahead and composebox modules.
Updates the API version and documentation for the addition of
"channel" as a wildcard mention. But does not change any of the
functionailty of (or deprecate) the "stream" wildcard at this
point.
Part of project to rename "stream" to "channel".
This is a prep commit to add a 'recipient_users' parameter to
the 'internal_send_huddle_message' function.
'emails' is no longer a required parameter. We can use either
of the 'emails' or 'recipient_users' parameter. 'emails' is
eventually used to fetch 'recipient_users', so if the
'recipient_users' is already available we should use that to
skip database query.
For organizations with "Zulip update announcements" stream set
to a default value, we wait for one day after sending group
DM to admins to allow them to change the stream from it's
default value if they wish to.
This prep commit refactors the function
'is_group_direct_message_sent_to_admins_atleast_one_week_ago' to
'is_group_direct_message_sent_to_admins_within_days' allowing us
to use a flexible timeframe instead of hardcoded 1 week.
We will reuse this function as a part of determinig whether
group DM to admins was sent within 1 day.
Previously, users were allowed to signup or change their names to
those which already existed in the realm.
This commit adds an Organization Permission, that shall enforce
users to use unique names while signing up or changing their
names. If a same or normalized full name is found in realm,
then a validation error is thrown.
Fixes#7830.
For multiline strings in triple quotes, a '\n' is included
at the end of each line.
Earlier, to skip '\n' we used to add an escape character '\'
at the end of each line.
This commit adds a function to avoid manually adding '\'.
As a part of the zulip news feature, we send an initial
group DM to admins suggesting them to update or set
the 'zulip_update_announcements_stream'.
This commit updates those messages to make it better.
As noted in the docstring for `bulk_insert_ums`, this is at least one
order of magnitude faster than using `bulk_create`. This also
includes a `ON CONFLICT DO NOTHING` which allows multiple
soft-reactivations to run at once without failing. We also adjust the
update of `last_active_message_id` to be safe against races.
Rather than use a bulk insert via Django, use the faster
`bulk_insert_all_ums` that we already have. This also adds a `ON
CONFLICT` clause, to make the insert resilient to race conditions.
There are currently two callsites, with different desired `ON
CONFLICT` behaviours:
- For `notify_reaction_update`, if the `UserMessage` had already been
created, we would have done nothing to change it.
- For `do_update_message_flags`, we would have ensured a specific bit
was (un)set.
Extend `create_historical_user_messages` and `bulk_insert_all_ums` to
support `ON CONFLICT (...) UPDATE SET flags = ...`.
The bots do not exist in the user table to look up their active
status, and attempting to import them into the analytics table will
result in duplicate rows.
Replace the long string for organisations that have notification
body/content disabled (settings.PUSH_NOTIFICATION_REDACT_CONTENT
set to true) with "New message".
This allows more of the limited space on the mobile device screen to
be used for additional messages rather than this verbose content.
Fixes#29152
For the common case of not needing to reference the UserMessage row
later, and for being a stream without private history, the UserMessage
row is irrelevant. Convert `has_user_message` to a thunk, and defer
loading it unless necessary.
Replaced HUDDLE attribute with DIRECT_MESSAGE_GROUP using VS Code search,
part of a general renaming of the object class.
Fixes part of #28640.
Co-authored-by: JohnLu2004 <JohnLu10212004@gmail.com>
This commit adds a management command that will run regularly
as a cron job to send zulip updates to realms based on their
current and latest zulip_update_announcements_level.
For realms with:
* level = None: Send a group DM to admins notifying them about
this new feature & suggestion to set the stream accordingly.
* level = 0:
* If stream is still not configured, wait for a week
before setting their level to latest level. They will
miss updates until their configure the stream.
* If stream is configured, send updates.
* level > 0: Send one message/update per level & increase
the level by 1 till the latest level.
Fixes#28604.
To improve onboarding experience, this commit adds a
one-time modal which introduces the recent conversations view.
Users see this one-time modal on visiting the recent
conversations view.
Fixes#29073.
To improve onboarding experience, this commit adds
a one-time modal which introduces the inbox view.
Users see this one-time modal on visiting the inbox view.
Fixes part of #29073.
Replace a separate call to subprocess, starting `node` from scratch,
with an optional standalone node Express service which performs the
rendering. In benchmarking, this reduces the overhead of a KaTeX call
from 120ms to 2.8ms. This is notable because enough calls to KaTeX in
a single message would previously time out the whole message
rendering.
The service is optional because he majority of deployments do not use
enough LaTeX to merit the additional memory usage (60Mb).
Fixes: #17425.
Just shows a config error page if the bouncer is not enabled. Uses a new
endpoint for this so that it can work nicely for both browser and
desktop app clients.
It's necessary, because the desktop app expects to get a json response
with either an error or billing_access_url to redirect to. Showing a
nice config error page can't be done via the json error mechanism, so
instead we just serve a redirect to the new error page, which the app
will open in the browser in a new window or tab.
Only affects zulipchat, by being based on the BILLING_ENABLED setting.
The restricted backends in this commit are
- AzureAD - restricted to Standard plan
- SAML - restricted to Plus plan, although it was already practically
restricted due to requiring server-side configuration to be done by us
This restriction is placed upon **enabling** a backend - so
organizations that already have a backend enabled, will continue to be
able to use it. This allows us to make exceptions and enable a backend
for an org manually via the shell, and to grandfather organizations into
keeping the backend they have been relying on.
Adds a re-usable lockfile_nonblocking helper to context_managers.
Relying on naive `os.mkdir` is not enough especially now that the
successful operation of this command is necessary for push notifications
to work for many servers.
We can't use `lockfile` context manager from
`zerver.lib.context_managers`, because we want the custom behavior of
failing if the lock can't be acquired, instead of waiting.
That's because if an instance of this gets stuck, we don't want to start
queueing up more processes waiting forever whenever the cronjob runs
again and fail->exit is preferrable instead.
LoggingCountStats with a daily duration and that are directly stored
on the RealmCount table (not via aggregation in process_count_stat),
can be in a state, after the hourly cron job to update analytics
counts, where the logged value will be live-updated later, because
the end time for the stat is still in the future.
As these logging counts are designed to be used on the self-hosted
installation for either debugging or rate limiting, sending these
partial/incomplete counts to the bouncer has low value.
This commit adds a realm-level setting named
'zulip_update_announcements_stream' that configures the
stream to which zulip updates should be posted.
Fixes part of #28604.
- Adds instructions for downloading a zuliprc file for a bot or for
yourself.
- Updates the button label to "Download zuliprc", since that's the
filename it downloads.
Fixes#28881.
This commit renames the realm-level setting
'signup_notifications_stream' to 'signup_announcements_stream'.
The new name reflects better what the setting does.
This commit renames the realm-level setting 'notifications_stream'
to 'new_stream_announcements_stream'.
The new name reflects better what the setting does.
5c96f94206 mistakenly appended, rather than prepended, the edit to
the history. This caused AssertionErrors when attempting to view the
history of moved messages, which check that the `last_edit_time`
matches the timestamp of the first edit in the list.
Fix the ordering, and update the `edit_history` for messages that were
affected. We limit to only messages edited since the commit was
merged, since that helps bound the affected messages somewhat.
The widening of the time between when a process is marked for
reload (at Tornado startup) and when it sends reload events makes it
unlikely-to-impossible that a single `/` request will span both of
them, and thus hit the WebReloadClientError corner case.
Remove it, as it is not worth the complication. The bad behaviour it
is attempting to prevent (of a reload right after opening `/`) was
always still possible -- if the `/` request completed right before
Tornado restarted -- so it is not clear that it was ever worth the
complication.
Having a non-identity `cache_transformer` is no different from running
it on every row of the query_function. Simplify understanding of the
codepath used in caching by merging the pieces of code.
Rather than pass around a list of message objects in-memory, we
instead keep the same constructed QuerySet which includes the later
propagated messages (if any), and use that same query to pick out
affected Attachment objects, rather than limiting to the set of ids.
This is not necessarily a win -- the list of message-ids *may* be very
long, and thus the query may be more concise, easier to send to
PostgreSQL, and faster for PostgreSQL to parse. However, the list of
ids is almost certainly better-indexed.
After processing the move, the QuerySet must be re-defined as a search
of ids (and possibly a very long list of such), since there is no
other way which is guaranteed to correctly single out the moved
messages. At this point, it is mostly equivalent to the list of
Message objects, and certainly takes no less memory.
Rather than use `bulk_update()` to batch-move chunks of messages, use
a single SQL query to move the messages. This is much more efficient
for large topic moves. Since the `edit_history` field is not yet
JSON (see #26496) this requires that PostgreSQL cast the current data
into `jsonb`, append the new data (also cast to `jsonb`), and then
re-cast that as text.
For single-message moves, this _increases_ the SQL query count by one,
since we have to re-query for the updated data from the database after
the bulk update. However, this is overall still a performance
improvement, which improves to 2x or 3x for larger topic moves. Below
is a table of duration in seconds to run `do_update_message` to move a
topic to a new stream, based on messages in the topic, for before and
after this change:
| Topic size | Before | After |
| ---------- | -------- | ------- |
| 1 | 0.1036 | 0.0868 |
| 2 | 0.1108 | 0.0925 |
| 5 | 0.1139 | 0.0959 |
| 10 | 0.1218 | 0.0972 |
| 20 | 0.1310 | 0.1098 |
| 50 | 0.1759 | 0.1366 |
| 100 | 0.2307 | 0.1662 |
| 200 | 0.3880 | 0.2229 |
| 500 | 0.7676 | 0.4052 |
| 1000 | 1.3990 | 0.6848 |
| 2000 | 2.9706 | 1.3370 |
| 5000 | 7.5218 | 3.2882 |
| 10000 | 14.0272 | 5.4434 |
This applies access restrictions in SQL, so that individual messages
do not need to be walked one-by-one. It only functions for stream
messages.
Use of this method significantly speeds up checks if we moved "all
visible messages" in a topic, since we no longer need to walk every
remaining message in the old topic to determine that at least one was
visible to the user. Similarly, it significantly speeds up merging
into existing topics, since it no longer must walk every message in
the new topic to determine if the user could see at least one.
Finally, it unlocks the ability to bulk-update only messages the user
has access to, in a single query (see subsequent commit).
The problem was that earlier this was just an uncaught JsonableError,
leading to a full traceback getting spammed to the admins.
The prior commit introduced a clear .code for this error on the bouncer
side, meaning the self-hosted server can now detect that and handle it
nicely, by just logging.error about it and also take the opportunity to
adjust the realm.push_notifications_... flags.
- Renames "Bots and integrations" to "Bots overview" everywhere
(sidebar, page title, page URL).
- Adds a copy of /api/integrations-overview (symbolic link) as the
second page in the Bots & integrations section, titled
"Integrations overview".
Fixes#28758.
Creates an incoming webhook integration for Patreon. The main
use case is getting notifications when new patrons sign up.
Fixes#18321.
Co-authored-by: Hari Prashant Bhimaraju <haripb01@gmail.com>
Co-authored-by: Sudipto Mondal <sudipto.mondal1997@gmail.com>
The previous query suffered from bad corner cases when the user had
received a large number of direct messages but sent very few,
comparatively. This mean that the first half of the UNION would
retrieve a very large number of UserMessage rows, requiring fetching a
large number of Message rows, merely to throw them away upon
determining that the recipient was the current user.
Instead of merging two queries of "last 1k received" + "last 1k sent",
we instead make better use of the UserMessage rows to find "last 1k
sent or received." This may change the list of recipients, as large
disparities in sent/received messages may result in pushing the
most-recently-sent users off of the list. These are likely uncommon
edge cases, however -- and the disparity is the whole reason for the
performance problem.
This also provides more correct answers. In the case where a user's
1001'th message sent was to person A today, but my most recent message
received was from them yesterday, the previous plan would show the
message I received yesterday message-id as the max, and not the more
recent message I sent today.
While we could theoretically raise the `RECENT_CONVERSATIONS_LIMIT` to
more frequently match the same recipient list as previously, this
increases the cost of the most common cases unreasonably. With a
1000-message limit, the common cases are slightly faster, and the tail
latencies are very much improved; raising `RECENT_CONVERSATIONS_LIMIT`
would increase the result similarity to the old algorithm, at the cost
of the p50 and p75.
| | Old | New |
| ------ | ------- | ------- |
| Mean | 0.05287 | 0.02520 |
| p50 | 0.00695 | 0.00556 |
| p75 | 0.05592 | 0.03351 |
| p90 | 0.14645 | 0.08026 |
| p95 | 0.20181 | 0.10906 |
| p99 | 0.30691 | 0.16014 |
| p99.9 | 0.57894 | 0.19521 |
| max | 22.0610 | 0.22184 |
On the whole, however, the much more bounded worst case are worth the
small changes to the resultset.
This is preparatory work towards adding a Topic model.
We plan to use the local variable name as 'topic' for
the Topic model objects.
Currently, we use *topic as the local variable name for
topic names.
We rename local variables of the form *topic to *topic_name
so that we don't need to think about type collisions in
individual code paths where we might want to talk about both
Topic objects and strings for the topic name.
This is preparatory work towards adding a Topic model.
We plan to use the local variable name as 'topic' for
the Topic model objects.
Currently, we use *topic as the local variable name for
topic names.
We rename local variables of the form *topic to *topic_name
so that we don't need to think about type collisions in
individual code paths where we might want to talk about both
Topic objects and strings for the topic name.
This is preparatory work towards adding a Topic model.
We plan to use the local variable name as 'topic' for
the Topic model objects.
Currently, we use *topic as the local variable name for
topic names.
We rename local variables of the form *topic to *topic_name
so that we don't need to think about type collisions in
individual code paths where we might want to talk about both
Topic objects and strings for the topic name.
Requests to these endpoint are about a specified user, and therefore
also have a notion of the RemoteRealm for these requests. Until now
these endpoints weren't getting the realm_uuid value, because it wasn't
used - but now it is needed for updating .last_request_datetime on the
RemoteRealm.
`<time:1234567890123>` causes a "signed integer is greater than
maximum" exception from dateutil.parser; datetime also cannot handle
it ("year 41091 is out of range") but that is a ValueError which is
already caught.
Catch the OverflowError thrown by dateutil.
boto3 has two different modalities of making API calls -- through
resources, and through clients. Resources are a higher-level
abstraction, and thus more generally useful, but some APIs are only
accessible through clients. It is possible to get to a client object
from a resource, but not vice versa.
Use `get_bucket(...).meta.client` when we need direct access to the
client object for more complex API calls; this lets all of the
configuration for how to access S3 to sit within `get_bucket`. Client
objects are not bound to only one bucket, but we get to them based on
the bucket we will be interacting with, for clarity.
We removed the cached session object, as it serves no real purpose.
e883ab057f started caching the boto client, which we had identified
as slow call. e883ab057f went further, calling
`get_boto_client().generate_presigned_url()` once and caching that
result.
This makes the inner cache on the client useless. Remove it.
If we `.distinct("delivery_email")` then we must also
`.order_by("delivery_email")`; adc987dc43 added the `.order_by`
call, which broke the newsletter codepath, since it did not contain
the `delivery_email` in the ordering fields.
Add a flag to distinct on emails in `send_custom_email`.
For remote servers, we cannot advertise `List-Unsubscribe=One-Click`,
which is specified in RFC 8058[^1] to mean that the `List-Unsubscribe`
URL supports a POST request with no arguments to unsubscribe. Because
we show an interstitial and confirmation page, as this is not just a
mailing list which is disabled if you click the link, it does not
support the mail system performing the unsubscribe for the user.
Remove the inaccurate header for remote servers.
[^1]: https://datatracker.ietf.org/doc/html/rfc8058
612f2c73d6 started passing add_context to
`send_custom_server_email`, but did not make it make use of it.
Also add the `hostname` as a built-in value, since that is most likely
the most useful property.
This fixes the exception case on the initial
`/api/v1/remotes/server/analytics/status` case. Other exceptions from
`send_to_push_bouncer` are allowed to escape.
Co-authored-by: Alex Vandiver <alexmv@zulip.com>
Previously, passing a url longer than 200 characters for
jitsi_server_url caused a low-level failure at DB level. This
commit adds this restriction at API level.
Fixes part of #27355.
Saying `**options: str` is a lie, since it contains bools. We pluck
out the two bools that we need properly typed because we will be
pushing them into function calls, and type them explicitly as bools.
If the exception was because the channel closed, attempting to NAK the
events will just raise another error, and is pointless, as the server
already marked the pending events as NAK'd.
This ensures determinism in these tests doing mock_send.assert_called
with - avoids producing test flakes due to a different order of
retrieval of these objects from the database.
- The server sends the list of registrations it believes to have with
the bouncer.
- The bouncer includes in the response the registrations that it doesn't
actually have and therefore the server should delete.
This commit creates a RealmAuditlog entry with a new event_type
'RealmAuditLog.REALM_IMPORTED' after the realm is reactivated.
It contains user count data (using realm_user_count_by_role)
stored in extra_data.
This helps to have an accurate user count data for the billing
system if someone tries to signup just after doing an import.