This adds support to accepting extra_data being dict from remote
servers' RealmAuditLog entries. So that it is forward-compatible with
servers that have migrated to use JSONField for RealmAuditLog just in
case. This prepares us for migrating zilencer's audit log models to use
JSONField for extra_data.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Part of splitting creating and editing scheduled messages.
Should be merged with final commit in series. Breaks tests.
Removes `scheduled_message_id` parameter from the create scheduled
message path.
Prep commit for splitting create/edit endpoint for scheduled
messages.
Because of `test-api` runs the tests in alphabetical order based on
the `operationId`, we need two scheduled messages in the test database.
The first for the curl example delete (delete-scheduled-message) and
the second for the curl example update (update-scheduled-message).
Prep commit for adding the scheduled-message endpoints to the API
documentation.
Adds a scheduled message for Iago in the test database so that it
can be deleted in the delete cURL example in the api-test suite.
This implements the core of the rewrite described in:
For the backend data model for UserPresence to one that supports much
more efficient queries and is more correct around handling of multiple
clients. The main loss of functionality is that we no longer track
which Client sent presence data (so we will no longer be able to say
using UserPresence "the user was last online on their desktop 15
minutes ago, but was online with their phone 3 minutes ago"). If we
consider that information important for the occasional investigation
query, we have can construct that answer data via UserActivity
already. It's not worth making Presence much more expensive/complex
to support it.
For slim_presence clients, this sends the same data format we sent
before, albeit with less complexity involved in constructing it. Note
that we at present will always send both last_active_time and
last_connected_time; we may revisit that in the future.
This commit doesn't include the finalizing migration, which drops the
UserPresenceOld table.
The way to deploy is to start the backfill migration with the server
down and then start the server *without* the user_presence queue worker,
to let the migration finish without having new data interfering with it.
Once the migration is done, the queue worker can be started, leading to
the presence data catching up to the current state as the queue worker
goes over the queued up events and updating the UserPresence table.
Co-authored-by: Mateusz Mandera <mateusz.mandera@zulip.com>
Servers that had upgraded from a Zulip server version that did not yet
support the user_uuid field to one that did could end up with some
mobile devices having two push notifications registrations, one with a
user_id and the other with a user_uuid.
Fix this issue by sending both user_id and user_uuid, and clearing
This commit updates the pattern for dealing with tuples
returned by the delete() query.
The '(num_deleted, ignored) = ModelName.objects.filter().delete()'
pattern is preferred due to better readability.
We avoid the pattern '(num_deleted, _)' because Django uses _
for translation, which may lead to future bugs.
To avoid people calling "create_user_group" instead of
"check_add_user_group", we rename it to make its purpose clearer.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Since this function creates a new user group into the database,
it is more appropriate to have it not as a generic "lib" function
but as an "action".
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
The Django convention is for __repr__ to include the type and __str__
to omit it. In fact its default __repr__ implementation for models
automatically adds a type prefix to __str__, which has resulted in the
type being duplicated:
>>> UserProfile.objects.first()
<UserProfile: <UserProfile: emailgateway@zulip.com <Realm: zulipinternal 1>>>
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The post-delete signal on AlertWord clears the realm cache; when it is
called repeatedly, this results in re-fetching the realm object O(n)
times, where n scales by number of users in the database.
Disconnect this cache-clearing signal before removing the AlertWord
entries, and reconnect it afterwards. This is not thread-safe, but
this section is single-threaded. It is also probably unnecessary to
re-connect the signal, as rest of `./manage.py populate_db` does not
delete AlertWord objects, but cleanliness dictates doing the
re-connection.
This drops the time to repeatedly run:
python3 ./manage.py populate_db --num-messages=0 --extra-users=1000
...from 47 seconds to 36 seconds.
This commits update the code to use user-level email_address_visibility
setting instead of realm-level to set or update the value of UserProfile.email
field and to send the emails to clients.
Major changes are -
- UserProfile.email field is set while creating the user according to
RealmUserDefault.email_address_visbility.
- UserProfile.email field is updated according to change in the setting.
- 'email_address_visibility' is added to person objects in user add event
and in avatar change event.
- client_gravatar can be different for different users when computing
avatar_url for messages and user objects since email available to clients
is dependent on user-level setting.
- For bots, email_address_visibility is set to EVERYONE while creating
them irrespective of realm-default value.
- Test changes are basically setting user-level setting instead of realm
setting and modifying the checks accordingly.
Black 23 enforces some slightly more specific rules about empty line
counts and redundant parenthesis removal, but the result is still
compatible with Black 22.
(This does not actually upgrade our Python environment to Black 23
yet.)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
These were useful as a transitional workaround to ignore type errors
that only show up with django-stubs, while avoiding errors about
unused type: ignore comments without django-stubs. Now that the
django-stubs transition is complete, switch to type: ignore comments
so that mypy will tell us if they become unnecessary. Many already
have.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This commit adds the OPTIONAL .realm attribute to Message
(and ArchivedMessage), with the server changes for making new Messages
have this set. Old Messages still have to be migrated to backfill this,
before it can be non-nullable.
Appropriate test changes to correctly set .realm for Messages the tests
manually create are included here as well.
Previously, we type the model fields with explicit type annotations
manually with the approximate types. This was because the lack of types
for Django.
django-stubs provides more specific types for all these fields that
incompatible with our previous approximate annotations. So now we can
remove the inline type annotations and rely on the types defined in the
stubs. This allows mypy to infer the types of the model fields for us.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Small follow-up to d86e4ac34d.
get_ makes it sound like it doesn't have side-effects, when these are
actually much like the django ORM .get_or_create function.
These are used for creating huddles and private messages (and some
UserPresence objects). It'd be really weird, and potentially create some
Messages that break our assumptions, for this to end up involving users
in multiple realms.
I believe currently this hasn't been happening, because when
this line runs, there are only users in "zulip" realm and system bots in
"zulipinternal" - but the query has been excluding bots already.
Still, this query should be explicit about grabbing users from a single
realm. This will also be helpful for the work adding the denormalized
Message.realm field - so that the realm of Message objects that get
manually created in generate_and_send_messages can be simply set to
"zulip" with confidence that it's correct.
In the presence of **kwargs, this is required by the Concatenate type
expected by default_never_cache_responses.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
In 5c49e4ba06, we neglected to include
the CSRF and caching decorators required for all API views in the new
remote_server_dispatch function.
I'm not sure why our automated tests didn't catch this, but this made
the remote server API endpoints nonfunctional in a production
environment.
This change incorporate should_rate_limit into rate_limit_user and
rate_limit_request_by_ip. Note a slight behavior change to other callers
to rate_limit_request_by_ip is made as we now check if the client is
eligible to be exempted from rate limiting now, which was previously
only done as a part of zerver.lib.rate_limiter.rate_limit.
Now we mock zerver.lib.rate_limiter.RateLimitedUser instead of
zerver.decorator.rate_limit_user in
zerver.tests.test_decorators.RateLimitTestCase, because rate_limit_user
will always be called but rate limit only happens the should_rate_limit
check passes;
we can continue to mock zerver.lib.rate_limiter.rate_limit_ip, because the
decorated view functions call rate_limit_request_by_ip that calls
rate_limit_ip when the should_rate_limit check passes.
We need to mock zerver.decorator.rate_limit_user for SkipRateLimitingTest
now because rate_limit has been removed. We don't need to mock
RateLimitedUser in this case because we are only verifying that
the skip_rate_limiting flag works.
To ensure coverage in add_logging_data, a new test case is added to use
a web_public_view (which decorates the view function with
add_logging_data) with a new flag to check_rate_limit_public_or_user_views.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This allows us to avoid importing from zilencer conditionally in
zerver.lib.rate_limiter, as we make rate limiting self-contained now.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This refactors the test case alongside, since normal views accessed by
remote server do not get rate limited by remote server anymore.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
`remote_server_path` allows us to get rid of all the `validate_entity`
calls in `zilencer.views` and remove all the `Union` type annotations
in the signatures of the authenticated view functions.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This allows us to separate the zilencer paths from other JSON paths,
with explicit type annotation expecting `RemoteZulipServer` as the
second parameter of the handler using
authenticated_remote_server_view.
The test case is also updated to remove a test for a situation that no
longer occurs anymore, since we don't perform subdomain checks on
remote servers.
Signed-off-by: Zixuan James Li <p359101898@gmail.com>
Adds an API endpoint for accessing read receipts for other users, as
well as a modal UI for displaying that information.
Enables the previously merged privacy settings UI for managing whether
a user makes read receipts data available to other users.
Documentation is pending, and we'll likely want to link to the
documentation with help_settings_link once it is complete.
Fixes#3618.
Co-authored-by: Tim Abbott <tabbott@zulip.com>
This commit modifies bulk_create_users to add the users to the
respective system groups. And due to this change, now bots in
development environment are also added to system groups.
Tests are changed accordingly as more UserGroupMembeship objects
are created.
`_cache` is not an attribute defined on `BaseCache`, but an
implementation detail of django_bmemcache.
Signed-off-by: Zixuan James Li <p359101898@gmail.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.
We increase the total number of messages, since increasing the number
of topics would otherwise have the side effect of making it hard to
find longer conversations.
We do not allow keeping vacant private streams as we deactivate them
when all users are unsubscribed, so it is better to add at least one
user in the initial private stream created while creating realm.
This commit adds code to copy the realm-level default of
settings while creating users through bulk_create_users.
We do not directly call 'copy_default_settings' as it
calls ".save()" but here we want to bulk_create the objects
for efficiency.
We also add the code to set realm-default of enter_sends as
True for the Zulip dev server as done in 754b547e8 and thus
we remove enter_sends argument from create_user_profile as
it is of no use now.
Bots don't generally do API requests to mark messages as read. If they
did, it's likely because the developer of the bot wants them to appear
in read receipts or similar (E.g. as an indication of what messages
have been processed).
So we should avoid setting the read flag on bot messages the test
database.
76deb30312 changed this to not just be the URL, but rather a
prefixed hash of the URL, but failed to update this location which
wrote to it. This meant that this pre-population step was writing to
the wrong keys in the durable cache, and thus ineffective.
Then, da33b72848 switched the cache to be in-memory, making this
write to the wrong keys in an in-process memory store. There is no
way to pre-fill this sort of cache, except at server start-up.
Finally, and most fundamentally, 8c0c9ca7a4 then disabled
`inline_url_embed_preview` by default, making the code entirely moot.
Remove the triply-unnecessary code.
We now call this function inside do_create_user(...,
realm_creation=True), which generally improves readability and
robustness of the codebase.
This fixes a bug where this onboarding content was not correctly done
when creating a realm via LDAP, and also will be important as we add
new code paths that might let you create a realm.
This commit adds users to the appropriate system user group
based on their role. We also change the user groups when
changing role of the user.
We also add migration to add existing users to the appropriate
user groups.
This commit adds update_users_in_full_members_system_group which
is currently used to update the full members group on changing
role of a user. This function will be modified in next commit such
that it can be used to update full members group on changing
waiting_period_threshold setting of realm.
This is the first step to making the full switch to self-hosted servers
use user uuids, per issue #18017. The old id format is still supported
of course, for backward compatibility.
This commit is separate in order to allow deploying *just* the bouncer
API change to production first.
This reverts bc15085098 (which provided
not justification for its change) and moves further, down to 2 retries
from the default of 5.
10 retries, with exponential backoff, is equivalent to sleeping 2^11
seconds, or just about 34 minutes (though the code uses a jitter which
may make this up to 51 minutes). This is an unreasonable amount of
time to spend in this codepath -- as only one worker is used, and it
is single-threaded, this could effectively block all missed message
notifications for half an hour or longer.
This is also necessary because messages sent through the push bouncer
are sent synchronously; the sending server uses a 30-second timeout,
set in PushBouncerSession. Having retries which linger longer than
this can cause duplicate messages; the sending server will time out
and re-queue the message in RabbitMQ, while the push bouncer's request
will continue, and may succeed.
Limit to 2 retries (APNS currently uses 3), and results expected max
of 4 seconds of sleep, potentially up to 6. If this fails, there
exists another retry loop above it, at the RabbitMQ layer (either
locally, or via the remote server's queue), which will result in up to
3 additional retries -- all told, the request will me made to FCM up
to 12 times.
After failing to notice a place where we wanted to hide timezone
information, we decided to add timezones to some of the test
users, so that we can better consider the effects of timezones
when manually testing.
Testing:
* ran populate_db and confirmed users had timezones in the UI
* updated test_populate_db.py
Since we do not allow to remove owners from bots, it is better
to keep owners for the bots in development environment as well.
We need to change puppeteer tests here because now desdemona
already has bots in dev server and thus "Active bots" section
is opened by default in the settings instead of "Add a new bot"
section.
Adds request as a parameter to json_success as a refactor towards
making `ignored_parameters_unsupported` functionality available
for all API endpoints.
Also, removes any data parameters that are an empty dict or
a dict with the generic success response values.
As @timabbott mentioned on #20577, this command was mostly useful
during early development of the feature, and is no longer needed now
that we have an API for accomplishing the same thing.
Fixes “RemovedInDjango40Warning: Passing None for the middleware
get_response argument is deprecated.” from LogRequests().
Signed-off-by: Anders Kaseorg <anders@zulip.com>
get_remote_server_by_uuid (called in validate_api_key) raises
ValidationError when given an invalid UUID due to how Django handles
UUIDField. We don't want that exception and prefer the ordinary
DoesNotExist exception to be raised.
APNs payloads nest the zulip-custom data further than the top level,
as Android notifications do. This led to APNs data silently never
being truncated; this case was not caught in tests because the mocks
provided the wrong data for the APNs structure.
Adjust to look in the appropriate place within the APNs data, and
truncate that.
As explained in the comments in the code, just doing UUID(string) and
catching ValueError is not enough, because the uuid library sometimes
tries to modify the string to convert it into a valid UUID:
>>> a = '18cedb98-5222-5f34-50a9-fc418e1ba972'
>>> uuid.UUID(a, version=4)
UUID('18cedb98-5222-4f34-90a9-fc418e1ba972')
Given that these values are uuids, it's better to use UUIDField which is
meant for exactly that, rather than an arbitrary CharField.
This requires modifying some tests to use valid uuids.
Enable spectator access for test `zulip` realm in developement
setup.
Add option in `do_create_realm` to configure
`enable_spectator_access` field of `Realm`.
Zixuan James Li
Lauryn Menard
Alex Vandiver
Anders Kaseorg
Tim Abbott
Mateusz Mandera
Prakhar Pratyush
Sahil Batra
Dinesh
Zixuan James Li
evykassirer
Steve Howell
Lauryn Menard
Eeshan Garg
Aman Agrawal