In a decorator annotated with generic type (ViewFuncT) -> ViewFuncT,
the type variable ViewFuncT = TypeVar(…) must be instantiated to
the *same* type in both places. This amounts to a claim that the
decorator preserves the signature of the view function, which is not
the case for decorators that add a user_profile parameter.
The corrected annotations enforce no particular relationship between
the input and output signatures, which is not the ideal type we might
get if mypy supported variadic generics, but is better than enforcing
a relationship that is guaranteed to be wrong.
This removes a bunch of ‘# type: ignore[call-arg] # mypy doesn't seem
to apply the decorator’ annotations. Mypy does apply the decorator,
but the decorator’s incorrect annotation as signature-preserving made
it appear as if it didn’t.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Also enable warn_unused_ignores. I think the fact that there are so
few of these is good evidence that it’s not a significant burden for
people fixing type errors.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Migrations that add items to a table in the same migration htat
craetes the table require atomic=False.
This fixes this exception:
Running migrations:
Applying zerver.0287_clear_duplicate_reactions... OK
Applying zerver.0288_reaction_unique_on_emoji_code... OK
Applying zerver.0289_tighten_attachment_size... OK
Applying zerver.0290_remove_night_mode_add_color_scheme...Traceback (most recent call last):
File "/home/zulip/deployments/2020-06-22-23-20-36/zulip-py3-venv/lib/python3.6/site-packages/django/db/backends/utils.py", line 84, in _execute
return self.cursor.execute(sql, params)
File "/home/zulip/deployments/2020-06-22-23-20-36/zerver/lib/db.py", line 33, in execute
return wrapper_execute(self, super().execute, query, vars)
File "/home/zulip/deployments/2020-06-22-23-20-36/zerver/lib/db.py", line 20, in wrapper_execute
return action(sql, params)
psycopg2.errors.ObjectInUse: cannot ALTER TABLE "zerver_userprofile" because it has pending trig
We only use this in a few places, but they're really important places
for understanding the types in the codebase, and so it's worth having
a bit of expository documentation explaining how we use it.
(And I expect we'll add more with time).
Fixes#14960.
The default of 6 thread may not be appropriate in certain
configurations. Taking half of the numer of CPUs available to the
process will be more flexible.
With this implementation of the feature of the automatic theme
detection, we make the following changes in the backend, frontend and
documentation.
This replaces the previous night_mode boolean with an enum, with the
default value being to use the prefers-color-scheme feature of the
operating system to determine which theme to use.
Fixes: #14451.
Co-authored-by: @kPerikou <44238834+kPerikou@users.noreply.github.com>
We can now invite new users as realm owners. We restrict only
owners to invite new users as owners both for single invite
and multiuse invite link. Also, only owners can revoke or resend
owner invitations.
Old: a validator returns None on success and returns an error string
on error.
New: a validator returns the validated value on success and raises
ValidationError on error.
This allows mypy to catch mismatches between the annotated type of a
REQ parameter and the type that the validator actually validates.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We had a bug in `validate_against_openapi_schema` that prevented it
from correctly inspecting nested arrays.
Fix the bug and address all the exceptions, either via
EXCLUDE_PROPERTIES or fixing them when simple. Also add a test case
for nested verification.
Attachment objects in production are only created in one place, which
passses a size. Additionally, I verified in multiple production
environments with old data that this never actually happens (or has
happened).
So we should make the data model correctly reflect the possibilities here.
Rename the validator to check_union, to conform
more to Python typing nomenclature.
And we rename one of the test helpers to the
simpler `check_types`. (The test helper
was using "variable" in the "var" sense.)
We assert that the post was successful, to give
more immediate feedback for tests that don't
bother to check the return value and may be
implicitly assuming this method just works in
all cases.
And we also make it more convenient for tests
that are happy-path tests--they don't have to
do the assertion themselves. (And they're still
free to do deeper checks on the json.)
We opt out with allow_fail=True. We probably want
a more direct API eventually for tests that are
clearly trying to test the failure path for
subscribing to streams.
It's possible that a couple tests here that I added
allow_fail=True to just have flawed data setup--
I don't have time to investigate all cases, but
hopefully they will at least stand out more.
The non-search code path here was simulating the response and escaping
logic from get_search_fields by duplicating what it would do with an
empty set of highlight locations.
We can produce much more readable code by just passing an empty list
of locations in this case.
Two things were broken here:
* we were using name(s) instead of id(s)
* we were always sending lists that only
had one element
Now we just send "stream_id" instead of "subscriptions".
If anything, we should start sending a list of users
instead of a list of streams. For example, see
the code below:
if peer_user_ids:
for new_user_id in new_user_ids:
event = dict(type="subscription", op="peer_add",
stream_id=stream.id,
user_id=new_user_id)
send_event(realm, event, peer_user_ids)
Note that this only affects the webapp, as mobile/ZT
don't use this.
Currently the API docs do not specify whether a given API parameter
is to be specified in `query` or in `path`. Edit the docs so as
to show the type of argument right beside argument name.
Currently, the OpenAPI extension for rendering description in docs
cannot parse {!api-admin-only.md!}. Edit order of markdown extensions
in app_filters.py so that rendering of OpenAPI elements takes place
before substitution of files using `include`.
The loop I added here in 5b49839b08 was
ill-conceived. The critical issue was that despite its name,
do_clear_mobile_push_notifications_for_ids does not immediately clear
push notifications (Except in our test suite, where `send_event`
immediately calls into the queue worker code!).
Instead, it queues work to clear those push notifications. Which
means that the first user to declare bankruptcy with a large number of
unreads will fill the queue, and then this will just be an infinite
loop adding more work to the queue.
This fixes a missing unique constraint on the Reactions data model
state when using multiple aliases for an emoji code. As with any
missing unique constraints, we first need to apply a migration that
eliminates violations of the rule; in this case, deleting the
duplicates is correct.
Added unique constraint for "user_profile", "message",
"reaction_type", "emoji_code".
Fixes#15347.
Mostly, this is a change in ordering to make more sense, but we also
fix several names that were clearly confusing.
We restore the convention that each endpoint has the same title at the
top of the page as what we have in the sidebar menu, which appears to
have been violated in many recent updates to API documentation.
api docs filenames are basically the operationId of their endpoint
in zulip.yaml with `_` replaced by `-`. But some operationIds have
changed, so change the affected filenames. Make changes in other
files accordingly.
This adds a new client_capability that clients such as the mobile apps
can use to avoid unreasonable network bandwidth consumed sending
avatar URLs in organizations with 10,000s of users.
Clients don't strictly need this data, as they can always use the
/avatar/{user_id} endpoint to fetch the avatar if desired.
This will be more efficient especially for realms with
10,000+ users because the avatar URLs would increase the
payload size significantly and cost us more bandwidth.
Fixes#15287.
We need this field to avoid O(N) database operations
while fetching realm user data for clients with
`user_avatar_url_field_optional` flag enabled.
Part of #15287.
This extends get_accounts_for_email test by adding a deactivated
user and assert that get_accounts_for_email doesn't return any accounts
for that deactivated user.
Fixes#14807.
With #14378, we regressed back to the state of that
prior to 7e0ea61b00.
We fix this by getting our avatar bucket on
object initialization, and use the appropriate means
of gathering the network location for the urls.
Fixes#14484.
_setup_export_files modifies the zulip realm. We used to
call realm.refresh_from_db in tests after _setup_export_files was
called to make sure that the change is reflected. But sometimes
calling refresh_from_db was missed out here and there.
This commit makes calling refresh_from_db after _setup_export_files
unnecessary.
This commit adds backend support for setting message_retention_days
while creating streams and updating it for an existing stream. We only
allow organization owners to set/update it for a stream.
'message_retention_days' field for a stream existed previously also, but
there was no way to set it while creating streams or update it for an
exisiting streams using any endpoint.
Previously, we had implemented:
<span class="timestamp" data-timestamp="unix time">Original text</span>
The new syntax is:
<time timestamp="ISO 8601 string">Original text</time>
<span class="timestamp-error">Invalid time format: Original text</span>
Since python and JS interpretations of the ISO format are very
slightly different, we force both of them to drop milliseconds
and use 'Z' instead of '+00:00' to represent that the string is
in UTC. The resultant strings look like: 2011-04-11T10:20:30Z.
Fixes#15431.
The term `parameter` is a better word than `argument` for data passed
to an API endpoint; this is why OpenAPI uses in their terminology.
Replace `argument` with `parameter` in the API docs to improve their
readability.
Fixes#15435.
Fixes#14498.
When a topic is moved to a different stream, the message may no
longer be reachable to guest user, if the user is not subscribed
to the new stream.
We used to send message update event to the client in these cases,
which seems to be confusing both to the client updating the message
and the server sending push_notifications for it.
Now, we delete the UserMessage entry for these messages for the
user and send a delete message event to the client; which makes
both push_notification and the event handling client think that
the message was deleted and hence no confusion in the code is
raised.
This makes the system store and track PushDeviceToken objects on
the local Zulip server when using the push notifications bouncer
and includes tests for this.
This is something we need to implement end-to-end encryption for
push notifications. We'll add the encryption key as an additional
property on the local PushDeviceToken object.
It also likely adds some value in the case that a server were to
switch between using the bouncer service and sending notifications
directly, though in practice that's unlikely to happen.
This migration fixes any PreregistrationUser objects that might have
been already corrupted to have the administrator role by the buggy
original version of migration 0198_preregistrationuser_invited_as.
Since invitations that create new users as administrators are rare, it
is cleaner to just remove the role from all PreregistrationUser
objects than to filter for just those older invitation objects that
could have been corrupted by the original migration.
This migration incorrectly swapped the role associated with invitation
objects between members and organization administrators, resulting in
most invitation objects that existed before the upgrade to Zulip
2.0.0-rc1 or later to be incorrectly administrator invitations.
Fixing the migration is safe and will help those installations
upgrading directly from 1.9.x to 2.1.5 or later.
A migration to fix the corrupted records will appear in an upcoming
commit.
The most import change here is the one in maybe_send_to_registration
codepath, as the insufficient validation there could lead to fetching
an expired PreregistrationUser that was invited as an administrator
admin even years ago, leading to this registration ending up in the
new user being a realm administrator.
Combined with the buggy migration in
0198_preregistrationuser_invited_as.py, this led to users incorrectly
joining as organizations administrators by accident. But even without
that bug, this issue could have allowed a user who was invited as an
administrator but then had that invitation expire and then joined via
social authentication incorrectly join as an organization administrator.
The second change is in ConfirmationEmailWorker, where this wasn't a
security problem, but if the server was stopped for long enough, with
some invites to send out email for in the queue, then after starting it
up again, the queue worker would send out emails for invites that
had already expired.
Google has removed the Google Hangouts brand, thus we are removing
them as video chat provider option.
This commit removes Google Hangouts integration and make a migration
that sets all realms that are using Hangouts as their video chat
provider to the default, jitsi.
With changes by tabbott to improve the overall video call documentation.
Fixes: #15298.
Fixes#14828.
Giving the /subdomain/<token>/ url there could feel buggy if the user
ended up using the token in the desktop app, and then tried clicking the
"continue in browser" link - which had the same token that would now be
expired. It's sufficient to simply link to /login/ instead.
This adds support for a "spoiler" syntax in Zulip's markdown, which
can be used to hide content that one doesn't want to be immediately
visible without a click.
We use our own spoiler block syntax inspired by Zulip's existing quote
and math block markdown extensions, rather than requiring a token on
every line, as is present in some other markdown spoiler
implementations.
Fixes#5802.
Co-authored-by: Dylan Nugent <dylnuge@gmail.com>
This adds a new function `get_apns_badge_count()` to
fetch count value for a user push notification and
then sends that value with the APNs payload.
Once a message is read from the web app, the count is
decremented accordingly and a push notification with
`event: remove` is sent to the iOS clients.
Fixes#10271.
Mocking `get_base_payload()` verifies the wrong output
when the code is actually correct. So, its better that
we call the real function here, especially when we are
adding the Apple case.
This line was effectively hardcoding a specific stream_post_policy,
overriding the value already present in the event, to no purpose.
(I believe it got here via cargo-culting induced by #13787.)
This commit removes is_old_stream property from the stream objects
returned by the API. This property was unnecessary and is essentially
equivalent to 'stream_weekly_traffic != null'.
We compute sub.is_old_stream in stream_data.update_calculated_fields
in frontend code and it is used to check whether we have a non-null
stream_weekly_traffic or not.
Fixes#15181.
This likely fix a bug that can leak thousands of messages into the
invalid state where:
* user_message.flags.active_mobile_push_notification is True
* user_message.flags.read is True
which is intended to be impossible except during the transient process
between marking messages as read sending the "remove push
notifications" event.
The bug is that if a user who is declaring bankruptcy with 10,000s of
unreads ends up having the database query to mark all of those as read
take 60s, the Django/uwsgi request will time out and kill the process.
If the postgres transaction still completes, we'll end up with the
second half of this function never being run.
A safer ordering is to do the smaller queries first.
We do this in a loop for correctness in the unlikely event there are
more than 10,000 of these.
Fixes#15285
This event will be used more now for guest users when moving
topic between streams (See #15277). So, instead of deleting
messages in the topic as part of different events which is
very slow and a bad UX, we now handle the messages to delete in
bulk which is a much better UX.
After merging PR #15355 we found that CircleCI may send
super minimal payloads. This does not seem to depend on
.circleci/config.yml since we received two different
types of payloads off of the same configuration.
Signed-off-by: Hemanth V. Alluri <hdrive1999@gmail.com>
This is designed to have no user-facing change unless the client
declares bulk_message_deletion in its client_capabilities.
Clients that do so will receive a single bulk event for bulk deletions
of messages within a single conversation (topic or PM thread).
Backend implementation of #15285.
This commits adds restriction on admins to set message retention policy.
We now only allow only organization owners to set message retention
policy.
Dropdown for changing retention policy is disabled in UI for admins also.
Whenever we use API queries to mark messages as read we now increment
two new LoggingCount stats, messages_read::hour and
messages_read_interactions::hour.
We add an early return in do_increment_logging_stat function if there
are no changes (increment is 0), as an optimization to avoid
unnecessary database queries.
We also log messages_read_interactions::hour Logging stat
as the number of API queries to mark messages as read.
We don't include tests for the case where do_update_pointer is called
because do_update_pointer will most likely be removed from the
codebase in the near future.
This uses `where_active_push_notification()` instead of
the flag condition to keep it consistent with the app
code as we use the same function to filter active push
notifications everywhere else.
Overrides some of internal functions of python-social-auth
to handle native flow.
Credits to Mateusz Mandera for the overridden functions.
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
This adds a powerful end-to-end test for Zulip's API documentation:
For every documented API endpoint (with a few declared exceptions that
we hope to remove), we verify that every API response received by our
extensive backend test suite matches the declared schema.
This is a critical step towards being able to have complete, high
quality API documentation.
Fixes#15340.
When doing query for same topic names in a stream, we should do a
case-insensitive exact match for the topic, since that's the data
model for topics in Zulip.
This is a test case verifying the current codebase produces incorrect
results. All the messages should have been edited in this case
regardless of the topic's case unless they belong to a different
stream.
Generated by pyupgrade --py36-plus --keep-percent-format.
Now including %d, %i, %u, and multi-line strings.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The Python 3.6 style does support non-total and even partially-total
TypedDict, but total gives us better guarantees.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The narrow parameter was incorrectly documented as a one-level array
rather than an array of arrays, and the tests incorrectly expected
this due to a combination of design and implementation bugs.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Use read-only types (List ↦ Sequence, Dict ↦ Mapping, Set ↦
AbstractSet) to guard against accidental mutation of the default
value.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
There seems to have been a confusion between two different uses of the
word “optional”:
• An optional parameter may be omitted and replaced with a default
value.
• An Optional type has None as a possible value.
Sometimes an optional parameter has a default value of None, or None
is otherwise a meaningful value to provide, in which case it makes
sense for the optional parameter to have an Optional type. But in
other cases, optional parameters should not have Optional type. Fix
them.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We triage on the event's "op" field instead
of using the type check.
And then we still get the implicit assertion
that we any "add" event produces a dict
for "subscriptions", since we dereference it
in the assertion for "subscribers".
Fixes#2665.
Regenerated by tabbott with `lint --fix` after a rebase and change in
parameters.
Note from tabbott: In a few cases, this converts technical debt in the
form of unsorted imports into different technical debt in the form of
our largest files having very long, ugly import sequences at the
start. I expect this change will increase pressure for us to split
those files, which isn't a bad thing.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Automatically generated by the following script, based on the output
of lint with flake8-comma:
import re
import sys
last_filename = None
last_row = None
lines = []
for msg in sys.stdin:
m = re.match(
r"\x1b\[35mflake8 \|\x1b\[0m \x1b\[1;31m(.+):(\d+):(\d+): (\w+)", msg
)
if m:
filename, row_str, col_str, err = m.groups()
row, col = int(row_str), int(col_str)
if filename == last_filename:
assert last_row != row
else:
if last_filename is not None:
with open(last_filename, "w") as f:
f.writelines(lines)
with open(filename) as f:
lines = f.readlines()
last_filename = filename
last_row = row
line = lines[row - 1]
if err in ["C812", "C815"]:
lines[row - 1] = line[: col - 1] + "," + line[col - 1 :]
elif err in ["C819"]:
assert line[col - 2] == ","
lines[row - 1] = line[: col - 2] + line[col - 1 :].lstrip(" ")
if last_filename is not None:
with open(last_filename, "w") as f:
f.writelines(lines)
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This commit adds three `.pysa` model files: `false_positives.pysa`
for ruling out false positive flows with `Sanitize` annotations,
`req_lib.pysa` for educating pysa about Zulip's `REQ()` pattern for
extracting user input, and `redirects.pysa` for capturing the risk
of open redirects within Zulip code. Additionally, this commit
introduces `mark_sanitized`, an identity function which can be used
to selectively clear taint in cases where `Sanitize` models will not
work. This commit also puts `mark_sanitized` to work removing known
false postive flows.
This commit adds 'add_query_to_redirect_url' to one additional
function which had not yet been written when
'add_query_to_redirect_url' was introduced. This helper centralizes
URL manipulation for redirects, making it easier to add Pysa
sanitization in subsequent commits.
This new endpoint returns a 'user' dictionary which, as of now,
contains a single key 'is_subscribed' with a boolean value that
represents whether the user with the given 'user_id' is subscribed
to the stream with the given 'stream_id'.
Fixes#14966.
The only clients that should use the typing
indicators endpoint are our internal clients,
and they should send a JSON-formatted list
of user_ids.
We now enforce this, which removes some
complexity surrounding legacy ways of sending
users, such as emails and comma-delimited
strings of user_ids.
There may be a very tiny number of mobile
clients that still use the old emails API.
This won't have any user-facing effect on
the mobile users themselves, but if you type
a message to your friend on an old mobile
app, the friend will no longer see typing
indicators.
Also, the mobile team may see some errors
in their Sentry logs from the server rejecting
posts from the old mobile clients.
The error messages we report here are a bit
more generic, since we now just use REQ
to do validation with this code:
validator=check_list(check_int)
This also allows us to remove a test hack
related to the API documentation. (We changed
the docs to reflect the modern API in an
earlier commit, but the tests couldn't be
fixed while we still had the more complex
semantics for the "to" parameter.)
Currently there are no checks in validate_against_openapi_schema
to check whether the `content` it received actually matched with
the `response code`. For example during testing if a certain endpoint
was returning 400 but it was expected to return 200, then it would
pass schema validation as it would only have `msg` and `result` keys.
Add this validation and fix the wrong response returning points.
Add test to validate example responses in zulip.yaml. Also change
zulip.yaml for some wrong examples or for cases which were not
covered by `test-api`. Also enhance `validate_against_openapi_schema`.
Prevent `JsonableError(_("Missing content"))` from
ever being triggered.
That error wasn't handle by anything, and thus just threw a 500, as
it's not a response to an HTTP request.
The right fix is to adjust the caller to ban the empty string in
content (or content that strips to the empty string).
Closes#15145.
Anders Kaseorg
Tim Abbott
Felix
Mateusz Mandera
orientor
MariaGkoulta
sahil839
Steve Howell
akashaviator
arpit551
Hashir Sarwar
Karim Tayie
Wyatt Hoodes
Vishnu KS
Siddharth Varshney
Rohitt Vashishtha
Dinesh
Aman Agrawal
Clara Dantas
Sara Gulotta
Hemanth V. Alluri
Anders Kaseorg
Graham Bleaney
Steve Howell