Long-term, we probably want to make the filtering options more
generic, but there's little harm in adding an option for a specific
group we're likely to email multiple times.
This extends the /json/typing endpoint to also accept
stream_id and topic. With this change, the requests
sent to /json/typing should have these:
* `to`: a list set to
- recipients for a PM
- stream_id for a stream message
* `topic`, in case of stream message
along with `op`(start or stop).
On receiving a request with stream_id and topic, we send
typing events to clients with stream_typing_notifications set
to True for all users subscribed to that stream.
The comment explains in more detail, but this should help avoid cases
where a Zulip server accidentally avoids the nag by having upgraded to
a 2-year old Zulip version from a 3-year-old version 2 months ago.
This commit adds support for a `None` option in the dropdown menu
of `Notification sound`. When this option is selected, no audible
notification is sent to the user.
`None` will appear as the first option in the dropdown menu, since
this is not categorized as a playable audio.
This new option is added so that folks can disable audio notifications
without losing their other notification configuration (like for PMs, mentions).
Necessary test case is added for this new option.
Fixes#16090.
Add a `--dry-run` flag to send_custom_email management command
in order to provide a mechanism to verify the emails of the recipients
and the text of the email being sent before actually sending them.
Add tests to:
- Check that no emails are actually sent when we are in the dry-run mode.
- Check if the emails are printed correctly when we are in the dry-run mode.
Fixes#17767
Previously the outgoing emails were sent over several SMTP
connections through the EmailSendingWorker; establishing a new
connection each time adds notable overhead.
Redefine EmailSendingWorker worker to be a LoopQueueProcessingWorker,
which allows it to handle batches of events. At the same time, persist
the connection across email sending, if possible.
The connection is initialized in the constructor of the worker
in order to keep the same connection throughout the whole process.
The concrete implementation of the consume_batch function is simply
processing each email one at a time until they have all been sent.
In order to reuse the previously implemented decorator to retry
sending failures a new method that meets the decorator's required
arguments is declared inside the EmailSendingWorker class. This
allows to retry the sending process of a particular email inside
the batch if the caught exception leaves this process retriable.
A second retry mechanism is used inside the initialize_connection
function to redo the opening of the connection until it works or
until three attempts failed. For this purpose the backoff module
has been added to the dependencies and a test has been added to
ensure that this retry mechanism works well.
The connection is closed when the stop method is called.
Fixes: #17672.
This was introduced in 8321bd3f92 to serve as a sort of drop-in
replacement for zerver.lib.queue.queue_json_publish, but its use has
been subsequently cut out (e.g. `9fcdb6c83ac5`).
Remote its last callsite.
It's better to just raise JsonableError here, as that makes this error
processed in the central place for this kind of thing in do_rest_call:
---------
except JsonableError as e:
response_message = e.msg
logging.info("Outhook trigger failed:", stack_info=True)
fail_with_message(event, response_message)
response_message = f"The outgoing webhook server attempted to send a message in Zulip, but that request resulted in the following error:\n> {e}"
notify_bot_owner(event, failure_message=response_message)
return None
----------
which does all the things that are supposed to happen -
fail_with_message, appropriate logging and notifying the bot owner.
These aren't good mocks of a good reponse - a good response is supposed
to contain valid json that doesn't trigger error-handling in the
codepath. Without this change, all these actually trip up on
json.loads(response.text) in process_success_response.
Remove content edit keys if present in edit_history_event
when passing to update_messages_for_topic_edit.
Since content edit is only applied to the edited_message,
this shouldn't be part of the rest of the messages for which
topic was edited. This was a bug identified by
editing topic and content of a message at the same time
when more than 1 message is affected.
model__id syntax implies needing a JOIN on the model table to fetch the
id. That's usually redundant, because the first table in the query
simply has a 'model_id' column, so the id can be fetched directly.
Django is actually smart enough to not do those redundant joins, but we
should still avoid this misguided syntax.
The exceptions are ManytoMany fields and queries doing a backward
relationship lookup. If "streams" is a many-to-many relationship, then
streams_id is invalid - streams__id syntax is needed. If "y" is a
foreign fields from X to Y:
class X:
y = models.ForeignKey(Y)
then object x of class X has the field x.y_id, but y of class Y doesn't
have y.x_id. Thus Y queries need to be done like
Y.objects.filter(x__id__in=some_list)
There exists a logic bug (see #18236) which causes duplicate
usermessage rows to be inserted. Currently, this stops catch-up for
all users.
Catch and record the exception for each affected user, so we at least
make catch-up progress on other users.
This reverses the policy that was set, but incompletely enforced, by
commit 951514dd7d. The self-closing tag
syntax is clearer, more consistent, simpler to parse, compatible with
XML, preferred by Prettier, and (most importantly now) required by
FormatJS.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This was a mostly harmless bug, since those users cannot have active
clients, but fixing it will improve performance in any Zulip
organization where the vast majority of users are deactivated.
This commit adds an API to `zproject/urls.py` to edit/update
the realm linkifier. Its helper function to update the
database is added in `zerver/lib/actions.py`.
`zulip.yaml` is documented accordingly as well, clearly
stating that this API updates one linkifier at a time.
The tests are added for the API and helper function which
updates the realm linkifier.
Fixes#10830.
Use backend-endpoint function instead of helper function in
`test_realm_linkifiers.py` so that tests are more end-to-end.
The removed helper function: `do_add_linkifier` is tested in
`zerver/tests/test_events.py`.
Linkifier error message: `Filter not found` is
updated to `Linkifier not found.`.
Similarly, `filter_id` description is updated to:
`The ID of the linkifier that you want to remove.`,
renamed the term `filter` with `linkifier`, in `zulip.yaml`.
We move compose.html to compose.hbs file while keeping
`#compose` still in `home.html` as a hanger
where append rest of the elements.
This will provide us with two benefits:
* We could share common elements between message_edit_form and
compose.
* We can insert compose directly in any element. We may decide to
do it for recent topics.
This prevents us from having to json encode every field in the POST
request to /realm/playgrounds, and keeps the client logic simpler
when adding a playground.
The caller is supposed validate the stream and user realm match, but
since this is a security-sensitive function, we should have this
defensive code to protect against some validation bugs in the caller
leading to this being called incorrectly and returning True.
Fixes#17922.
These two places fetch subscriptions for the sake of getting user ids to
send events to. Clearly deactivated users should be excluded from that.
get_active_subscriptions_for_stream_id should allow specifying whether
subscriptions of deactivated users should be included in the result.
Active subs of deactivated users are a subtlety that's easy to miss
when writing relevant code, so we make include_deactivated_users a
mandatory kwarg - this will force callers to definitely give thought to
whether such subs should be included or not.
This commit is just a refactoring, we keep original behavior everywhere
- there are places where subs of deactivates users should probably be
excluded but aren't - we don't fix that here, it'll be addressed in
follow-up commits.
This commit adds new helper can_move_messages_between_streams
which will be used to check whether a user is allowed to move
messages from one stream to another according to value of
'move_messages_between_streams_policy'.
Current production code uses client_id in the event dict and this test
should be updated to reflect that. Old format event can still be
consumed by the worker, but that is already tested by
WorkerTest.test_UserActivityWorker.
Since c3a8a15bae removed the last
instance of code using the dictionary code path, we actually need to
wait until one can no longer upgrade directly from 4.x to master in
order to avoid breakage should we remove this compatibility code,
since only today did we stop generating the old event format.
The bulk deletion codepath was using dicts instead of user ids in the
event, as opposed to the other codepath which was adjusted to pass just
user ids before. We make the bulk codepath consistent with the other
one. Due to the dict-type events happening in 3.*, we move the goal for
deleting the compat code in process_notification to 5.0.
This was dropped in commit 840cf4b885
(#15091), but commit 1432067959
(#17047) mistakenly reintroduced it.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
django.conf.urls.url is actually a deprecated alias of
django.urls.re_path, but we want path instead of re_path.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
django.utils.encoding.smart_text is a deprecated alias of
django.utils.encoding.smart_str as of Django 3.0, and will be removed
in Django 4.0.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
django.utils.http.is_safe_url is a deprecated alias of
django.utils.http.url_has_allowed_host_and_scheme as of Django 3.0,
and will be removed in Django 4.0.
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.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
I have added a documentation page for the GitHub Actions integration to
`/integrations/doc/github-actions` with a link to the Zulip GitHub
Actions repository.
Tweaked by tabbott to add cross-links with the main GitHub integration.
This widget only filters the user's subscription -- it's only suggest
public streams that the user is not subscribed to. "Filter" is the
correct label for a widget with this use case.
This wasn't being validated before. There wasn't any possibility to
actually succeed in moving a private message, because the codepath would
fail at assert message.is_stream_message() in do_update_message - but we
should have proper error handling for that case instead of internal
server errors.
Otherwise an admin can move a topic from a private stream they're no
longer a part of - including the newest messages in the topic, that
they're not supposed to have access to.
A bug in the implementation of the topic moving API resulted in
organization administrators being able to move messages to streams they
shouldn't be allowed to - private streams they weren't subscribed to and
streams in other organization hosted by the same Zulip installation.
In our current model realm admins can't send messages to private streams
they're not subscribed to - and being able move messages to a
stream effectively allows to send messages to that stream and thus the
two need to be consistent.
A bug in the implementation of the all_public_streams API feature
resulted in guest users being able to receive message traffic to public
streams that should have been only accessible to members of the
organization.
A bug in the implementation of the can_forge_sender permission
(previously is_api_super_user) resulted in users with this permission
being able to send messages appearing as if sent by a system bots,
including to other organizations hosted by the same Zulip installation.
- The send message API had a bug allowing an api super user to
use forging to send messages to other realms' streams, as a
cross-realm bot. We fix this most directly by eliminating the
realm_str parameter - it is not necessary for any valid current use
case. The email gateway doesn't use this API despite the comment in
that block suggesting otherwise.
- The conditionals inside access_stream_for_send_message are changed up
to improve security. They were generally not ordered very well,
allowing the function to successfully return due to very weak
acceptance conditions - skipping the higher importance checks that
should lead to raising an error.
- The query count in test_subs is decreased because
access_stream_for_send_message returns earlier when doing its check
for a cross-realm bot sender - some subscription checking queries are
skipped.
- A linkifier test in test_message_dict needs to be changed. It didn't
make much sense in the first place, because it was creating a message
by a normal user, to a stream outside of the user's realm. That
shouldn't even be allowed.
A bug in the implementation of replies to messages sent by outgoing
webhooks to private streams meant that an outgoing webhook bot could be
used to send messages to private streams that the user was not intended
to be able to send messages to.
Completely skipping stream access check in check_message whenever the
sender is an outgoing webhook bot is insecure, as it might allow someone
with access to the bot's API key to send arbitrary messages to all
streams in the organization. The check is only meant to be bypassed in
send_response_message, where the stream message is only being sent
because someone mentioned the bot in that stream (and thus the bot
posting there is the desired outcome). We get much better control over
what's going by passing an explicit argument to check_message when
skipping the access check is desirable.
Organization admins can use this setting to restrict the maximum
rating of GIFs that will be retrieved from GIPHY. Also, there
is option to disable GIPHY too.
We had a mix of the two names, and "video call provider" both feels
more professional and more clear about precisely what it does.
We don't change the API fields, since it doesn't seem worth an API
migration.
Moves documentation about using zoom as video call provider
to /integrations. This documentation was earlier present
at /help/start-a-call and is moved as asked in issue #17588.
Moves documentation about using Big Blue Button as video call
provider to /integrations. This documentation was earlier
present at /help/start-a-call and is moved as asked in issue #17588.
Moves documentation about using jitsi as video call provider
to /integrations. This documentation was earlier present
at /help/start-a-call and is moved as asked in issue #17588.
In zulip.yaml, `pattern` and `url_format_string` are extracted
as a shared parameters.
This is done as a preparatory commit for `Add settings UI to edit
linkifiers`.
Related issue: #10830.
In "test_curl_examples.py" we find the functions that registered but
never called. To improve readablity, now we have the full
implementation in curl_param_value_generators, rather than inspecting
its fields from another module.
Currently, there are separate tests for testing change of one role
to other, precisely 8, with most of them having similar structure
of code. This commit adds a helper function check_user_role_change
which contains all the code for testing and the tests for different
role just use this helper function to avoid duplication of code.
This refactor is helpful considering we would want to add tests
for moderators also, which would contain multiple tests for
testing changing different user roles to moderator and vice versa.
Tweaked by timabbott to make the code more readable by checking for
every user role flag instead of just checking the certain flags and
using conditionals.
Co-authored-by: Tim Abbott
This commit removes can_access_all_realm_members function as
it is not used anywhere in code other than tests.
This function was originally added in 4483e33102 and was
only used in digest.py other than the tests, but its use
in diget.py was removed in 735b6cb761 and the function
itself was not removed from models.py.
We refactor check_has_permission_policies to check for all user roles for
each value of policy. This will help in handle a case where a guest is
allowed to do something but moderator isn't.
We need to do user_profile.refresh_from_db() in validation_func because
the realm object from user_profile is used in has_permission and we need
updated realm instance after changing the policy.
This is a follow-up commit to 9a4c58cb.
Plurals are handled natively by the ICU MessageFormat syntax, so I
think we don’t have to do anything here.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
* This introduces a new event type `realm_linkifiers` and
a new key for the initial data fetch of the same name.
Newer clients will be expected to use these.
* Backwards compatibility is ensured by changing neither
the current event nor the /register key. The data which
these hold is the same as before, but internally, it is
generated by processing the `realm_linkifiers` data.
We send both the old and the new event types to clients
whenever the linkifiers are changed.
Older clients will simply ignore the new event type, and
vice versa.
* The `realm/filters:GET` endpoint (which returns tuples)
is currently used by none of the official Zulip clients.
This commit replaces it with `realm/linkifiers:GET` which
returns data in the new dictionary format.
TODO: Update the `get_realm_filters` method in the API
bindings, to hit this new URL instead of the old one.
* This also updates the webapp frontend to use the newer
events and keys.
Changed the name of the test-user cordelia from `Cordelia Lear` to
`Cordelia, Lear's daughter`.
This change will enable us to test users with escape characters in
their names.
I also updated the Node, Puppeteer, Backend tests and Fixtures to
support this change.
This logic likely never ran due to a combination of bugs.
* Running `maybe_update_markdown_engines` unconditionally meant that
`if md_engine_key in md_engines` was likely always true.
* Introduced in 65838bb: DEFAULT_MARKDOWN_KEY could never be in
md_engines, so should we have ever reached that code path, we'd have
tried to rebuild all markdown engines every time.
And it also wasn't clearly helpful -- because we fetch all linkifiers
for a realm on every request anyway, we don't really save database
queries by doing a bulk fetch on startup, and doing so would likely
result in a material regression to Zulip's overall startup time that
we were creating markdown engines for large numbers of realms in bulk
during process startup.
Tim Abbott
Dinesh
akshatdalton
Nyior
Siddharth Asthana
Cyril Pletinckx
Alex Vandiver
sahil839
Mateusz Mandera
Anders Kaseorg
Aman Agrawal
Riken Shah
Sumanth V Rao
Vishnu KS
Adam Birds
m-e-l-u-h-a-n
100RABHpy
Abhijeet Prasad Bodas
Arun Sankar