Previously, this command would reliably fail:
```
tools/test-backend --skip-provision-check --parallel=3
zerver.tests.test_email_log.EmailLogTest.test_forward_address_details
zerver.tests.test_email_log.EmailLogTest.test_generate_and_clear_email_log
zerver.tests.test_example.TestDevelopmentEmailsLog
```
and now it reliably succeeds. :-)
After hours of fiddling/googling/hair-tearing, I found that
mocking-away Django Connection.send_messages() was the best:
- We're testing Zulip and not Django.
- Mocking at this lower level exercises more of our code.
- EmailLogBackEnd._do_send_messages() helper method added to simplify mocking.
Fixes#21925.
According to the documentation: “Pika does not have any notion of
threading in the code. If you want to use Pika with threading, make
sure you have a Pika connection per thread, created in that thread. It
is not safe to share one Pika connection across threads, with one
exception: you may call the connection method add_callback_threadsafe
from another thread to schedule a callback within an active pika
connection.”
https://pika.readthedocs.io/en/stable/faq.html
This also means that synchronous Django code running in Tornado will
use its own synchronous SimpleQueueClient rather than sharing the
asynchronous TornadoQueueClient, which is unfortunate but necessary as
they’re about to be on different threads.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We previously forked tornado.autoreload to work around a problem where
it would crash if you introduce a syntax error and not recover if you
fix it (https://github.com/tornadoweb/tornado/issues/2398).
A much more maintainable workaround for that issue, at least in
current Tornado, is to use tornado.autoreload as the main module.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This matches the metadata that we store in the database, and means
that the S3 metadatata invariant of always having a `user_profile_id`
in the metadata.
This does not fix existing imports, which may still have missing
`user_profile_id`s.
The changes in the last few commits changed the semantics of the
organization default language to no longer be the primary source of
information for a user's language when creating a new account.
Here, we change the settings UI and /help/ documentation to reflect
this.
This commit reads the browser locale during user registration, and
sets it as default language of user if it is supported by Zulip.
Otherwise, it is set to realm's default language.
This commit adds get_browser_language_code function
which returns None if there is no Accept-language
header in the request or Accept-languge header contains
only unsupported languages or all languages (meaning
header having value of '*'). Otherwise it returns the
language with highest weight/quality-value.
slack_incoming webhook previously used has_request_variables to
extract payload from HttpRequest object first, before trying to
access HttpRequest.body again in view.py. This caused an error
when one sends a request without payload - it is forbidden to
read from request data stream twice.
Instead of relying on has_request_variables, this PR extracts
payload depending on content type in view.py directly to avoid
reading request data stream twice.
Fixes#19056.
To provide a smoother experience of accessing a web public stream,
we don't ask user to login unless user directly requests a
`/login` URL.
Fixes#21690.
This refactored `get_mentioned_user_group_name` from
`zerver/lib/email_notifications.py` to
`zerver/lib/notification_data.py` just after
`get_user_group_mentions_data` to indicate the logical
similarity between them.
Signed-off-by: Zixuan James Li <359101898@qq.com>
This commit renames get_user_group_direct_members function to
get_user_group_direct_member_ids as it returns a list of ids
and to avoid it being parallel to get_recursive_group_members,
which returns a QuerySet.
The default of a Stream is to be public - having
history_public_to_subscribers default to False is inconsistent with
that. The defaults on the model should generally be consistent.
history_public_to_subscribers wasn't explicitly set when creating
streams via build_stream, thus relying on the model's default of False.
This lead to public streams being created with that value set to False,
which doesn't make sense.
We can solve this by inferring the correct value based on invite_only in
the build_stream funtion itself - rather than needing to add a flag
argument to it.
This commit also includes a migration to fix public stream with the
wrong history_public_to_subscribers value.
Fixes#21784.
As a consequence:
• Bump minimum supported Python version to 3.8.
• Move Vagrant environment to Ubuntu 20.04, which has Python 3.8.
• Move CI frontend tests to Ubuntu 20.04.
• Move production build test to Ubuntu 20.04.
• Move 3.4 upgrade test to Ubuntu 20.04.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Adds a drop-down menu for updating the organization type in the
`organization_profile_admin` page. Implements front end for
this setting to work / update like other organization profile,
notification and permissions settings.
One special note about this dropdown is that the listed options
should change once an organization has successfully set a type
other than 'unspecified' in the database. To accomplish this
the initial settings overlay build checks the realm_org_type
value in the page_params to select the correct options list,
and when the dropdown value is reset, either for update events
or for discarding changes, the page_params value is again used
to check for whether the 'unspecified' value should be present
as an option in the dropdown menu.
Adds basic node test for the `server_events_dispatch`.
Also adds a new help center documentation article for this
organization setting that is linked to in the UI.
Fixes#21692.
`org_type` already exists as a field in the Realm model and is
used when organizations are created / updated in Zulip Cloud,
via the `/analytics/support` view.
Extends the `PATCH /realm` view to be able update `org_type` as
other realm / organization settings are updated, but using the
special log / action that was created for the analytics view.
Adds a field to the `realm op: update` / `realm op: update_dict`
events, which also means an event is now sent when and if the
`org_type` is updated via the analytics view. This is similar
to how updates to an organization's `plan_type` trigger events.
Adds `realm_org_type` as a realm setting fetched from the
`POST /register` endpoint.
This commit adds 'GET /user_groups/{user_group_id}/members'
endpoint to get members of a user group. "direct_member_only"
parameter can be passed as True to the endpoint to get only
direct members of the user group and not the members of
subgroup.
This commit adds 'GET /user_groups/{id}/members/{id}' endpoint to check
whether a user is member of a group.
This commit also adds for_read parameter to access_user_group_by_id,
which if passed as True will provide access to read user group even
if it a system group or if non-admin acting user is not part of the
group.
This commits adds is_user_in_group function
which can be used to check whether a user
is part of a user group or not. It also
supports recursive parameter for including
the members of all the subgroups as well.
This commit also adds 'subgroups' field to the user_group present
in the event sent on creating a user group. We do not allow passing
the subgroups while creating a user group as of this commit, but added
the field in the event object to pass tests.
This commit changes the invite API to accept invitation
expiration time in minutes since we are going to add a
custom option in further commits which would allow a user
to set expiration time in minutes, hours and weeks as well.
When `update_message` events were updated to have a consistent
format for both normal message updates/edits and special
rendering preview updates, the logic used in the tornado event
queue processor to identify the special events for sending
notifications no longer applied.
Updates that logic to use the `rendering_only` flag (if present)
that was added to the `update_message` event format to identify
if the event processor should potentially send notifications to
users.
For upgrade compatibility, if `rendering_only` flag is not present,
uses previous event structure and checks for the absence of the
`user_id` property, which indicated the special rendering preview
updates.
Fixes#16022.
Added a setting to the bottom of Settings > Display settings > Theme section
to display the reacting users on a message when numnber of reactions are
small.
This is a preparatory commit for #20980.
It doesn't make sense to run sync_ldap_user_data if user_profiles list
is empty. Otherwise this misleading exception gets raised:
```
raise Exception(
"LDAP sync would have deactivated all users. This is most likely due "
"to a misconfiguration of LDAP settings. Rolling back...\n"
"Use the --force option if the mass deactivation is intended."
)
```
With some work by tabbott to manage the type of user_profiles and
provide a special error message for the empty server case.
The relevant function is waiting to be merged in #21299 - but we have
already used it on Zulip Cloud, creating RealmAuditLog entries with the
number 107 and thus should reserve it before another PR takes
it for another purpose, creating confusion in the logs.
Based on an audit, this closes out the last core instances in which
acting_user was not being passed explicitly when creating
RealmAuditLog instances.
There are some outstanding issues in the billing system, which we plan
to extract as a separate issue.
Fixes#14808.
The only purpose of this seems to be to not have to reset the cache;
fae59502ab added it without any explanation for why it is necessary.
Remove it, and explicitly flush the cache in the one place where it is
necessary.
This cache was added in da33b72848 to serve as a replacement for the
durable database cache, in development; the previous commit has
switched that to be the non-durable memcached backend.
The special-case for "in-memory" in development is mostly-unnecessary
in contrast to memcached -- `./tools/run-dev.py` flushes memcached on
every startup. This differs in behaviour slightly, in that if the
codepath is changed and `run-dev` restarts Django, the cache is not
cleared. This seems an unlikely occurrence, however, and the code
cleanup from its removal is worth it.
The choice to cache these in the database dates back to c93f1d4eda,
with the comment added in da33b72848 while working around the
durability of the "database" cache in local development.
The values were stored in a durable cache, as they needed to be
ensured to persist between when they were inserted in
`get_link_embed_data` and when they were used in
`render_incoming_message` via `link_embed_data_from_cache`.
However, database accesses are not fast compared to memcached, and we
wish to avoid the overhead of the database connection from the
`embed_links` worker. Specifically, making the connection may not be
thread-safe -- and in low-memory (and Docker) configurations, all
workers run as separate threads in a single process. This can lead to
stalled database connections in `embed_links` workers, and failed
previews.
Since the previous commit made the durability of the cache no longer
necessary, this will have minimal effect; at worst, posting the same
URL twice, on either side of an upgrade, will result in two preview
fetches of it.
The `get_link_embed_data` / `link_embed_data_from_cache` pair as
introduced in c93f1d4eda uses the cache
as a temporary store inside of the `embed_links` worker; this means
that it must be durable storage, or the worker will stall and re-fetch
the same links to preview them.
Switch to plumbing through the fetched URL embed data as an parameter
to the Markdown evaluation which uses them, rather than using the
cache as an intermediary. This frees up the cache to be merely a
non-durable cache.
As a side-effect, this removes get_cache_with_key, and
link_embed_data_from_cache which was its only callsite.
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.
`cachify` is essentially caching the return value of a function using only
the non-keyword-only arguments as the key.
The use case of the function in the backend can be sufficiently covered by
`functools.lru_cache` as an unbound cache. There is no signficant difference
apart from `cachify` overlooking keyword-only arguments, and
`functools.lru_cache` being conveniently typed.
Signed-off-by: Zixuan James Li <359101898@qq.com>
This demonstrates a way to resolve the long-standing issue
of typing higher-order identity functions without using
`cast` and in a type-safe manner for decorators in `cache.py`.
Signed-off-by: Zixuan James Li <359101898@qq.com>
do_deactivate_user can't be run in an atomic block due to concerns
around revoking session in a transaction. See
62ba8e455d for more details.
Without the change in this commit, the process of deactivating a user
via SCIM is broken.
Adds and updates changelog documentation for
`POST /users/me/status` feature level 86 addition
of new emoji parameters.
Makes description text for emoji `reaction_type` consistent
throughout API documentation and also adds better description
of the `unicode_emoji` namespace.
Redirects emoji field links in `user_status` event to go to
the parameters in `/update-status` endpoint, which was not a
documented endpoint when the event documentation was created.
It's natural that someone might try a wrong password 5 times, and then
go through a successful password reset; forcing such users to wait
half an hour before typing in the password they just changed the
account to seems unnecessarily punitive.
Clear the rate-limit upon successful password change.
Removes `token_kind` parameter being passed to
`remove_apns_device_token` and `remove_android_reg_id` code
paths / endpoints. Possibly missed in a refactor of this
function as the tests for adding these tokens do not pass
a `token_kind` parameter.
Removes `zulip_org_id` and `zulip_org_kay` from code testing
`deactivate_remote_server`. These parameters are passed when
a remote server is added, so possibly a copy and paste error
when these tests were written / last refactored.
`update_realm_custom_profile_field` does not take `field_type`
as a parameter, so this removes it from any related tests.
Possibly these test parameters were missed in a refactor of this
endpoint / code.
`service_interface` is not a parameter of `add_bot_backend`, but
`interface_type` is, and that has the same default value as what
was being provided by the test, so updated for the parameter name
change, which was possibly missed in a previous code refactor.
`update_default_stream_group_info` was being passed `op` and
`group_name` in various tests, which are not implemented as
parameters for that endpoint / code path. So this removes those
from the existing tests. This is not a documented API endpoint,
so perhaps these were just overlooked when these tests were
written / last refactored.
If an API request specified a `client` parameter, we were
already prioritizing that value over parsing the UserAgent.
In order to have these parameters logged in the `RequestNotes`
as processed parameters instead of ignored parameters, we add
the `has_request_variables` decorator to `parse_client` and
then process the potential `client` parameter through the REQ
framework.
Co-authored by: Tim Abbott <tabbott@zulip.com>
We remove the StackOverflow link because it is now so dated as to be
irrelevant -- it does not use `self.ident`, and cargo-cults the return
value of PyThreadState_SetAsyncExc.
As noted in the docstring for this function, the timeout is
best-effort only -- if the thread is blocked in a syscall, it will not
service the exception until it returns. It can also choose to catch
and ignore the TimeoutExpired; in either case it will still be running
even after the `timeout()` function returns.
Raising a vare TimeoutExpired it still somewhat accurate, but obscures
that the backend thread may still be running along merrily. Notice
such cases, and log a warning about them.
Having just thrown an exception into the thread, it is often useful to
know _what_ was the slow code that we interrupted. Raising a bare
TimeoutExpired here obscures that information, as any `exc_info` will
end there.
Examine the thread for any exception information, and use that to
re-raise. This exception information is not guaranteed to exist -- if
the thread didn't respond to the exception in time, or caught it, for
instance.
The quote in question originates in python/cpython@b8b6d0c2c6, when
the code was added. However, the code stopped having that comment,
and was no longer able to return anything but 1 or 0, starting in
python/cpython@4643c2fda1 -- Python 2.5.
Remove the block.
Reformats two events (`reaction op: add` and `reaction op:remove`)
to follow the general format of events in the OpenAPI that are
returned by the `/get-events` endpoint.
Removes unneeded reference to `EmojiBase` schema in `user_status`
return value for the `/register-queue` endpoint. Also, clarifies
the text about the `user_status` object and fields being returned.
Adds a non-endpoint specific page to the API documentation about
organization-level roles and permissions for users in order to
highlight important and useful information for clients and API
users.
Also, adds links to new documentation page in related areas
of the API documentation.
We were not setting the `historical` flag correctly for
messages fetched via `json_fetch_raw_message` when used didn't
have any UserMessage.
Extended relevant tests to fetch check message flags too.
Instead of using request.POST to get any potential `sender`
parameters for `send_message_backend`, moves it to the REQ
framework parameters as `req_sender`.
Also, updates `create_mirrored_message_users` to take specific
parameters instead of an HTTP request parameter, which then
accessed parameters via request.POST. And updates existing tests
for those changes.
4815f6e28b tried to de-duplicate bot
email addresses, but instead caused duplicates to crash:
```
Traceback (most recent call last):
File "./manage.py", line 157, in <module>
execute_from_command_line(sys.argv)
File "./manage.py", line 122, in execute_from_command_line
utility.execute()
File "/srv/zulip-venv-cache/56ac6adf406011a100282dd526d03537be84d23e/zulip-py3-venv/lib/python3.8/site-packages/django/core/management/__init__.py", line 413, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/srv/zulip-venv-cache/56ac6adf406011a100282dd526d03537be84d23e/zulip-py3-venv/lib/python3.8/site-packages/django/core/management/base.py", line 354, in run_from_argv
self.execute(*args, **cmd_options)
File "/srv/zulip-venv-cache/56ac6adf406011a100282dd526d03537be84d23e/zulip-py3-venv/lib/python3.8/site-packages/django/core/management/base.py", line 398, in execute
output = self.handle(*args, **options)
File "/home/zulip/deployments/2022-03-16-22-25-42/zerver/management/commands/convert_slack_data.py", line 59, in handle
do_convert_data(path, output_dir, token, threads=num_threads)
File "/home/zulip/deployments/2022-03-16-22-25-42/zerver/data_import/slack.py", line 1320, in do_convert_data
) = slack_workspace_to_realm(
File "/home/zulip/deployments/2022-03-16-22-25-42/zerver/data_import/slack.py", line 141, in slack_workspace_to_realm
) = users_to_zerver_userprofile(slack_data_dir, user_list, realm_id, int(NOW), domain_name)
File "/home/zulip/deployments/2022-03-16-22-25-42/zerver/data_import/slack.py", line 248, in users_to_zerver_userprofile
email = get_user_email(user, domain_name)
File "/home/zulip/deployments/2022-03-16-22-25-42/zerver/data_import/slack.py", line 406, in get_user_email
return SlackBotEmail.get_email(user["profile"], domain_name)
File "/home/zulip/deployments/2022-03-16-22-25-42/zerver/data_import/slack.py", line 85, in get_email
email_prefix += cls.duplicate_email_count[email]
TypeError: can only concatenate str (not "int") to str
```
Fix the stringification, make it case-insensitive, append with a dash
for readability, and add tests for all of the above.
`disable-message-edit-history`: Remove text about EDITED label
and link to `view-a-messages-edit-history` instead.
`edit-or-delete-a-message`: Reformat 'EDITED' and '(deleted)'
to be bold instead of using backticks. Make link to view
edit history clearer.
Note that the text used in the `OpenGraphTest` in
`test_middleware.py` had to be updated for the changes to
`disable-message-edit-history`.
This avoids an error when a user has already muted the new topic name.
We do this by ignoring duplicates, rather than catching the
IntegrityError, because this edit happens in a transaction, and that
would abort the transaction.
This is necessary for the migration 0386_fix_attachment_caches to run,
and likely makes more convenient any future parallel code interacting
with both Attachment and ArchivedAttachment.
Our original implementation of moving muted topic records when a topic
is moved took a shortcut of treating all change_later usage as
something with intent to move the whole topic.
This works OK when moving the whole topic via this interface, but not
when moving a last off-topic message in the topic.
Address this by changing the rule to match the existing
moved_all_visible_messages variable.
Adds tab for web-public streams in documentation for setting
who can create new streams, as well as some text about why
this is limited to certain roles.
Removes list of actions that can be restricted to full members
due to maintainability concerns for that type of list in the
documentation and replaces it with a short descriptive text
explaining that many settings in Zulip support this restriction.
This migration needs to be run after the previous commit is deployed
to a given Zulip installation, to fix any stale values of
is_realm_public and is_web_public.
Previously, Attachment.is_realm_public and its cousin,
Attachment.is_web_public, were properties that began as False and
transitioned to True only when a message containing a link to the
attachment was sent to the appropriate class of stream, or such a link
was added as part of editing a message.
This pattern meant that neither field was updated in situations where
the access permissions for a message changed:
* Moving the message to a different stream.
* Changing the permissions for a stream containing links to the message.
This correctness issue has limited security impact, because uploaded
files are secured both by a random URL and by these access checks.
To fix this, we reformulate these fields as a cache, with code paths
that change the permissions affecting an attachment responsible for
setting these values to the `None` (uncached) state. We prefer setting
this `None` state over computing the correct permissions, because the
correct post-edit permissions are a function of all messages
containing the attachment, and we don't want to be responsible for
fetching all of those messages in the edit code paths.
When the credentials are provided by dint of being run on an EC2
instance with an assigned Role, we must be able to fetch the instance
metadata from IMDS -- which is precisely the type of internal-IP
request that Smokescreen denies.
While botocore supports a `proxies` argument to the `Config` object,
this is not actually respected when making the IMDS queries; only the
environment variables are read from. See
https://github.com/boto/botocore/issues/2644
As such, implement S3_SKIP_PROXY by monkey-patching the
`botocore.utils.should_bypass_proxies` function, to allow requests to
IMDS to be made without Smokescreen impeding them.
Fixes#20715.
The documentation at https://api.beeminder.com/#goal says this is
“number”; empirically, we do in fact get decimal points.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
`prepare_linkifier_pattern`, as of db934be064, adds a match to the
end of the regex, of either the end of string, or a non-word character
-- this is in place of a negative look-ahead, which is no longer
possible in re2. This causes the regex to consume trailing
whitespace, and thus not be able to match twice in succession with
`pattern.finditer` -- "#1234#5678" fails to match because the space
is consumed by the first match of the regex.
Rather than use `pattern.finditer`, write own own version, which
rewinds over the non-word character consumed after the match, if any.
This allows the same "after" non-word character to also satisfy the
"before" of the next match.
Fixes#21502.
Extends the linking to Zulip documentation to cover:
- Getting URLs to messages via the message timestamp.
- Getting links to topics via the three-dots menu.
- Getting links to streams via right-click context menu.
Creates a new tabbed section for using the browser
address bar to copy URLs.
* Don't print the empty list for the vast majority of realms where
this is a noop.
* Make output a little more clear that this isn't revoking all
Confirmations, just those associated with deactivated users.
Add support for moving MutedTopic entries to another stream where
the user has access to shared history in both streams and
`propagate_mode != "change_one"`.
Also, we delete them the current user does not have access to the
target stream.
This is intended for rare situations where one is creating multiple
realms via a script.
After all the preparatory refactoring in this last several commits, we
can now provide a working implementation of a create_realm management
command.
We set nocoverage for the new function. Ideally it'd eventually get an
automated test, but we don't want to block this helpful refactoring on
doing so.
We remove a bit of error handling for cases where someone provided
only one of the email and full name parameters, with the benefit of
this being a lot cleaner.
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 improves robustness of any code paths calling do_create_realm,
which previously needed to call this correctly to achieve the same
results as creating a user via the UI.
This also fixes a bug where this code was not called if a realm were
created using the LDAP code path.
This parameter was introduced in
ea11ce4ae6, and no longer serves a
purpose. Zulip will already correctly record that the user has not
agreed to ToS, and either prompt them on first login or not depending
whether the server is configured to require ToS.
This is an important design detail, so we document this aspect of
creating users via both the management command and API code paths with
an explicit parameter value and comment.
Ordinary organization administrators shouldn't be allowed to change
ownership of a bot with the can_create_users permission.
This is a special permission that is granted manually by server
administrators to an organization (to a UserProfile of the org owners'
choice) after approval by a server administator. The code comments
provide more detail about why this is sensitive.
The BigBlueButton integration had a problem with generating
the random password with only 12 characters. This would
cause the attendeePW to be the same as the moderatorPW,
which might be fine but seems like something that could be an
error in a future version of BigBlueButton.
The name for a BigBlueButton meeting is now generated from the stream
name and topic name.
The createTime option is used to have the user redirected to a link
that is only valid for this meeting.
Even if the same link in Zulip is used again, a new createTime
parameter will be created, as the Meeting on the BigBlueButton server
has to be recreated.
Fixes#16498.
Fixes#20509.
Fixes#20804.
Previously, when a topic was edited (including being resolved), it
would become unmuted for any users who had muted it, which was
annoying.
While it's not possible to determine the user's intent completely,
this is clearly incorrect behavior in the `change_all` case, such as
resolving a topic.
The comments discuss some scenarios where we might want to enhance
this further, but this is the best we can do without large increases
in complexity.
Fixes#15210.
Co-authored-by: akshatdalton <akshat.dak@students.iiit.ac.in>
he possibility for it being null was likely an oversight -- it should
have been removed after the early migrations to backfill the field
when it was added.
We've confirmed there are no existing violations of this invariant in
Zulip Cloud.
This is a natural follow-up to
93e8740218 - invitations sent by users
deactivated before the commit still need to be revoked, via a
migration.
The logic for finding the Confirmations to deactivated is based on
get_valid_invite_confirmations_generated_by_user in actions.py.
Co-authored-by: Steve Howell <showell@zulip.com>
Co-authored-by: Tim Abbott <tabbott@zulip.com>
This commit adds the backend functionality to
mark messages as unread through update_message_flags
with `unread` flag and `remove` operation.
We also manage incoming events in the webapp.
Tweaked by tabbott to simplify the implementation and add an API
feature level update to the documentation.
This commit was originally drafted by showell, and showell
also finalized the changes. Many thanks to Suyash here for
the main work here, which was to get all the tests and
documentation work moving forward.
This commit creates a new TypedDict RealmPlaygroundDict for realm
playground objects. Now the list of playgrounds in the events sent
to clients and the "added_playground" field of RealmAuditLog entry
use RealmPlaygroundDict instead of Dict.
This commit modifies the notify_realm_playgrounds function to accept
realm_playgrounds as argument from the caller instead of computing it
in the function to avoid duplicate queries since the realm playgrounds
list will be required in its caller functions as well in further commits.
Clearing the sessions inside the transaction makes Zulip vulnerable to
a narrow window where the deleted session has not yet been committed,
but has been removed from the memcached cache. During this window, a
request with the session-id which has just been deleted can
successfully re-fill the memcached cache, as the in-database delete is
not yet committed, and thus not yet visible. After the delete
transaction commits, the cache will be left with a cached session,
which allows further site access until it expires (after
SESSION_COOKIE_AGE seconds), is ejected from the cache due to memory
pressure, or the server is upgraded.
Move the session deletion outside of the transaction.
Because the testsuite runs inside of a transaction, it is impossible
to test this is CI; the testsuite uses the non-caching
`django.contrib.sessions.backends.db` backend, regardless. The test
added in this commit thus does not fail before this commit; it is
merely a base expression that the session should be deleted somehow,
and does not exercise the assert added in the previous commit.
Commit ab8aae6d0c (#12161) incorrectly
assumed that ‘new’ is a string. In the case of change == "links",
it’s a dict.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
These error messages weren't marked for translation.
DEACTIVATED_ACCOUNT_ERROR and PASSWORD_TOO_WEAK_ERROR are used in
several places and imported, so we can't move them to be in-line errors
and we keep them at top-level, marked with gettext_lazy.
Using mark_safe on errors with content in them taken from user-input is
a clearly bad idea. With that said, this code
was not exploitable in the current state, given that username is a value
you have to POST to /login/, and the endpoint is CSRF-protected.
We also remove use of mark_safe from the errors without user input them,
but that are just plaintext and thus don't need it.
Adds documentation for admins to manage users via the user profile
modal for these actions:
- Deactivating a user
- Changing a user's role
- Changing a user's name
Creates two new tab sections because we still want to document
the ability to do these actions through the users section in
the organizational settings modal.
Also cleans up some text in the help center article for changing
a user's role.
Fixes#21318.
Fixes#21415.
Adds content on user group permissions / management to the general
help center article for user groups (`/help/user-groups`) and
removes the then redundant `/help/restrict-user-group-management`
article.
Redirects links in help center and api documentation from deleted
article to the new configure user group settings section of
`/help/user-groups`.
Fixes#21383.
This commit adds a cron job which runs every hour to add the users to
full members system group if user is promoted to a full member.
This should ensure that full member status is available no more than
an hour after configuration suggests it should be.
There can be cases when system groups data is not present while
importing, like when importing from other products, so this
commit adds code to create system user groups and add users to
it according to their role.
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.
We pass list of user ids instead of user profile objects to
remove_members_from_user_group. We still need to call user_id_to_users
in the views function instead of directly passing the ids to
remove_members_from_user_group to make sure we check whether all
ids are valid or not.
We pass list of user ids instead of user profile objects to
bulk_add_members_to_user_group. We still need to call user_id_to_users
in the views function instead of directly passing the ids to
bulk_add_members_to_user_group to make sure we check whether all
ids are valid or not.
Previous behavior was logging only the uuid if it was provided by the
remote server, but that's insufficient, because the user may actually
have no devices registered with uuis and we (at the bouncer) end up
sending notifications to id-based registrations. Not having that id
logged makes it impossible to figure out what's going on.
Fixes#18017.
In previous commits, the change to the bouncer API was introduced to
support this and then a series of migrations added .uuid to
UserProfiles.
Now the code for self-hosted servers that makes requests
to the bouncer is changed to make use of it.
This is in a separate commit to make deployment easier. It ensures that
this is only marked non-null after the backfill migration (backfilling
.uuid for all old UserProfiles) runs - which was added in the previous
commit.
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.
Previously, this URL just returned the `raw_content` field. It seems
cleanest to just make it a single-message variant of GET /messages,
deprecating the only format.
This will make it convenient to add a handful of organizations to the
beta of this feature during its first few weeks to try to catch bugs,
before we open it to everyone in Zulip Cloud.
The correct return type of get_realm_domains should
be List[Dict[str, Union[bool, str]]] instead of
List[Dict[str, str]] because allowed_subdomains is
a bool field not str.
A user can subscribe to a stream and sometimes (depending
on stream permissions) see messages from the stream
that were sent before they subscribed, and that user
won't have a UserMessage row for that message.
In order to do things like star a message, we need
to create UserMessage records on the fly.
In the past we wisely constrained this logic to the
specific use cases. But I think we can generalize
the logic now. For example, we are now building a
feature to mark messages as unread, and it motivates
the same need to auto-create UserMessage rows.
So now we handle this in a more generalized fashion.
Refactors and cleans up the shared `MessagesBase` schema in
the OpenAPI so that it accurately reflects the general base
for message objects for endpoints that use it as a reference.
A follow-up to adding `edit_history` as a property of message
objects. And a prepartory commit for `GET /messages/{msg_id}`
to return not only the raw content of the message but also
the message object.
When removing notifications, we skip the access control on if the user
still can read them -- they should not have a notification of them,
both because they currently cannot read the message, as well as
because they have already done so.
Translators found it confusing, since it's not at all obvious that the
word "quote" should not be translated.
I'm not altogether happy with the code formatting for this.
While we're changing this, also standardize on the "```` quote" style
of quote blocks to ensure code/quote blocks in stream descriptions are
unlikely to conflict with this syntax.
In steady-state, requests to FCM take about a second; however, in
cases where the remote FCM server is unstable, the request has been
observed to block for more than a minute.
As noted in the previous commit, pushes must complete within 30s;
fail fast, and let the retries and exponential backoff handle errors.
The worst-case total time taken with timeouts and errors for an FCM
notification is 19.5s. Unfortunately, `aioapns` does not appear to
have any timeouts, and thus this commit cannot guarantee a total of
fewer than 30s.
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.
This model is by designed intended to exist on a 1:1 relationship with
Realms, and we attempt to ensure that with application code, but we
should have a unique constraint too, since a database with duplicate
such entries would be corrupted.
We do this via the standard Django OneToOneField.
It was there to work around https://bugs.python.org/issue17519. This
workaround with del seems like a partial improvement.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We want TypedDicts that have actual teeth.
In order to make type checks meaningful, we want
to avoid Any, object, or crazy Union types when
we aggregate each type of message, so we replaced
a generic function with three concrete functions.
Provide stream privacy and description in stream notification events
when stream is created.
In function "send_messages_for_new_subscribers" for when stream is
created, put policy name and description of the stream.
Fixes#21004
Removes `LEGACY_PREV_TOPIC` which is no longer needed due to the
message edit history migration.
Also remove additions to the linter exclude list that were added
earlier in this commit series.
Since we've changed the database to contain these new fields, we just
need to stop dropping them in the API code.
This also changes the public API to match the database format again
by removing `prev_subject` from edit history API.
Adds an API changelog feature update for the renamed `prev_subject`
field (to `prev_topic`) and new fields (`topic` and `stream`)
in the message `edit_history`.
Also, documents said `edit_history` in the `MessagesBase` schema
in the api documentation, which is used by the `/get-messages`,
`/get-events` and `/zulip-outgoing-webhooks` endpoints.
Fixes#21076.
Co-authored-by: Lauryn Menard <lauryn.menard@gmail.com>
Now that we have code to support reading prev_topic, this is no longer
necessary.
We'll want to deploy this change to production before running the
migration to remove prev_subject from edit history entries, so that
prev_subject can be fully purged from the database.
We modify the message_edit_history marshalling code so that this
commit does not change the API, since we haven't backfilled the data
yet.
FormattedEditHistoryEvent, introduced in the previous commit, doesn't
directly inherit fields from EditHistoryEvent, so no changes are
required there.
We fix the mutation of caller and other bad patterns, as well as
adding explicit typing to make the code readable.
We also update the OpenAPI documentation for previously
undocumented `prev_strem` field in the `/get-message-history`
endpoint for API validation testing.
Co-authored-by: Lauryn Menard <lauryn.menard@gmail.com>
These types will help make iteration on this code easier.
Note that `user_id` can be null due to the fact that
edit history entries before March 2017 did not log
the user that made the edit, which was years after
supporting topic edits (discovered in test deployment
of migration on chat.zulip.org).
Co-authored-by: Lauryn Menard <lauryn.menard@gmail.com>
Moves the encodeHashComponent and decodeHashComponent functions out of
hash_util and into internal_url which belongs to shared. This is to
accommodate sharing of this code with mobile or any other codebases that
do not wish to duplicate logic.
TestMaybeSendToRegistration needs tweaking here, because it wasn't
setting the subdomain for the dummy request, so
maybe_send_to_registration was actually running with realm=None, which
is not right for these tests.
Also, test_sso_only_when_preregistration_user_exists was creating
PreregistrationUser without setting the realm, which was also incorrect.
create_preregistration_user is a footgun, because it takes the realm
from the request. The calling code is supposed to validate that
registration for the realm is allowed
first, but can sometimes do that on "realm" taken from something else
than the request - and later on calls create_preregistration_user, thus
leading to prereg user creation on unvalidated request.realm.
It's safer, and makes more sense, for this function to take the intended
realm as argument, instead of taking the entire request. It follows that
the same should be done for prepare_activation_url.
In these tests, the code ends up with a logged in session when it's
undesired - later on these tests make requests to a different subdomain
- where a logged in session is not supposed to exist. This leads to an
unintended, strange situation where request.user is a user from the old
subdomain but the request itself is to a *different* subdomain. This
throws off get_realm_from_request, which will return the realm from
request.user.realm - which is not what these tests want and can lead to
these tests failing when some of the production code being tested
switches to using get_realm_from_request instead of
get_realm(get_subdomain).
The codepaths for joining an organization via a multi-use invitation
(accounts_home_from_multiuse_invite and maybe_send_to_registration)
weren't validating whether
the organization the invite was generated for matches the organization
the user attempts to join - potentially allowing an attacker with access
to organization A to generate a multi-use invite and use it to join
organization B within the same deployment, that they shouldn't have
access to.
The database value for expiry_date is None for the invite
that will never expire and the clients send -1 as value
in the API similar to the message retention setting.
Also, when passing invite_expire_in_days as an argument
in various functions, invite_expire_in_days is passed as
-1 for "Never expires" option since invite_expire_in_days
is an optional argument in some functions and thus we cannot
pass "None" value.
A migration should not import zerver.lib.streams at all, but this
solves the immediate problem with check-database-compatibility.py.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Removes `client` parameter from backend tests using the
`POST /messages` endpoint when the test can use the default
`User-Agent` as the client, which is set to `ZulipMobile` for API
requests and a browser user agent string for web app requests.
Various tests use the `PATCH /stream/{stream_id} endpoint in
`test_subs.py`. Because the stream id is in the URL path, it
does not also need to be passed as a query parameter.
Removes instances of `stream_name` being passed as a query
parameter to tests.
Removes `topic_name` parameter in `test_message_flags.py`
where is being passed to a test for marking a stream as
read because it is an ignored parameter for that endpoint.
This was only used for upgrading from Zulip < 1.9.0, which is no
longer possible because Zulip < 2.1.0 had no common supported
platforms with current main.
If we ever want this optimization for a future migration, it would be
better implemented using Django merge migrations.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
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
Adds `realm_web_public_access_enabled` as a realm-specific server
setting potentially returned by the `/get-server-settings` endpoint
so that clients that support browsing of web-public stream content
without an account can generate a login page that supports that
type of access.
Various backend tests use the `PATCH /messages/{msg_id}` endpoint.
For that endpoint, the message ID is encoded in the URL path and
ignored if provided as a parameter in the the query.
Verified that the tests were providing the same message ID to both
the path and then removed the ignored parameter in the query.
This commit refactors get_user_by_email function
to use access_user_by_email which is similar to
already existing access_user_by_id and thus using
get_user_data function added recently.
We also remove the unnecessary check for email as
email will always be passed to this endpoint.
Preparatory commit for #10970.
This commit adds get_user_data which is called by
get_members_backend to compute the client_gravatar
value and then return the data of a single user or
all accessible users.
This function will also be used by get_user_by_email
in further commtis.
When pulling batches out of the ScheduledEmail list in a single
transaction, an unexpected failure to send an email will result in the
whole batch getting retried. This will result in infinite email
sending loops.
Pull a single row off at a time and send it. We continue without
retries to the next email on EmailNotDeliveredException, but will
retry infinitely on other exceptions.
Fixes: #20943.
Putting all of the logic in a `finally` block is equivalent to a bare
`except` block, which silently consumes all exceptions.
Move only the most-necessary parts into the except; this lets
`BadImageError` exceptions from `zerver/lib/upload.py` to escape,
allowing better the generic "Image file upload failed" to be replaced
with a more specific message.
It also allows unexpected exceptions, as the previous commit resolved,
to escape and 500. This lets them be detected and resolved, rather
than give users a silently bad experience.
5dab6e9d31 began honoring the list of disposals for every frame.
Unfortunately, passing a list of disposals for a non-animated image
raises an exception:
```
File "zerver/lib/upload.py", line 212, in resize_emoji
image_data = resize_gif(im, size)
File "zerver/lib/upload.py", line 165, in resize_gif
frames[0].save(
File "[...]/PIL/Image.py", line 2212, in save
save_handler(self, fp, filename)
File "[...]/PIL/GifImagePlugin.py", line 605, in _save
_write_single_frame(im, fp, palette)
File "[...]/PIL/GifImagePlugin.py", line 506, in _write_single_frame
_write_local_header(fp, im, (0, 0), flags)
File "[...]/PIL/GifImagePlugin.py", line 647, in _write_local_header
disposal = int(im.encoderinfo.get("disposal", 0))
TypeError: int() argument must be a string, a bytes-like object or a
number, not 'list'
```
`check_add_realm_emoji` calls this as:
```
try:
is_animated = upload_emoji_image(image_file, emoji_file_name, a
uthor)
emoji_uploaded_successfully = True
finally:
if not emoji_uploaded_successfully:
realm_emoji.delete()
return None
# ...
```
This is equivalent to dropping _all_ exceptions silently. As such,
Zulip has silently rejected all non-animated images larger than 64x64
since 5dab6e9d31.
Adjust to only pass a single disposal if there are no additional
frames. Add a test for non-animated images, which requires also
fixing the incidental bug that all GIF images were being recorded as
animated, regardless of if they had more than 1 frame or not.
The deferred_work events can't be processed in the test suite
environment, since it tries to make requests to the host <realm.uri>
which is "zulip.testserver" and obviously not going to work. Since the
test suite base data set has no animated emoji, there's nothing to do,
and we can skip this step.
This code only runs when applying the migration to an already
provisioned test db - because during an initial db set up there are no
realms yet, so no events get pushed by the migration.
The previous random strategy for picking 5 emails would result in
collisions in 1 out of 10000.35 tests, leading to
psycopg2.errors.UniqueViolation.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
For aliases that will no longer be listed, see the third column of
grep '^L ' zulip-py3-venv/lib/python3.*/site-packages/pytz/zoneinfo/tzdata.zi
Time zones previously set to an alias will be canonicalized on demand.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
A recent Postgres upstream release appears to have broken PGroonga.
While we wait for https://github.com/pgroonga/pgroonga/issues/203 to
be resolved, disable PGroonga in our automated tests so that Zulip
CI passes.
Sometimes we may get data to import, due to export bugs, malformed data
etc., which doesn't have the invariant of RealmEmoji.author always being
set. The import code should fix that, by choosing a reasonable default
and setting it.
There doesn’t seem to be a reason to override this, and the upstream
method it was based on has diverged since this was written.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Although our NonClosingPool prevents the SQLAlchemy connection from
closing the underlying Django connection, we still want to properly
dispose of the associated SQLAlchemy structures.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Fixes these warnings with SQLALCHEMY_WARN_20=1:
RemovedIn20Warning: Using non-integer/slice indices on Row is
deprecated and will be removed in version 2.0; please use
row._mapping[<key>], or the mappings() accessor on the Result
object. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
RemovedIn20Warning: Using the 'in' operator to test for string or
column keys, or integer indexes, in a :class:`.Row` object is
deprecated and will be removed in a future release. Use the
`Row._fields` or `Row._mapping` attribute, i.e. 'key in
row._fields' (Background on SQLAlchemy 2.0 at:
https://sqlalche.me/e/b8d9)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Fixes this warning with SQLALCHEMY_WARN_20=1:
RemovedIn20Warning: The legacy calling style of select() is deprecated
and will be removed in SQLAlchemy 2.0. Please use the new calling
style described at select(). (Background on SQLAlchemy 2.0 at:
https://sqlalche.me/e/b8d9)
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Fixes “SADeprecationWarning: The Select.column() method is deprecated
and will be removed in a future release. Please use
Select.add_columns() (deprecated since: 1.4)”.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Fixes “SADeprecationWarning: Implicit coercion of SELECT and textual
SELECT constructs into FROM clauses is deprecated; please call
.subquery() on any Core select or ORM Query object in order to produce
a subquery object.”
Signed-off-by: Anders Kaseorg <anders@zulip.com>
match_querystring is irrelevant in these cases. Fixes this warning:
/srv/zulip-py3-venv/lib/python3.7/site-packages/responses/__init__.py:340:
DeprecationWarning: Argument 'match_querystring' is deprecated. Use
'responses.matchers.query_param_matcher' or
'responses.matchers.query_string_matcher'
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The tool needs to run this function, since it uses django's send_email
directly instead of going through our zerver.lib.send_email.send_email
codepath.
The message ID is encoded in the URL, not the PATCH parameters, so
this argument was ignored. I verified that it appears to have always
matched the value present in the URL.
The new logic better matches reasonable user expectations, that if you
move all the messages, that's a whole-topic move, regardless of which
propagation mode you selected.
When moving only part of a topic, it's useful to display that
information to users in these notifications so that it's clear what's
happening.
The most important consequence is actually just increasing confidence
that when you see that the whole topic was moved, that's accurate.
Substantially modified by tabbott.
Fixes#20575.
To avoid an uncaught IntegrityError causing a 500 HTTP response in a
race between two processes trying to mute a topic, we catch the
integrity error and raise the error exception with status 400 we'd
have gotten if the second request had been a bit later.
Fixes#21011.
The S3 backend implementation of upload_emoji_image was accessing
emoji_file.name - which is redundant because emoji_file_name already
gets passed in and can be used, and an object of type IO[bytes] may not
have the .name attribute. Spotted by @Fingel.
Fixes#20132.
EMAIL_HOST_USER without EMAIL_HOST_PASSWORD is not going to be a valid
configuration, and may result from making mistake in correctly setting
it in the secrets file and end up being a non-obvious cause of failure
to send email. Logging an error will be useful for detecting it. Further
conditions can be added to the function in the future.
Calling `get_apns_context` opens (and caches) an open connection to
the APNs servers. Since `apns_enabled` is called from Django
codepaths, this means that the Django processes hold unnecessary
connections open to the APNs servers.
Switch `apns_enabled` to checking what `get_apns_context` checks when
we're just returning True/False.
aioapns 2.1 removed the loop parameter from the aioapns.APNs
constructor, because Python 3.10 removed the loop parameter from the
asyncio.Lock constructor.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
The new release adds the commit:
20ac22b96d
Which allows us to get rid of the entire ugly override that was needed
to do this commit's job in our code. What we do here in this commit:
* Use django-scim2 0.17.1
* Revert the relevant parts of f5a65846a8
* Adjust the expected error message in test_exception_details_not_revealed_to_client
since the message thrown by django-scim2 in this release is slightly
different.
We do not have to add anything to set EXPOSE_SCIM_EXCEPTIONS, since
django-scim2 uses False as the default, which is what we want - and we
have the aforementioned test verifying that indeed information doesn't
get revealed to the SCIM client.
I incorrectly removed this when simplifying
dbddbee5a115b9352862cb13d4c66820865c30b6; while that commit did not
require the hunk re-added here, the later commit
3be622ffa7 added a call that did require it.
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 a preparatory step to refactoring json_success to accept
request as a parameter, change `do_report_error`, which is
called from the events queue for "error_reports", to return
None instead of json_success.
Adds an assertion error to `ErrorReporter` queue processor
and removes `JsonableError` from `do_report_error`.
It is likely that `do_error_report` was moved from a view in a
previous refactor, but was not updated to no longer return an
HttpReponse.
As a preparatory step to refactoring json_success to accept
request as a parameter, update helper function `compose_views`
in `views.streams.py` to return the response data and call
json_success from view functions that utilize `compose_views`.
Also, updates related test in `zerver.tests.test_subs.py`.
As a preparatory step to refactoring json_success to accept
request as a parameter, change interface of helper functions:
`handle_deferred_message` in `views.message_send.py` and
`mute_topic` and `unmute_topic` in `views.muting.py`, so
that they return None or data for json_success.
Instead call json_sucess in the caller function, which already
has the HttpRequest as a parameter.
Adds a check for `additionalProperties: true` when there are no
properties listed in the schema.
This currently only happens in one place, but will be helpful for
deduplicating text between the `register-queue` and `get-events`
endpoints.
Wordle has recently become a thing and it uses green, yellow and white (or
black in dark mode) large square unicode characters to let people share their
gameplay. Zulip converts the white and black large square unicode characters to
emojis, but not the green and yellow ones. This causes the Wordle grid to be
misaligned when shared on Zulip.
This commit adds green and yellow large square emojis to our emoji list to fix
the problem.
Previously, we only checked mandatory_topics setting before
sending message in frontend and there was no restriction in
backend. This commit adds the check in backend also making
sure messages without topic cannot be sent through API as
well if mandatory_topics setting is set to True.
Previously, users found it annoying that the automated "Resolve topic"
notifications triggered an unread for everyone in the stream; this
discouraged some users from using the feature on older threads for
fear of being annoying. We change this to a better default, of only
users who participated in the topic (via either messages or reactions)
being eligible for the new message being unread.
We will likely want to create global and stream-level notifications
settings to control this behavior as a follow-up -- some users, like
me, might prefer the simpler "Always unread" behavior in some streams.
Note that the automated notifications that a topic was resolved will
still result in the topic being moved to the top of the left sidebar.
This would be somewhat difficult to change, since the left sidebar
algorithm just looks at the highest message ID in the topic.
Fixes#19709.
Tests added by Aman Agrawal (amanagr@zulip.com).
In English, compound adjectives should essentially always be
hyphenated. This makes them easier to parse, especially for users who
might not recognize that the words “web public” go together as a
phrase.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
There are three user settings that are integer enums:
`color_scheme`, `demote_inactive_streams` and
`desktop_icon_count_displays`.
Unlike the other user settings, these were using the `content`
keyword instead of the `schema` keyword in their definitions,
which caused them not to be rendered correctly in the api
documentation.
Changes the keyword to `schema` and fixes the indentation
for these three user settings in the two endpoints using
them.
Removes various instances of quotation marks that are not needed,
specifically looking at instances of arrays, e.g. `- "`, in the
OpenAPI documentation.
The target realm was not being passed to create_attachment in
upload_message_file implementations. This was a bug in the edge-case of
cross-realm messages - in particular, causing a bug in the email
gateway:
When an email with an attachment is sent, the message is mirrored to
Zulip with Email Gateway Bot as the message sender and uploader of the
attachment. Due to the realm not being passed to create_attachment, the
Attachment would get created with .realm being the system bot realm,
making the attachment inaccessible under some conditions due to failing
the following condition check (that's expected to pass, provided that
the .realm is set correctly):
```
if (
attachment.is_realm_public
and attachment.realm == user_profile.realm
and user_profile.can_access_public_streams()
):
# Any user in the realm can access realm-public files
return True
```
Fixes the rendering of enums to show strings with quotation marks,
while integers will continue to be rendered without quotation marks.
This allows for an empty string to be passed as an enum value and be
rendered as such in the documentation. Null will be rendered without
quotation marks, like integer values.
Makes `edit_timestamp` and `user_id` required fields for all
`update_message` events.
Adds `rendering_only` as another required field to signal if
events are only updating the rendered content of the message,
which is currently the case for adding inline url previews.
Updates `test_event.py` so that `do_update_message` and
`do_update_embedded_data` refer to the same testing schema
for `update_message` events, and therefore reflect the same
required fields for the `update_message` event.
The OpenAPI definition for `update_message` events is also
updated to reflect the required field and descriptions of
various properties are updated for the addition of the
`rendering_only` property.
Moves details about the rate limit error object and handling to
the OpenAPI documentation description for that common error.
Previously, this information was on the general rest error
handling documentation page without clear connection to the
specific rate limit error.
Fixes a typo in the changelog (feature 36) for that same error
and also fixes a misplaced colon in the description of the error
for missing request parameters.
Adds detailed definition of objects in the `subscription_data` parameter
array for the `/update-subscription-settings` endpoint.
Fixes#20825. Follow-up to #20409.
Formats and moves whether a field of an object in a request
parameter is required or optional to be in the same location
and have the same formatting as the general api parameter
documentation.
Also formats any examples within the object detailed
description to be the same as the general api parameter
documentation.
Follow up to #20409.
Adds a line break before the descriptive text for return
values and events in the api documentation in order to
help with readability of descriptions with multiple
paragraphs of descriptive text.
Adjustments made to the CSS of list items in unordered
lists to visually group the first paragraph of text
to any following paragraphs or unordered lists.
As a consequence:
• Bump minimum supported Python version to 3.7.
• Move Vagrant environment to Debian 10, which has Python 3.7.
• Move CI frontend tests to Debian 10.
• Move production build test to Debian 10.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
regarding -
POST https://yourZulipDomain.zulipchat.com/api/v1/users/me/subscriptions
The definition of the "subscription" parameter didn't include full
information about the parameter. It only said that an array of objects
is passed as a parameter, and relied on description of the parameter
to explain what the object contained. I edited the definition to contain
the full information about the object.
Fixes#20824.
The change to curl_param_value_generators.py warrants a brief
explanation. Stream permission changes now generate a notification
message. Our curl example test for removing a reaction comes after
the two tests for updating the stream permission changes, thus the
hardcoded message ID in that test needs to be incremented by 2 to
account for the two notification messages that now come before it.
This is a part of #20289.
do_make_stream_web_public and do_change_stream_invite_only seem
to contain very similar logic that could just live inside the
do_change_stream_permission function that handles all permission
changes in one place.
queue_client.queues does not list all the queues that exist on the
server (you can’t do that over AMQP); the condition "test_suite" in
queue_client.queues was always false. So the test_suite queue could
accumulate extra messages that broke test_queue_error_json.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This also fixes a warning from
RealmExportTest.test_endpoint_local_uploads: “ResourceWarning:
unclosed file <_io.BufferedReader
name='/srv/zulip/var/…/test-export.tar.gz'>”.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Adds a check for object in parameter type that will render the details
of the object in the parameter description if they are in the object
definition in the OpenAPI documentation.
Fixes#19424.
revoke_invites_generated_by_user should send invites_changed event if it
actually revokes some invitations. This is called in the user
deactivatoin codepath.
Event of type "realm_user", op "remove", emitted by do_deactivate_user
should remove the user id from subscriptions in the state. We weren't
catching this bug, because test_do_deactivate_bot uses a newly created
bot, so no stream subscriptions are affected. The bug shows up if
deactivating e.g. cordelia - thus we want to have two tests instead,
one for testing bot deactivation and one for user deactivation.
We now use recipient_id % 24 for new stream colors
when users have already used all 24 of our canned
colors.
This fix doesn't address the scenario that somebody
dislikes one of our current canned colors, so if a
user continually changes canned color N to some other
color for new streams, their new streams will continue
to include color N (and the user will still need to
change them).
This fix doesn't address the fact that it can be expensive
during bulk-add situations to query for all the colors
that users have already used up.
See https://chat.zulip.org/#narrow/stream/3-backend/topic/assigning.20stream.20colors
for more discussion.
The limit here is purely to prevent breakage in case of a pathological
number of images in a single message; 5 images is entirely possible in
a reasonable message, and causes user confusion when they are not
expended.
Increase the limit to 10 per message.
Django 3.2 expects a list, and Django 4.1 will require one. Fixes
“RemovedInDjango41Warning: Using a boolean value for
requires_system_checks is deprecated. Use '__all__' instead of True,
and [] (an empty list) instead of False.”
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This was deprecated in Django 3.1 for being jQuery-specific, and
removed in Django 4.0. Replicate the jQuery-specific check.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
oneOf with two identical branches (modulo example) is a bug because
oneOf means exclusive or. It’s also a totally inappropriate kludge
for encoding multiple examples. The OpenAPI specification provides a
perfectly good standard way to do that:
https://spec.openapis.org/oas/v3.0.3#example-object
However, we don’t handle that in our OpenAPI documentation generator
yet, so for now just merge the examples.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
This was a oneOf with two identical branches modulo example, which is
always a bug because oneOf means exclusive or. But the example for
the first branch did not fit the schema for AddSubscriptionsResponse,
which is a subset of JsonSuccessBase.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Although allOf is often used to indicate inheritance, its semantics
are that of a plain set intersection. The intersection of a nullable
property with a non-nullable property is a non-nullable property.
Therefore, if we want an inherited property to remain nullable, we
need to mark it as such.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
django.utils.translation.ugettext is a deprecated alias of
django.utils.translation.gettext as of Django 3.0, and will be removed
in Django 4.0.
Commit e7ed907cf6 (#18174) fixed this
before, but new instances have been added.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
We do not accept heterogeneous arrays containing both user ids and
email addresses.
This also happens to disallow an empty array, which is fine since the
principals parameter should be omitted if the default to the calling
user is desired.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Fixes “DeprecationWarning: 'jinja2.Markup' is deprecated and will be
removed in Jinja 3.1. Import 'markupsafe.Markup' instead.”
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Updates regex in the openapi markdown extension to match api
endpoint names that contain dashes, which is the case for
`zulip-outgoing-webhook` and `rest-error-handling`.
The subscriber list was not updating without a refresh on
reactivating user, because the subscriptions data with the
client was not updated on reactivation.
This commit adds code to send peer_add subscription events
on reactivating the user.
We do not send peer_remove events on deactivating the user,
but the subscriber list is still live-updated because we
have the data of the streams which the deactivated user is
susbcribed to and the clients itself updates the data and UI
on receiving event of deactivation of user, which it is not
possible when reactivating the user.
Fixes#20383.
Leaving old invitations valid, potentially for a very long time, is
clearly unexpected and undesired behavior under normal circumstances. A
user shouldn't be able to e.g. generate a multiuse invite link, get
banned from the organization by being deactivated and then just re-join
using the link they've created for themselves.
do_revoke_user_invite and do_revoke_multi_use_invite were using objects
after their deletion to pass the argument to notify_invites_changed. We
should avoid that. The function was only using the .realm attribute of
the received objects, so it's simpler to make it just take realm as its
argument.
The msg parameter is a string to be displayed when the expected
exception wasn’t raised, not a pattern to match against the raised
exception’s message.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Adam Sah
Anders Kaseorg
Lauryn Menard
Alex Vandiver
Tim Abbott
Sahil Batra
Siddharth Asthana
Christopher Chong
Aman Agrawal
Chris Bobbe
Zixuan James Li
Mateusz Mandera
evykassirer
Raghav Luthra
Hannah Salameh
Lauryn Menard
jai2201
Alya Abbott
Simmo Saan
Felix
Nikhil Maske
byshen-dev
Suyash Vardhan Mathur
Steve Howell
Kartik Srivastava
Dinesh
NerdyLucifer
Austin Riba
YashRE42
Shantanu
Shubh Gupta
madrix01
Puneeth Chaganti
Sharif Naas
Eeshan Garg
Palash
BIKI DAS