We now send a new user_topic event while muting and unmuting topics.
fetch_initial_state_data now returns an additional user_topics array to
the client that will maintain the user-topic relationship data.
This will support any future addition of new features to modify the
relationship between a user-topic pair.
This commit adds the relevent backend code and schema for the new
event.
Updates documentation to include information about user presence
objects with `aggregated` key (instead of the user's email) where
appropriate.
Also, cleans up spelling, grammar and formatting errors in the
descriptive text for these objects / endpoints.
`render_table` calls itself recursively when it finds nested
`additionalProperties` (i.e. nested objects) in response schema,
to render their properties.
This fixes `render_table` to call `render_desc` along with
calling itself, to render the description of the nested
`additionalProperties` as well.
We separate the permission checks for content and topic edits
by changing the can_edit_topic_or_content to can_edit_topic
and use it only for checking topic edits and check content
edits separately in check_update_message itself. There is no
change in behavior as of this PR, there will be more changes
as per #21739.
This is a prep commit for #21739. The permission checks for
them are essentially separate except the one that message
sender is allowed to edit content and topic irresepctive of
edit_topic_policy setting, and this will too be changed in
future commit and so it will be better to have these checks
separate for readability.
We can also probably create a new function for checking content
edits but currently we only check the sender is same as the use
who is editing and it does not make sense to have a separate
function for just one check. We can do so in future in case we
do some more refactoring for #21739.
The presence of `auto_signup` in idp_settings_dict in the test case
test_social_auth_registration_auto_signup is incompatible with the
previous type annotation of SOCIAL_AUTH_OIDC_ENABLED_IDPS, where `bool`
is not allowed.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
In 3b4f8cc85b,
we added support to `auto_signup`, but this field was not defined in
`SAMLIdPConfigDict`, causing mypy type error in
`SAMLAuthenBackendTest.test_social_auth_registration_auto_signup`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
We construct model instances in the import tool solely for the purpose
of serializing them with the `model_to_dict` helper that returns a
dictionary. Passing `float` to these models' DateTimeField is not
accepted by the type checker. Modifying the dictionary instead avoids
this typing issue.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
We pass the realm_emoji dict to notify_realm_emoji instead
of computing it in notify_realm_emoji itself, because its
caller functions will require the dict in further commits
for creating RealmAuditLog entries.
This commit also renames the legacy event variable to
legacy_event from event because same event is sent
twice if we keep only single variable and change its
value. This was happening because the event value was
changed when the first send_event was actually executed.
We pass the realm_linkfiers list to notify_linkifiers instead of
computing it in notify_linkifiers itself, because its caller
functions will require the list in further commits for creating
RealmAuditLog entries.
Since we not allow enabling public access on limited plan realms,
we set the enable_spectator_access setting to False when downgrading
to a limited plan. Setting is still shown in the UI but it is
disabled.
This commit adds code to send stream creation and peer add events
when stream is changed from private to public. These events are
only sent to users who are not susbcribed to the stream and are
not realm admins as subscribers and realm admins already have
the stream data. This will update the stream data with clients
and will remove the need to reload to view the modified stream.
Fixes#22194.
`ParamSpec` can be easily applied to many use cases of ViewFuncT with
`Concatenate` to help us get rid of the `cast` calls. This does not
include decorators that require the second argument being
`UserProfile`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
As we refactor this, any decorators that `zulip_login_required` depends
on are also refactored to use `ParamSpec`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
We should not monkey-patch message when unnecessary. Adding
`service_queue_events` to `SendMessageRequests` suits our need to type
safety here.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Now that we can assume Python 3.6+, we can use the
email.headerregistry module to replace hacky manual email address
parsing.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We have already checked the size of the file in `upload_file_backend`.
This is the only caller of `upload_message_image_from_request`, and
indirectly the only caller of `get_file_info`. There is no need to
retrieve this information again.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
It is not possible in the codebase to have request.user be None. But
it is possible to have it not present at all. `delattr` is more
appropriate here.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This makes the test cleaner and we don't have to overwrite the `get_host`
callable on `HttpRequest`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`context` as `AccessDeniedError` is incompatible with
`RequestVariableMissingError`. Mypy does not allow such redefinition.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`DiscoverRunner.run_tests` has a return type of `int`. While
`Runner.run_tests` has a wildly different `Tuple[bool, List[str]]`.
This refactors it so that we have the correct return type, by passing
the additional information about failed tests through a side effect to directly
write the failed tests to a file.
Note that we have to make `failed_tests_path` optional as otherwise the method
signature will not be compatible with the supertype.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This reverts part of commit 1432067959
(#17047). The spooky warnings foretold by the comment don’t seem to
show up.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
`context` as `AccessDeniedError` is incompatible with
`RequestVariableMissingError`. Mypy does not allow such redefinition.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
It is really a generator of test cases from the test suite. Which should
be typed as an `Iterable` instead.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`context_data` is only available on `SimpleTemplateResposne`, we can't
narrow `TestHttpResponse` to it because the latter is not in fact a
subtype of `HttpResponse`.
Differently, `redirect_chain` is an attribute that only appears on the
test response when the test client method is called with `follow=True`.
`TestHttpResponse` does not have that by defalut, either.
The occurence of these two cases are rare enough throughout the codebase
and we can't get around that without aggressively overloading the test client
or refactoring `_MonkeyPatchedWSGIResponse` in the upstream.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This eliminates the possibility of having `request.user` as
`RemoteZulipServer` by refactoring it as an attribute of `RequestNotes`.
So we can effectively narrow the type of `request.user` by testing
`user.is_authenticated` in most cases (except that of `SCIMClient`) in
code paths that require access to `.format_requestor_for_logs` where we
previously expect either `UserProfile` or `RemoteZulipServer` backed by
the implied polymorphism.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
In b46af40bd3,
we set this attribute because back then we might call `rate_limit_user`
on `RemoteZulipServer`.
This is no longer the case as `RemoteZulipServer` now has its own rate
limiting and we never call `rate_limit_user` without an `isinstance` check
for `UserProfile`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This verifies that `request_for_logs` is correctly set for requests
with different types of authentication.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`BaseNotes(str, str).get_notes` does not do anything here.
It was introduced in
53888e5a26
by unintendedly.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
We can express the same idea more simply by not passing `user` in
cases where it isn't valid for UserActivity.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Fixes some in-app and documentation references to to new user
announcements and new stream announcements that were still using
'notifications' to refer them. These were missed in the original
pass for updating this language.
A user ran into an issue while upgrading where
ContentType.objects.get(model="realmreactivationstatus",
app_label="zerver") fails due to the object being missing. The reason
for that is to be yet figured out, but the immediate solution is clear
in the sense that the migration can just quit early
if not Confirmation.objects.filter(type=REALM_REACTIVATION).exists() and
that'll effectively skip it for almost all servers (because realm
reactivations links are something that's really only useful on Zulip
Cloud).
We are no longer creating confirmation objects associated with realms
directly. This should test for `RealmReactivationStatus` instead.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Newrelic updated the payload that's sent via the webhook incoming call
causing a bug in the newrelic webhook endpoint.
This fixes the bug by updating the endpoint to respect the new format
of the payload as well as the old format. This should be updated once
the old format is EOLed.
Fixes#22338.
This makes it mandatory to narrow the type of the user to `UserProfile`
before calling this helper.
This effectively removes the `request.user` check. We do not call login_page
anywhere else without getting through the authentication middleware.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
In the test case `test_check_if_every_integration_has_logo_that_exists`,
`urlsplit(integration.logo_url).path` gets inferred as possibly bytes
because `integration.logo_url` might be `None`.
5598b49851/stdlib/urllib/parse.pyi (L166-L169)
TODO:
We might want to ensure that every integration has a `logo_url` with an
explicit assertion in `Integrations` (as noted in the comment).
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`expected_draft_contents` would be inferred as a list of mutable
mappings that only allow `int` as the value, and thus incompatible with
the `draft_dicts[i]` to be expanded. This is fixed by adding explicit
type annotation.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This was added in d43b031a32 and was
unused when it was added. This is an error that we want to remove.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
We likely just wanted to check that `validate_password` succeeds without
any exception being raised. A simple call is sufficient to verify that,
since `validate_password` does not return anything and raises an
exception on failure.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This check was added in 495a8476be.
Now that django-stubs finds that the left operand of the `and` will
always evaluates to `True`, so it makes sense to remove it.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`report/error` is a path where we allow anonymous user access. This has
to be correctly denoted in the type annotation of the user argument of
the view function.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This fixes the type annotations of `Set` derived from `QuerySet` objects,
and add necessary assertions.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`HostRequestMock` has `user` default to `None`, which later gets
initialized as `AnonymousUser`. The separate initialization here is
unnecessary.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Previously, we had a function named create_add_users_to_system_user_groups
for creating system user groups and adding users to them in case when
exports do not contain these groups when importing from other services.
This commit just separates out the call to create_system_user_groups_for_realm
outside the function and the function is thus renamed to
add_users_to_system_user_group. This change is done because in further
commits we would need to update the import order and user groups will
be created before creating user profile objects.
This commit extracts whether a stream is accessible or not
in a new function such that "Subscription" object is passed
by the caller and thus we can use these functions to check
access of multiple streams in a loop without querying the
database in a loop for subscription objects.
This commit renames admin_access_required parameter of
list_to_streams function to unsubscribing_others since that
parameter is used and passed as True only when calling
the function while unsubscribing others and in further
commits we would allow non-admins too to unsubscribe others
based on can_remove_subscribers_group setting.
This commit removes the instances of using "Stream.objects.create"
in tests with make_stream function. This change will help us to
avoid adding code for things to be done after creating streams in
multiple places. We can instead just add it in make_stream function
only.
The .status value of EmailChangeStatus was not being looked
at anywhere to prevent re-use of email change confirmation links. This
is not a security issue, since the EmailChangeStatus object has a fixed
value for the new_email, while the confirmation link has expiry time of
1 day, which prevents any reasonable malicious scenarios.
We fix this by making get_object_from_key look at
confirmation.content_object.status - which applies
generally to all confirmations where the attached object has the .status
attribute. This is desired, because we never want to
successfully get_object_from_key an object that has already been used or
reused.
This makes the prereg_user.status check in check_prereg_key redundant so
it can be deleted.
Type inference does not work when the default value of `REQ` is
non-optional while `ResultT` is optional. Mypy tries to unify
`json_validator` with `Validator[int]` in `invite_users_backend` instead
of the desired `Validator[Optional[int]]` because of the presence of the
default value `settings.INVITATION_LINK_VALIDITY_MINUTES`, which is
inferred to be an `int`. Mypy does not resort to a less specific type but
instead gives up early.
This issue applies to invite_users_backend and generate_multiuse_invite_backend
in zerver.views.invite.
There might be a way that we can add an overload to get around this, but
it's probably not worth the complexity until it comes up again more frequently.
We do in fact allow `invite_expires_in_minutes` to be `None` in places
like `do_invite_users`, `invite_users_backend`, etc, and we have
`settings.INVITATION_LINK_VALIDITY_MINUTES` as the default for them. So
it makes sense to allow having an optional value for this setting. And
since there isn't a way to independently set the value of this constant,
we move it to a different place.
TODO:
This is a temporary fix that should be refactored when the bug is fixed.
The encountered mypy issue: https://github.com/python/mypy/issues/13234
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Iterating over ValidatorError does not necessarily return a tuple. This
uses the `message_dict` property on `ValidationError` instead to make
sure that we always get a `dict` (it otherwise raises an `AttributeError`
when the `dict` is not available).
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
The `RateLimited` exception can be caught by `JsonErrorHandler`, so it
is not necessary to have the try...except statement here. It is also invalid
to pass a string to initialize `RateLimited`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`extra_data` as a `TextField` expects a `str`, but we had been passing
`dict` instead. This is a temporary solution before #18391 to fix the
type annotation.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This adds a `check_string_in` validator to ensure that `op` is actually
valid before we finally return `json_success()`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Fixes#21266.
We want to tie the prereg_user to the MultiUseInvite directly rather
than to the MultiUserInvite's confirmation object, because the latter is
not possible. This is because the flow is that after going through the
multiuse invite link, the PreregistrationUser is created together with a
Confirmation object, creating a confirmation link (via
create_confirmation_link) to which then the user is redirected to finish
account creation. This means that the PreregistrationUser is already
tied to a Confirmation, so that attribute is occupied.
A standard OpenAPI document has no reason to redundantly include this
information in description fields, as standard generators already
display it.
This uniformly moves the URL above the description, which seems fine.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The shared fields of `RawUserInfoDict` and `UserInfoDict` could have
been reused if they both require all keys or none. This is unfortunately
not the case, because subclassing does not override `__total__`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Since we in fact are using the django test client to generate a response
here, the return type should be `TestHttpResponse` instead.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`_callback_str` was removed in Django in 1.10, and other logic relevant
to that particular attribute was removed in
32849b80ad, but not to its entirety. It
does not make sense to fall back to `_callback_str`. The
`get_callback_string` helper is no longer needed.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
On registration and login pages on self-hosted Zulip servers,
it is not helpful and confusing to show the full navigation footer
for the Zulip website. Instead, we should show a minimal footer.
Fixes#21776
Due to an incorrect authorization check in Zulip Server 5.4 and
earlier, a member of an organization could craft an API call that
grants organization administrator privileges to one of their bots.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Previously if `test_forward_address_details` failed, the file
created when setting the `forward_address` may not have been
removed, which would then cause an `EmailNotDeliveredException`
to be raised when then creating a new user in the dev environment.
Wraps the test in a try block, with a finally block for the call
to remove the file.
Before this, a link still couldn't be re-used because it would trip up
exception further down user creation codepaths, but that was still a
bug. check_prereg_key is supposed to correctly validate the key - and
trigger an error page being returned if a key (or for any other reason,
the attached PreregistrationUser object) is reused.
test_validate_email_not_already_in_realm needs to be adjusted, because
it was actually re-using a key.
This reverts commit 40fcf5a633.
This commit triggers bug that we haven't fully tracked down, where web
app clients will continually send `update_message_flags` requests,
that then send out via the events system "0 messages were marked as
read" notices, eventually leading to a load spike.
The Tornado part can likely be fixed by checking if
updated_message_ids is empty, but we need to track down the frontend
bug as well.
`_cache` is not an attribute defined on `BaseCache`, but an
implementation detail of django_bmemcache.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Two endpoints had linked markdown files that were used in
their general descriptions to add warning notes with important
information (`/create-user` and `/get-user-groups`).
This moves the warning content to be inline in the endpoint
descriptions so that the important content is in the OpenAPI
documentation and is still formatted to be rendered in a warning
block.
Deletes `can-create-users-only.md` and `api-members-only.md`
since they were only used for these two endpoint descriptions.
Also, cleans up the other instance of a inline warning block in
an endpoint description (`/fetch-api-key`).
Instead of using `request.POST` to access the `data` parameter used
in the internal `notify_tornado` path, adds `has_request_variables`
decorator and accesses `data` as a `REQ` parameter.
Expands `test_tornado_endpoint` in `test_event_system.py` for
`data` being a required parameter for this path.
Instead of using `request.POST` to access `forward_address` for
the parameter used in `set_forward_address` in `email_page`, adds
`has_request_variable` decorator and an optional `forward_address`
parameter through the `REQ` framework.
Adds an assertion that `forward_address` is not `None` for `POST`
requests.
Previously, automated stream messages for new user signups were not
being translated into the realm's default language for said messages.
Moves `override_language` context manager so that it wraps the
new user message content in `notify_new_user` and topic string in
`send_message_to_signup_notification_stream`.
Fixes#22510.
The supertype contains `*args` and `**kwargs`, this adapts the signature
of the `get` method to make MarkdownDirectoryView compatible with it.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
The calling arguments here are completely wrong. The first argument
should be `request`, and `self` should never get passed to `.get`.
Because `TemplateView` happened to not use `request`, and we happened
to pass `article` as a keyword argument, this error slipped through.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Updates changelog entry for feature level 1 about GitLab to include
the endpoint with the changes. Also noted that the change updated
a deprecated return value.
Added changes note to the `gitlab` boolean in the
`authentication_methods` return value for the
`/get-server-settings` endpoint.
Part of work on #22102.
Updates the changelog note in feature level 1 about adding None as
a video call provider to include the endpoints where this realm
setting is used.
Updates the OpenAPI doc for the realm setting `video_chat_provider`
to include information about the enum values and meanings.
Part of work on #22102.
Corrects omissions or inconsistencies between the api changelog
and the api documentation for Zulip 3.0, feature level 1,
except for the final two bullet points about GitLab authentication
and adding None as a video call provider option.
The final two bullet points will be addressed in separate commits.
Part of work on #22102.
Initial round of fixes and clean-ups found during audit of
changelog entries for feature levels 1-27, which correspond
to the 3.0 release.
There are a few changes that are not related to those feature
levels, but fit within the context of clean-ups (spelling mistakes
or errors in api documentation formatting/structure/style).
One notable non-3.0 release fix is making all changes notes in
the OpenAPI documentation for 2.x releases use the correct
version numbering-scheme for those releases (e.g. 2.0.0).
Follow-up commits / PRs will address inconsitencies and omissions
for these feature levels found during the audit.
Updates references / language about organization settings that
were previously labeled as "Notifications", but are now labeled
as "Automated messages and emails".
Fixes#22136.
Co-authored by: Lauryn Menard <lauryn@zulip.com>
PostgreSQL's `default_statistics_target` is used to track how many
"most common values" ("MCVs") for a column when performing an
`ANALYZE`. For `tsvector` columns, the number of values is actually
10x this number, because each row contains multiple values for the
column[1]. The `default_statistics_target` defaults to 100[2], and
Zulip does not adjust this at the server level.
This translates to 1000 entries in the MCV for tsvectors. For
large tables like `zerver_messages`, a too-small value can cause
mis-planned query plans. The query planner assumes that any
entry *not* found in the MCV list is *half* as likely as the
least-likely value in it. If the table is large, and the MCV list is
too short (as 1000 values is for large deployments), arbitrary
no-in-the-MCV words will often be estimated by the query planner to
occur comparatively quite frequently in the index. Based on this, the
planner will instead choose to scan all messages accessible by the
user, filtering by word in tsvector, instead of using the tsvector
index and filtering by being accessible to the user. This results in
degraded performance for word searching.
However, PostgreSQL allows adjustment of this value on a per-column
basis. Add a migration to adjust the value up to 10k for
`search_tsvector` on `zerver_message`, which results in 100k entries
in that MCV list.
PostgreSQL's documentation says[3]:
> Raising the limit might allow more accurate planner estimates to be
> made, particularly for columns with irregular data distributions, at
> the price of consuming more space in `pg_statistic` and slightly
> more time to compute the estimates.
These costs seem adequate for the utility of having better search.
In the event that the pgroonga backend is in use, these larger index
statistics are simply wasted space and `VACUUM` computational time,
but the costs are likely still reasonable -- even 100k values are
dwarfed by the size of the database needed to generate 100k unique
entries in tsvectors.
[1]: https://github.com/postgres/postgres/blob/REL_14_4/src/backend/utils/adt/array_typanalyze.c#L261-L267
[2]: https://www.postgresql.org/docs/14/runtime-config-query.html#GUC-DEFAULT-STATISTICS-TARGET
[3]: https://www.postgresql.org/docs/14/planner-stats.html#id-1.5.13.5.3
In `JsonableErrorHandler`, we convert `MissingAuthenticationError` into
a response that has `WWW-Authenticated` set for `/api` or `/json` views.
This covers and verify the value of the header for unauthenticated
access.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
The dangling comment was not very helpful and unclear about the pieces
of code it was referring to.
We expand the part about linking the prereg_user to the created user,
while the part about "revoking other preregistration users" is
redundant, because the relevant code block lower down already has
comments on it with better explanations.
Closes#22274.
This assertion was added in 4b903c5dcd
where it may have made sense, because indeed when doing realm creation
there was always a PreregistrationUser (created because realms were
created via going to a generated realm creation link). With the addition
of the create_realm command that's no longer the case.
It would be unnatural to create a PreregistrationUser in the
realm_creation command, because there is no confirmation link for it to
be tied to - and it just doesn't make sense conceptually.
The intended, correct behavior added in
4b903c5dcd is still maintained - the code
lower down correctly handles the
(prereg_user is None and realm_creation) case.
The type safety of a TypeGuard is unchecked by mypy. While this
particular TypeGuard is safe given the current context, one could
imagine future changes that make it unsafe, so it’s preferable to
avoid unchecked constructs whenever possible.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The field_data sent from client while creating a select
type field is a dict with a number as key.
In development database the field data for "Favorite editor"
field was of different form where the option label was used
as key in the dict.
This commit fixes it to be of the same as it is when creating
a field from web-app. As a result, we also need to update
the tests and this commit also update field_data for other
select-type fields.
This refactors the test case with more explicit type annotations, fixing
type errors discovered provided type annotations for
`CustomProfileField`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This change ensures that we can call the validate and update helper for
custom profile data later.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
We refactor the validator of `update_user_custom_profile_data` to ensure
that the validated variable is properly typed as
a `ProfileDataElementUpdateDict`, so we can call
`validate_user_custom_profile_data` and
`do_update_user_custom_profile_data_if_changed` directly later (unlike
`update_user_backend`, where `value` is allowed to be `None`, the
validator already ensures that no further check is required).
django-stubs types the return type of query.values(...) as a TypedDict.
This makes Dict[str, Any] that we have been using incompatible with it.
We use TypeGuard to ensure that `service_bot_tuples` is correctly
inferred to be `Tuple[int, int]` instead of `Tuple[int, Optional[int]]`.
Given that `bot_type` is optional for `ActiveUserDict`, we need to
narrow `row` to `ActiveBotUserDict` to make sure that `bot_type` is
non-optional. An advantage of this approach is that no assertions or
type casts are needed.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Otherwise mypy infers the type of `expected_result` to be incompatible
with the first argument of `fix_ordering_of_result`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
StreamingHttpResponse is inferred without the isinstance check in the
else branch. We refactor this is shorten the code and also type narrow
it appropriately.
`request.method` is not `None` in normal use cases, unless an
`HttpRequest` is directly instantiated without the method being set.
This situation does not apply to `WSGIRequest` at all.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Asserting response.stream is False is just suggesting the response being
an `HttpResponse`. This removes `StreamingHttpResponse` with the more
generic `HttpResponseBase` with an isinstance-check.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`m.output` is a `list` of `str`s. It does not make sense comparing it to
a `str`. Guessed the intention here is to use `self.assert_length`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Items in `django.core.mail.outbox` are by default typed as the less
general `EmailMessage` type. Before accessing the attribute
`alternatives`, we need to narrow the type to `EmailMultiAlternatives`.
Then narrow the tuple value we want to access to `str` before using
it in `assertIn` or `self.normalize_string`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Instead of mutating the original `QueryDict`, we shall create a new
one when updating the `QueryDict`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
The body of `json_success` should be a `Mapping[str, object]`, optional
value is not allowed here.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
We use this decorator on subclasses of `MigrationsTestCase`, which does
not have `self`s being `MigrationsTestCase`, but the corresponding
subclass.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
It's hard to come up with a realistic story where this would matter:
SHARED_SECRET is generated automatically during server setup at the
same time as SECRET_KEY, which is a required setting, but it seems
preferable to be explicit that this is a required parameter for the
internal_notify authentication model.
Instead of using request.POST to get any potential `secret`
parameter used in `authenticate_notify` for `internal_notify_view`
decorator, moves it to the REQ framework parameters as `req_secret`.
Updates existing tests to explicitly test for a request without
`secret` parameter, which defaults to `None`; this is also tested
in `test_event_system.py`.
Similar to the previous commit, we should access request.user only
after it has been initialized, rather than having awkward hasattr
checks.
With updates to the settings comments about LogRequests by tabbott.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`request.user` gets set in Django's `AuthenticationMiddleware`, which
runs after our `HostDomainMiddleware`.
This makes `hasattr` checks necessary in any code path that uses the
`request.user` attribute. In this case, there are functions in
`context_processors` that get called in the middleware.
Since neither `CsrfMiddleware` nor `HostDomainMiddleware` are required
to run before `AuthenticationMiddleware`, moving it two slots up in
`computed_settings` is sufficient to avoid the `hasattr` checks.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
A request that has went through the auth middleware shouldn't have
`.user` being `None`. We should use `AnonymousUser` by default to
represent unauthenticated users.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
If the emoji name contains forward slashes, the `str` converter
would treat it as a URL delimiter. Instead use the path converter, so
that forward slashes are included in the emoji name variable.
Fixes#22377
This commit removes "role" field from subscription
objects since we are not moving forward with stream
administrator concept and instead working on new
permssions model as per #19525.
This commit removes WILDCARD_MENTION_POLICY_STREAM_ADMINS
option of wildcard_mention_policy since we are not moving
forward with stream administrator concept and instead working
on new permssions model as per #19525.
We also add a migration to change wildcard_mention_policy of
existing realms to WILDCARD_MENTION_POLICY_ADMINS. This change
is fine since we were already treating both the setting values
as same as stream admin concept was not implemented completely.
This commit removes the is_stream_admin property of Subscription
model and also updates check_stream_access_for_delete_or_update
to not return true when is_stream_admin is True.
We also removes the relevant tests.
This change is done as we would not be moving forward with the
stream administrator concept as we have decided to modify the
permissions model as per #19525.
As discussed in the new comments results in a better failure mode if
an error occurs while adding subscriptions; running the merge tool
again after fixing whatever caused the error will work just fine.
The name does not really comply with the actual behavior of
the decorator since it returns True for an unauthenticated user.
This makes it clear that the 2fa check only applies to users that
are already logged in.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This replaces user.is_verified with is_2fa_verified.
The helper does extra checks such that the user being checked for 2fa
authentication status is valid.
`request.user.is_verified` is functionally the same as `is_verified`
from `django_otp.middleware`, except that the former is monkey-patched
onto the user object by the 2FA middleware. We use the latter wrapped
in `is_2fa_verified` instead to avoid accessing the patched attribute.
See also: 6b24d56e59/docs/source/overview.rst (authentication-and-verification)
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This simulates the situation in which the user is not
authenticated (as an AnonymousUser) and have 2FA enabled.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Adds a note to the `content` and `stream_id` parameters for the
`/update-message` endpoint that indicates these parameters throw
an error when sent in the same request.
Also, updates the main description of the endpoint to include
changing a message's stream. And updates some of the parameter
descriptions to be more consistent with each other and clear.
Zulip Server 2.1.0 and above have a UI tool, accessible only to server
owners and server administrators, which provides a way to download a
“public data” export. While this export tool is only accessible to
administrators, in many configurations server administrators are not
expected to have access to private messages and private
streams. However, the “public data” export which administrators could
generate contained the attachment contents for all attachments, even
those from private messages and streams.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
To prevent new contributors who are adding new user settings from
being confused about the legacy dicts for display settings and
notifications settings, we create `modern_settings` and
`modern_notifications_settings` dicts with comments documenting
that new user settings should be added to one of them.
Note that the `modern_notification_settings` is a placeholder for
new notifications settings as there have been no new notification
settings added since 430c5cb8 (when the legacy dicts were created)
and is currently annotated as `Dict[str, Any]` (which can be
removed if/when a new setting is added).
Updates `git-webhook-url-with-branches.md` (and two files that use
that file as an include link) for some of the follow-ups from #22315
to the Markdown parser. With this fix, all integrations docs that
reference this file as an include link should render the url as a
div element with `.codehilite` class.
The mypy django-stubs plugin incorrectly infers optional value for order,
which makes the `value_list` to be typed as `Iterable[Optional[int]]`.
We use a type cast here to ensure that
`try_reorder_relam_custom_profile_fields` won't causes mypy to complain
about it.
TODO: Remove the cast when https://github.com/typeddjango/django-stubs/issues/444 gets resovled.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`member_ids` needs to be defined as an `Iterable` as it will otherwise
inferred to have incompatible types in the else branch.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
TODO: For now, we import `_QuerySet` as `ValuesQuerySet`. But there
is a convenient reexport of `ValuesQuerySet` in `django_stubs_ext`
that does the same thing. Once we get django-stubs integrated,
we should import `ValuesQuerySet` from `django_stubs_ext` instead.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
The pattern of using the same variable to apply filters
or alter the `QuerySet` in other ways might produce `QuerySet`s
with incompatible types. This behavior is not allowed by mypy.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
The returned dictionary is not at all used outside the function, so it's
sufficient to make it available only within the helper function itself.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This ensure that the return type is compatible with the
actual type of `realm.realmdomain_set.values`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
To explain the rationale of this change, for example, there is
`get_user_activity_summary` which accepts either a `Collection[UserActivity]`,
where `QuerySet[T]` is not strictly `Sequence[T]` because its slicing behavior
is different from the `Protocol`, making `Collection` necessary.
Similarily, we should have `Iterable[T]` instead of `List[T]` so that
`QuerySet[T]` will also be an acceptable subtype, or `Sequence[T]` when we
also expect it to be indexed.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
We were blindly adding / removing flag from UserMessages without
check if they even need to be updated.
This caused server to repeatedly update flags for messages which
already had been updated, creating a confusion for other clients
like mobile.
Fixes#22164
Added 2 new functions which check if a flag is present in a
user message or not using bit comparison.
Refactor `where_unread/starred/active_push_notification` to use
these functions.
Existing tests are enough to check if these functions work correctly.
Resizing emoji can fail, especially for animated GIFs; in such cases,
it is useful to have the original data on hand, to be able to dissect
the failure.
This was removed in Django 4.0 except in historical migrations. We
might as well replace it everywhere.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Our uWSGI configuration doesn’t correctly activate our virtualenv. We
should investigate that, but until we do, we need to invoke html2text
by an absolute path.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This was for the old /messages/latest API that was removed in commit
e06722657a.
If we wanted a new check like this, it shouldn’t go in zulip_finish,
because that only runs when the client gets an asynchronous response
from polling an initially-empty queue, and not when the client gets a
synchronous response from polling a nonempty queue.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We wrap methods of the django test client for the test suite, and
type keyword variadic arguments as `ClientArg` as it might called
with a mix of `bool` and `str`.
This is problematic when we call the original methods on the test
client as we attempt to unpack the dictionary of keyword arguments,
which has no type guarantee that certain keys that the test client
requires to be bool will certainly be bool.
For example, you can call
`self.client_post(url, info, follow="invalid")` without getting a
mypy error while the django test client requires `follow: bool`.
The unsafely typed keyword variadic arguments leads to error within
the body the wrapped test client functions as we call
`django_client.post` with `**kwargs` when django-stubs gets added,
making it necessary to refactor these wrappers for type safety.
The approach here minimizes the need to refactor callers, as we
keep `kwargs` being variadic while change its type from `ClientArg`
to `str` after defining all the possible `bool` arguments that might
previously appear in `kwargs`. We also copy the defaults from the
django test client as they are unlikely to change.
The tornado test cases are also refactored due to the change of
the signature of `set_http_headers` with the `skip_user_agent` being
added as a keyword argument. We want to unconditionally set this flag to
`True` because the `HTTP_USER_AGENT` is not supported. It also removes a
unnecessary duplication of an argument.
This is a part of the django-stubs refactorings.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This is a prep commit for tightening the types for our wrapped test
client.
The callers of the test client methods are refactored to either call
them without unpacking at all or create a TypedDict for the keyword
arguments to be unpacked. This allows the type checker to know exactly what
keys are present and their corresponding type.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`POST` is an immutable attribute and `_files` is an internal attribute
of `HttpRequest`. With type annotations provided by `django-stubs`, mypy
stops us from modifying these attributes. This uses `cast` and `setattr`
to avoid typing issues.
This is a part of django-stubs refactorings.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
We no longer need to access the internal `LANGUAGE_CODE` attribute by
using `django.utils.translation.get_language`.
A test case overriding the translation is added to ensure the password
reset form sending to users requested from a wrong domain is properly
translated.
This is a part of django-stubs refactorings.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
For the same reason as `handler_id` has, we define `_request`
as an attribute. Note that the name `request` is already taken.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This prevents us from relying on a side-effect of `allocate_handler_id`
that monkey-patches `handler_id` on the `AsyncDjangoHandler` object,
allowing mypy to acknowledge the existence of `handler_id` as an `int`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This ensures that all the keyword arguments in `move_rows`
have the correct types. Note that `returning_id` is supposed to be a
flag instead of a `Composable` `Literal`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This fixes inclusion of a multi-paragraph file into a list item.
Followup to commit dc33a0ae67 (#22315).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
markdown-include is GPL licensed.
Also, rewrite it as a block processor, so that it works correctly
inside indented blocks.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Sometimes (e.g. when moving an old realm out of the way of an import
into that name) we do *not* wish to add a redirect realm. Add a flag
to support that.
We previously parsed any request with method other than {GET, POST} and
Content-Type other than multipart/form-data as if it were
application/x-www-form-urlencoded.
Check that Content-Type is application/x-www-form-urlencoded before
parsing the body that way. Restrict this logic to {DELETE, PATCH,
PUT} (having a body at all doesn’t make sense for {CONNECT, HEAD,
OPTIONS, TRACE}).
Signed-off-by: Anders Kaseorg <anders@zulip.com>
When there is no topic/stream being change, `propagate_mode` becomes
unnecessary. We add an assertion to ensure that the previous assumption
that `propagate_mode` is not `None` still holds when either `topic_name`
or `new_stream` is not `None`.
We can possibly improve this by overloading `do_update_message` and
`check_update_message`, but that's beyond the scope of the PR and
feasibility of doing that should also be further discussed.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Without an explicit type annotation, mypy infers the type of
values in `page_params` upon its initialization as a `Union`, while
other computed values haven't been assigned yet. We break this
over-conservative inferred type by annotating `page_params` as a
`Dict[str, object]`.
We could have created a `TypedDict` to have accurate type annotation
for all of the fields, but it does not seem worth it at this point since
the data structure is not widely used in the backend.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
We don't have a specific type for the reports returned from the error
logging handlers. The check is necessary as they are currently typed
as `Dict[str, object]` in `run_handler`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`request.session` is `django.contrib.sessions.backends.base.SessionBase`
`instead of django.contrib.sessions.models.Session`.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
While it is possible to have `stream.recipient_id` being `None`,
the code works under the assumption that it is not. Potentially
we will get a runtime error, but it is not quite explicit without
the assertion.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This change is solely for removing the attribute error that mypy raises when we
access `pg_version` on `connection`. django-stubs annotate `connection` as
`BaseDatabaseWrapper` while it is actually a proxy object, so we cannot
use an regular assertion with isinstance to narrow the type.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
The “validator” component of the tuple does not follow the Validator
contract as of 7e9db327b3 (#15498).
Define a separate type for it.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Tim Abbott
Kartik Srivastava
Lauryn Menard
Sahil Batra
Zixuan James Li
Mateusz Mandera
Anders Kaseorg
Lauryn Menard
rodwan-bakkar
Adam Sah
Aman Agrawal
Christopher Chong
ritikBhandari
Alex Vandiver
Ganesh Pawar
Alya Abbott
evykassirer
Zixuan James Li