The `django-sendfile2` module unfortunately only supports a single
`SENDFILE` root path -- an invariant which subsequent commits need to
break. Especially as Zulip only runs with a single webserver, and
thus sendfile backend, the functionality is simple to inline.
It is worth noting that the following headers from the initial Django
response are _preserved_, if present, and sent unmodified to the
client; all other headers are overridden by those supplied by the
internal redirect[^1]:
- Content-Type
- Content-Disposition
- Accept-Ranges
- Set-Cookie
- Cache-Control
- Expires
As such, we explicitly unset the Content-type header to allow nginx to
set it from the static file, but set Content-Disposition and
Cache-Control as we want them to be.
[^1]: https://www.nginx.com/resources/wiki/start/topics/examples/xsendfile/
Enforcing a consistent `type` helps us double-check that we're not
playing fast-and-loose with any file paths for local files. As noted
in the comment, this is purely for defense-in-depth.
Passing `write_local_file` a consistent `type` requires removing the
"avatars" out of `realm_avatar_and_logo_path` -- which makes it
consistent across upload backends.
This, in turn, requires a compensatory change to zerver.lib.export, to
be explicit that the realm icons are exported from the avatars
directory. This clarity is likely an improvement.
sendfile already applied a Content-Disposition header, but the
algorithm may provide both `filename=` and `filename*=` values (which
is potentially confusing to clients) and incorrectly slash-escapes
quotes in Unicode strings.
Django provides a correct implementation, but it is only accessible to
FileResponse objects. Since the entire point is to offload the
filehandle handling, we cannot use a FileResponse.
Django 4.2 will make the function available outside of FileResponse.
Until then, extract our own Content-Disposition handling, based on
Django's.
We remove the very verbose comment added in d4360e2287, describing
Content-Disposition headers, as it does not add much.
Add more tests analogous to existing ones but for different scenarios.
This is mostly boring text, but is important for completeness, since the
notificability logic underneath is subtle.
Split the one giant `test_end_to_end_missedmessage_hook` into many
smaller tests.
This allows us to not worry about resetting database state after each
test case and also allows extracting a lot of common stuff into setUp
and tearDown.
There is probably even more scope of deduplication here (for example,
the mock and the `assert_maybe_enqueue_notifications_call_args` call are
same for all test cases) but that might not be worth the added
complexity.
We also change a few
```
user_profile.<setting> = <value>
user_profile.save()
```
expressions to instead use the `do_change_user_setting` function.
For alert words, we currently don't send email/push notifications --
only desktop notifications. Thus, we don't need to consider alert words
here, since desktop notifications do not utilize the presence status
calculated at this stage.
Tested manually that alert word desktop notifications work as expected.
When we implement email/push notifications for alert words (issues #5137
and #13127), we can add new fields like
`notifications_data.alert_word_email_notify`, similar to the existing
`notifications_data.wildcard_mention_email_notify`, which will allow us
to keep the alert word notifiability check inside the dataclass, similar
to how the mentions checks are done currently. So, even when that
feature is implemented, the code which this commit removes would be
unnecessary.
Intercom sends a HEAD request to validate the webhook URL on their side,
which was not expected in the previous implementation.
This fixes the problem that we send out a confusing error message for it.
Fixes#23912.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This is a best-effort rendering of the "fields" of Slack incoming
hooks, which Slack renders in two columns. We approximate them in a
Markdown table, with some minor in-place replacements.
Fixes#22228.
`check_text_block` transformed its input, making the object it
returned not the same object it was passed; this invalidated it for
use in `check_list`. It is also, in general, unlike all other
validators.
Make it return a TypedDict cast of its input.
If `invite_as` is passed as a number outside the range of a PostgreSQL
`SMALLINT` field, the database throws an exception. Move this exception
to the glass as a validation error to allow better client-side error
handling and reduce database round-trips.
Updates the help center article to match the style and formatting
of "Import from Slack" and replaces existing content with its
corresponding Markdown macro.
When this code was moved from being in zerver in 21a2fd482e, it kept
the `if ZILENCER_ENABLED` blocks. Since ZILENCER and CORPORATE are
generally either both on or both off, the if statement became
mostly-unnecessary.
However, because tests cannot easily remove elements from
INSTALLED_APPS and re-determine URL resolution, we switch to checking
`if CORPORATE_ENABLED` as a guard, and leave these in-place.
The other side effect of this is that with e54ded49c4, most Zulip
deployments started to 404 requests for `/apps` instead of redirecting
them to `https://zulip.com/apps/` since they no longer had any path
configured for `/apps`. Unfortunately, this URL is in widespread use
in the app (e.g. in links from the Welcome Bot), so we should ensure
that it does successfully redirect.
Add the `/apps` path to `zerver`, but only if not CORPORATE_ENABLED,
so the URLs do not overlap.
‘logging.warning("Naive datetime:", item)’ is an invalid call that
crashes with “TypeError: not all arguments converted during string
formatting”. I take that to mean this check has not been tripped in
the six years it’s been there, and can safely be replaced with an
error.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Some email clients (notably, Gmail Web) support automatically threading
emails together if recipients and subjects match[1]. Manual testing
indicated that prefixing a subject with "[bracketed content]" does not
break this threading behavior, but the added checkmark in a resolved
topic's title does. Before sending an email notification, determine
whether the topic is resolved, and pass this information to the Jinja
template to properly format a threadable email subject.
Fixes: #22538
[1]: https://support.google.com/mail/answer/5900
Previously, stream names and topics (without consideration for their
resolution status) were concatenated in Python-land and passed through
to the template. To more cleanly separate concerns, and to prepare for
accounting for topic resolution status being a third, independent,
component of a subject line, instead pass stream and topic strings
independently to the Jinja template, which can format them as it sees
fit.
Additionally, migrate existing EditMessageTest to use this helper
method, with the side effect of migrating the tested flow from a
/json/messages URL to a /api/v1/messages URL.
This commit renames "can_edit_topic_of_any_message" function
in models.py to "can_move_messages_to_another_topic" and
"user_can_edit_topic_of_any_message" function in settings_data.js
to "user_can_move_messages_to_another_topic".
This change is done since topic editing permission does not
depend on message sender now and messages are considered same
irrespective of whether the user who is editing the topic had sent
the message or not. This also makes the naming consistent with
what we use for the label of this setting in webapp and how we
describe this action in help documentation.
This commit changes the topic edit permssions to not depend whether the user
editing the message had sent the message or it was sent by someone else.
We only do backend changes in this commit and frontend changes will be done
in further commits.
Previously, we always allowed topic edits when the user themseleves had
sent the message not considering the edit_topic_policy and the 3-day time
limit. But now we consider all messages as same and editing is allowed only
according to edit_topic_policy setting and the time limit of 3 days in
addition for users who are not admins or moderators.
We change the topic and stream edit permssions to not depend on
allow_message_editing setting in the API and are allowed even
if allow_message_editing is set to False based on other settings
like edit_topic_policy and can_move_message_between_streams.
Fixes a part of #21739.
This uses the linkifier index among the list of linkifiers in the
replacement as the priority to order the replacement order for
patterns in the topic. This avoids having multiple overlapping matches
that each produce a link.
The linkifier with the lowest id will be prioritized when its pattern
overlaps with another. Linkifiers are prioritized over raw URLs.
Note that the same algorithm is used for local echoing and the
backend markdown processor.
Fixes#23715.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This explicitly enforces ordering on the linkifiers. This is useful when
there are overlapping linkifier patterns that matches the same text. In
our current linkifier implementation, this order affects how the
patterns are handled in the markdown processor, with the earlier ones
being prioritized.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
The same pattern being matched multiple times in a topic cannot be
properly ordered using topic_name.find(match_text) and etc. when there
are multiple matches of the same pattern in the topic.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Moves files in `templates/zerver/help/include` that are used
specifically for API documentation pages to be in a new directory:
`templates/zerver/api/include`.
Adds a boolean parameter to `render_markdown_path` to be used
for help center documentation articles.
Also moves the test file `empty.md` to the new directory since
this is the default directory for these special include macros
that are used in documentation pages.
Moves files in `templates/zerver/help/include` that are used
specifically for integrations documentation to be in a new
directory: `templates/zerver/integrations/include`.
Adds a boolean parameter to `render_markdown_path` to be used
for integrations documentation pages.
Track `create_realm` and `new_realm_send_confirm` using
google analytics.
This will help us track number of users who want to
create a new Zulip organization.
Since the removal of `CurlHttpRequest` in Rabbix 6.2, the old script for
setting up the Zabbix integration no longer works.
https://www.zabbix.com/documentation/6.2/en/manual/installation/upgrade_notes_620?hl=CurlHttpRequest#curlhttprequest-removed
This updates the documentation to use `HttpRequest` instead and keep it
up-to-date with the latest Zabbix server. We raise the minimum supported
version from 5.2 to 5.4 because `HttpRequest` was introduced in 5.4.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
‘exit’ is pulled in for the interactive interpreter as a side effect
of the site module; this can be disabled with python -S and shouldn’t
be relied on.
Also, use the NoReturn type where appropriate.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
As we have seen no further cases of this in production since #23215,
increase the severity to an error, and switch from returning a
list (which is not type-safe if the function declares a QuerySet
return) to returning the QuerySet without caching.
Failing to store the result in the cache, with an error, seems
superior to raising an exception; in both cases the next request will
redo the work, but we are guaranteed a worse user experience if we 500
the request.
Ref https://github.com/zulip/zulip/pull/23215#discussion_r994186493
The previous regular expression required a `[^\w]` at the start and
end of the match. This had two unintended effects -- it meant that it
could never match at the start or end of a string, and it meant
that *adjacent* words required *two* non-word characters between them,
as the pattern matches cannot overlap.
Switch to allowing string start/end to anchor the matches, and make
the trailing `[^\w]` be a zero-width look-ahead, to allow the patterns
to overlap. Also remove the spurious `^` within the inner character
classes, which prevented `*foo^bar*` from matching. Finally, add
tests to cover the functionality, which was previously untested.
Updates the Hello World integration documentation and the section
of the related tutorial on documenting the example integration
for the currently used shared macro `create-bot-construct-url.md`.
Also, updates them to use the numbered style currently used in
the majority of the integrations documentation pages.
remove_user_from_user_group's only caller has been removed in 271333301d.
Its usage has been superseded by remove_members_from_user_group.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Doing rapid pace mark-as-unread in the Zulip web application, one
observed assertion failures showing that the server would send an
event containing multiple message IDs but only one of the messages
present in the message_details side data structure.
The cause of this was the "virtual events" compression system; two
flags/remove/read events were being combined by simply concatenating
the lists of events, without any attempt to merge the
`message_details` field on those objects.
The immediate fix is to disable virtual events compression for this
event class, but it's not unlikely we'll need to just eliminate the
virtual_events system entirely, because it seems difficult to make it
soundly handle a message whose state for a given flag changes back and
forth while the client is offline.
But we'll leave that for later, since removing that optimization
deserves more discussion than fixing this event corruption bug.
The naive solution #23465 creates situations where the same user can have
multiple reactions as the base emojis are not unique, e.g. +1::skin2
and +1::skin4 would both reduce to +1 but the userlists are separate.
This solution handles the reduction, merges the same-base reactions,
and deduplicates the userlist.
Co-authored-by: Alex Vandiver <alexmv@zulip.com>
Co-authored-by: rht <rhtbot@protonmail.com>