This is a prep commit to extract out the logic to
create message from 'internal_send_huddle_message'
into a separate function 'internal_prep_huddle_message'.
We will use this new function to get the huddle message
without sending it immediately.
In general, we never want to use savepoints.
This prep commit adds savepoint=False in do_send_messages
as we don't want to just rollback to this savepoint and
proceed if we encounter any error while sending zulip updates
via cron.
A user who was no longer subscribed to a private stream kept their
UserMessage row for a message sent while they were in it; this is
expected. However, they _also_ kept that row even if the message was
moved to a different private stream that they were also not subscribed
to. This violates the invariant that users without subscriptions
never have UserMessage rows.
This `if new_stream is not None` block was improperly indented,
causing it to only run if the propagation mode was not `change_one`.
Since the block controlled creation and deletion of UserMessage rows,
this led to messages being improperly still visible to members of the
old stream if they were being moved from public to private streams.
Clients also failed to receive `delete_message` events, so the
messages remained visible in their feeds until they reloaded the
application.
We don't want to show one-time modals introducing 'Inbox' and
'Recent conversations' views to existing users.
When a user views a modal, we mark it as read by storing a row
in the 'OnboardingStep' model. Once marked as read, the user will
no longer see the modal.
This commit adds a migration to create
two rows per user (mark them as read) in OnboardingStep model.
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.
Links to the available message flag table in the feature level 224
changelog entry, as there are relevant **Changes** notes for this
feature level in that part of the API documentation.
Updates the order and formatting of these new and deprecated flags
in the available flags table. Also, adds a link to the topic
wildcard mentions section of the help center documentation.
Makes small clean ups to the changes notes for this feature level,
as well as the changelog entry itself.
The original commit for these feature level 224 API changes was
c597de6a1d.
Refactor `parse_client` view to use `typed_endpoint decorator`
instead of `has_request_variables`. This change improves code consistency
and enhances codebase comprehension.
PostgreSQL's estimate of the number of usermessage rows for a single
message can be wildly off, due to poor statistics generation. This
causes this query, with 100-message batch sizes, to incorrectly
estimate millions of matched rows, causing it to perform a full-table
index scan, rather than piecemeal using the `message_id` index.
Reduce the batch size to 50, which is enough to tip in favor of a
rational query plan.
Refactor `report_csp_violations` view to use `typed_endpoint` decorator
instead of `has_request_variables`. This change improves code
consistency and enhances codebase comprehension.
Depending on the kind of config error being shown, different "go back"
links may be more appropriate.
We probably hard-coded /login/ for it, because these config errors are
most commonly used for authentication backend config error, where it
makes sense to have /login/ as "go back", because the user most likely
indeed got there from the login page.
However, for remote_billing_bouncer_not_configured, it doesn't make
sense, because the user almost surely is already logged in and got there
by clicking "Plan management" inside the gear menu in the logged in app.
It's best for these to just be consistent. Therefore:
1. The .../not-configured/ error page endpoint should be restricted to
.has_billing_access users only.
2. For consistency, self_hosting_auth_view_common is tweaked to also do
the .has_billing_access check as the first thing, to avoid revealing
configuration information via its redirect/error-handling behavior.
The revealed configuration information seems super harmless, but it's
simpler to not have to worry about it and just be consistent.
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.
When a server doesn't submit a remote realm info which was
previously submitted, we mark it as locally deleted.
If such a realm has paid plan attached to it, we should investigate.
This commit adds logic to send an email to sales@zulip.com for
investigation.
This commit updates default for delete_own_message_policy
setting to "Everyone" as it is helpful to allow everyone
to delete their own messages in a new organization where
users might be using Zulip for the first time.
This commit updates default for move_messages_between_streams_policy
setting to "Members and above" as it is helpful to allow members
to move messages between streams in new organizations where users
might be using Zulip for first time.
The presence of `len(messages)` outside the transaction caused the
full resultset to be fetched outside of the transaction. This should
ideally be inside the transaction, and also only need be the count.
However, also note that the process of counting matching rows, and
then executing a second query which embeds the same query, is
susceptible to phantom reads, where a query with the same conditions
returns different resultsets, under PostgreSQL's default transaction
isolation of "read committed." While this is possible to resolve by
pulling the returned IDs into a Python list, it would not address the
issue that concurrent updates which change the resultset would make
the overall algorithm still incorrect.
Add a comment clarifying the conditions under which the algorithm is
correct. A more correct algorithm would walk the UserMessage rows
which are unread and in the stream, but this requires a
whole-UserMessage index which would be quite large for such an
infrequent use case.
This makes no immediate reloads the default for runtornado, matching
the production configuration, and changes the development incantation
to be the one to specify the departure from the norm, with
--immediate-reloads.
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.
Due to the channel_map_to_topics URL parameter in the Slack webhook,
it was not migrated to use the check_send_webhook_message.
By using check_send_webhook_message, any topic parameter in the
webhook URL will be prioritized over mapping Slack channels to
topics, e.g. when channel_map_to_topics is true. This is because
the default behaviour for incoming webhooks is to send a default
topic as a parameter to check_send_webhook_message in case there
is no topic specified in the URL.
In contrast, we can override the stream passed in the URL when
channel_map_to_topics is false by passing the Slack channel name
to check_send_webhook_message. The default behaviour for incoming
webhooks is to send a direct message if there is no specified
stream in the URL, so a default stream is not generally passed
to check_send_webhook_message.
Fixes#27601.
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.
The previous logic incorrectly used the server-level number of users
even when a (presumably smaller) realm-level count was available.
Fixes a bug introduced in 2e1ed4431a.
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.
RemoteRealm customer takes precedence over RemoteServer
in general. But if an inactive plan is associated with
RemoteRealm and an active plan with RemoteServer, the
ACTIVE plan takes precendence.
Co-authored-by: Prakhar Pratyush <prakhar@zulip.com>
Previously, in DM disabled org messaging to bot was not working when
starting new conversation and adding bot as recipient because of not
updating on recipient change. And secondly, self messaging was not
allowed.
This commit ensures that the DM to bot and self are allowed irrespective
of dm restrictions.
tests: Verify DMs adhere to DM restriction policy.
Fixes#28412
Signed-off-by: sayyedarib <sayyedaribhussain4321@gmail.com>
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.
Collapsing was done incorrectly, as 65c400e06d added `zulip_version`
and `zulip_feature_level`, but did not update the virtual event logic
to copy those new values into the virtual event.
However, it is unlikely that a server will be upgraded multiple times
in quick enough succession for this to ever be relevant. Remove the
logic, which is additional complication for little or no gain.
Commit bd6471f0e3 (#28691) added this
reference to the old name, even though it had already been renamed in
commit b220d29fed (#17775), presumably
because that had failed to update the OpenAPI description.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
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).
This is a preparatory commit that refactors the check_update_message
method to extract the checks containing whether a user can edit the
message or not into a separate method -validate_message_content_edit,
so that it can be re used later.
This logic was apparently missed when we implemented private streams
with shared history; the correct check is to look at whether the user
can access message history in the stream, which used to be equivalent
to whether it's a private stream.
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.
This commit removes the stale 'email_gateway' parameter
from 'do_send_messages' function.
This should have been removed in 6c473ed75f,
when the call to 'build_message_send_dict' was removed
from 'do_send_messages'.
This error message didn’t make sense for the check as written, and our
OpenAPI document already provides the expected format for our 200
responses.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Real requests would not validate against the previous version. There
seems to be no consistent way to determine whether a string parameter
should be coerced to an integer for validation against an allOf
schema (which works at the level of JSON objects, not strings).
See also https://github.com/python-openapi/openapi-core/issues/698.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The endpoint was lacking validation that the authentication_methods dict
submitted by the user made sense. So e.g. it allowed submitting a
nonsense key like NoSuchBackend or modifying the realm's configured
authentication methods for a backend that's not enabled on the server,
which should not be allowed.
Both were ultimately harmless, because:
1. Submitting NoSuchBackend would luckily just trigger a KeyError inside
the transaction.atomic() block in do_set_realm_authentication_methods
so it would actually roll back the database changes it was trying to
make. So this couldn't actually create some weird
RealmAuthenticationMethod entries.
2. Silently enabling or disabling e.g. GitHub for a realm when GitHub
isn't enabled on the server doesn't really change anything. And this
action is only available to the realm's admins to begin with, so
there's no attack vector here.
test_supported_backends_only_updated wasn't actually testing anything,
because the state it was asserting:
```
self.assertFalse(github_auth_enabled(realm))
self.assertTrue(dev_auth_enabled(realm))
self.assertFalse(password_auth_enabled(realm))
```
matched the desired state submitted to the API...
```
result = self.client_patch(
"/json/realm",
{
"authentication_methods": orjson.dumps(
{"Email": False, "Dev": True, "GitHub": False}
).decode()
},
)
```
so we just replace it with a new test that tests the param validation.
- 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.
We use Alertmanager as an aggregation place for example for failing CI pipelines,
and `graph` does not always reflect the source of the alert. It's called `source` originally
and I think it should stay this way.
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>
This commit updates the API to check the permission to subscribe other
users while creating multi-use invites. The API will raise error if
the user passes the "stream_ids" parameter (even when it contains only
default streams) and the calling user does not have permission to
subscribe others to streams.
We did not add this before as we only allowed admins to create
multiuse invites, but now we have added a setting which can be used
to allow users with other roles as well to create multiuse invites.
Prakhar Pratyush
Alex Vandiver
Lauryn Menard
Mahhheshh
Aman Agrawal
Mateusz Mandera
David Rosa
Anders Kaseorg
Sahil Batra
Alya Abbott
Tim Abbott
sayyedarib
Karl Stolley
roanster007
shashank-23002
Artur Szcześniak