This restructures the API endpoints that we currently have implemented
more or less for exclusive use by the mobile and desktop apps (things
like checking what authentication methods are supported) to use a
system that can be effectively parsed by our test_openapi
documentation.
This brings us close to being able to eliminate
`buggy_documentation_endpoints` as a persistently nonempty list.
This add some regular expression manipulation hacks to make it
possible for us to validate the documentation for the presence
endpoint with a slightly more complex regular expression capture
group.
Previously, our OpenAPI documentation validation was failing for some
endpoints because it didn't account for the `in: path` type of
parameter, resulting in a mismatch between what was declared via REQ
and what was declared in the OpenAPI docs.
We fix this by excluding the path type parameters in both places from
what's considered by documentation using the `path_only` flag.
I doubt this is the correct long-term fix; in particular, I don't
think we're actually running the validators for these path-only
parameters. The examples that exist today are all IDs with validators
for being non-negative numbers, but longer-term I think we'll want to
do something different (possibly at the REQ layer, see the TODO).
Instead of just mocking some fake events, we use the code
path that generates slow query events and publishes them
to SlowQueryWorker.
This test improvement would have got a recent potential regression
caught in code review.
Our new curl example generation logic was broken, in that it hardcoded
localhost:9991 (without an HTTP method or anything) as the API URL.
It requires a bit of plumbing to make this possible.
Apparently, the filters written for the send_password_reset_email (and
some other management commands) didn't correctly consider the case of
deactivated users.
While some commands, like syncing LDAP data (which can include whether
a user should be deactivated) want to process all users, other
commands generally only want to interact with active users. We fix
this and add some tests.
The original seems to be unmaintained
(johnsensible/django-sendfile#65). Notably, this fixes a bug in the
filename parameter, which perviously showed the Python 3 repr of a
byte string (johnsensible/django-sendfile#49).
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
The `users/me/subscriptions` endpoint accidentally started returning
subscriber information for each stream. This is convenient, but
unnecessarily costly for those clients which either don't need it
(most API apps) or already acquire this information via /register
(including Zulip's apps).
This change removes that data set from the default response. Clients
which had come to rely on it, or would like to rely on it in future,
may still access it via an additional documented API parameter.
Fixes#12917.
I changed the class of the title in order to use the same styling as the
other similar pages (like `/accounts/go` or `/login`).
Changed the related test.
For the emails that are associated to an existing account in an
organisation, the avatars will be displayed in the email selection
page. This includes avatar data in what is passed to the page.
Added `avatar_urls` to the context in `test_templates.py`.
Apparently GitHub changed the email address for these; we need to
update our code accordingly.
One cannot receive emails on the username@users.noreply.github.com, so
if someone tries creating an account with this email address, that
person would not be able to verify the account.
The previous iteration still had the failure mode of not actually
testing anything, because it didn't trigger the data export code path
(and in fact was getting an HTTP 401 authentication denied error).
This test was broken due to using an empty `RealmAuditLog`
table. We fix this by mocking the creation of an export,
thus creating an entry, similar to what we do in our other
tests.
Previous cleanups (mostly the removals of Python __future__ imports)
were done in a way that introduced leading newlines. Delete leading
newlines from all files, except static/assets/zulip-emoji/NOTICE,
which is a verbatim copy of the Apache 2.0 license.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Now that we can create cURL examples based on the OpenAPI
documentation. We can begin using simple one line tags in
the documentation instead of manually creating cURL examples.
Fixes part of #12878.
Now we can also include extra keyword arguments to specify
modifications in how the example code should be generated
in the generate_code_example template tag.
E.g. generate_code_example(curl, exclude=["param1", "param2"])
This commit extends api_code_examples.py to support automatically
generating cURL examples from the OpenAPI documentation. This way
work won't have to be repeated and we can also drastically reduce
the chance of introducing faulty cURL examples (via. an automated
test which can now be easily created).
This commit progress our efforts to reduce pending_endpoints
as well as to migrate away from templates/zerver/api/fixtures
and towards our OpenAPI documentation.
Similar to commit d62b75fc.
The current code looks like it's trying to redirect /integrations/doc/email
to /integrations when EMAIL_GATEWAY_PATTERN is not set.
I think it doesn't currently do this. The test for that pathway has a bug:
self.get_doc('integrations/doc-html/email', subdomain='zulip') needs a
leading slash, and putting the slash back in results in the test failing.
This redirection is not really desired behavior -- better is to
unconditionally show that the email integration exists, and just point the
user to https://zulip.readthedocs.io/en/latest/production/email-gateway.html
(this is done in a child commit).
This gives us access to typing_extensions.Deque, which was not added
to typing until 3.5.4.
(PROVISION_VERSION is not bumped because the transitive dependency set
in dev.txt hasn’t changed.)
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This verifies that the client passed a last_event_id that actually
came from the queue instead of making up an ID from the future. It
turns out one of our tests was making up such an ID, but legitimate
clients are expected not to do so.
The previous version of this commit (commit
e00d4be6d5, #12888) had to be reverted
(commit b86c5cc490) because it was
missing the `to_dict`/`from_dict` migration code.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Our implementation requires at least 1 space after the
'#' not not break existing linkifiers like '#123', etc.
that generally follow the convention we show in linkifier
examples.
- [valid] : # Hello
- [valid] : # Hello
- [invalid]: #Hello
For the frontend, we have taken the code from v0.7.0 of
upstream marked and made minor changes to avoid having
to refactor a significant part of our marked code.
For the backend, we merely have to change the regex to
force require spaces after #, and add hashheader to our
list of blockparsers.
Fixes#11418.
This commits reduces the number of values returned by
channel_to_zerver_stream function by setting the values
directly in realm dict and returning it instead.
This verifies that the client passed a last_event_id that actually
came from the queue instead of making up an ID from the future. It
turns out one of our tests was making up such an ID, but legitimate
clients are expected not to do so.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This feature is intended to cover all of our ways of exporting a
realm, not just the initial "public export" feature, so we should name
things appropriately for that goal.
Additionally, we don't want to include data exports in page_params;
the original implementation was actually buggy and would have.
When a person creates a new realm, they'll likely want to create a
bunch of initial streams at once. When doing so, it could be annoying
to have to mark all of the new stream notification messages as read.
Thus to make this process smoother, we should automatically mark
the messages generated by the Notification Bot in the notifications
(announcements) stream, as well as in the newly created stream itself
as read by the stream creator.
Fixes#12765.
This commit add an pretty elaborate extension to the existing
openapi documentation validation test: test_openapi_arguments.
This does a metacode analysis, comparing the openapi documentation
with the appropriate function's declaration, default values etc.
While it has some limitations, it is able to catch various common
classes of mistakes in the types declared for our OpenAPI
documentation.
Django’s default FileSystemFinder disallows STATICFILES_DIRS from
containing STATIC_ROOT (by raising an ImproperlyConfigured exception),
because STATIC_ROOT is supposed to be the result of collecting all the
static files in the project, not one of the potentially many sources
of static files.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This change serves to declutter webhook-errors.log, which is
filled with too many UnexpectedWebhookEventType exceptions.
Keeping UnexpectedWebhookEventType in zerver/lib/webhooks/common.py
led to a cyclic import when we tried to import the exception in
zerver/decorators.py, so this commit also moves this exception to
another appropriate module. Note that our webhooks still import
this exception via zerver/lib/webhooks/common.py.
Changed the requirements for UserProfile in order to allow use of
the formataddr function in send_mail.py.
Converted send_email to use formataddr in conjunction with the commit
that strengthened requirements for full_name, such that they can now be
used in the to field of emails.
Fixes#4676.
We were seeing errors when pubishing typical events in the form of
`Dict[str, Any]` as the expected type to be a `Union`. So we instead
change the only non-dictionary call, to pass a dict instead of `str`.
The code generating pub_dates for messages would fail to distribute them
across days if tot_messages was too large.
We refactor this code as a separate function (for clarity and to unit
test for the bug we're fixing), and change the structure and naming to a
form that more clearly describes what's happening. We also shift away
from the approach of all the float-to-int conversions as this is in
general tricky and bug prone - django's timedelta() handles floats as
arguments, so we take advantage of that.
This replaces the two custom Google authentication backends originally
written in 2012 with using the shared python-social-auth codebase that
we already use for the GitHub authentication backend. These are:
* GoogleMobileOauth2Backend, the ancient code path for mobile
authentication last used by the EOL original Zulip Android app.
* The `finish_google_oauth2` code path in zerver/views/auth.py, which
was the webapp (and modern mobile app) Google authentication code
path.
This change doesn't fix any known bugs; its main benefit is that we
get to remove hundreds of lines of security-sensitive semi-duplicated
code, replacing it with a widely trusted, high quality third-party
library.
During the time between when we refactored the GitHub authentication
backend to use SocialAuthBase and now (when we're about to migrate
GoogleAuthBackend to use that code path as well), we accidentally
added some GitHub-specific authentication backend tests to the common
test class.
Fix this by moving them to the GitHub-specific subclass.
This is a prep commit for adding validation of the request variable
types since then we would need to actually analyze the code of the
actual function itself and we would need a variable storing the
function itself.
In commit 7c71e98, we added a special exception for the
/users/me/subscriptions endpoint in the automatic validation test.
By adding some extra documentation, we now remove this extra code,
as well as the endpoint from the list of pending endpoints.
In the validation test, we now use a different message for when there
is an endpoint in pending_endpoints with some documentation already.
This change is a bit hackish, but it's okay since we'll be removing it
once we've resolved all pending endpoints (which is bound to happen).
If a url doesn't have a scheme, browsers would treat it as a relative
url and open something like: https://chat.zulip.org/google.com instead.
This PR fixes the issue on the backend; the frontend implementation
remains out of sync and the user sending the message wouldn't see
any linkification for urls without a scheme.
Fixes#12791.
The test_docs change is because Django runs test cases with DEBUG =
False, which ordinarily means it doesn’t serve /static during tests.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
Making sender name go in-line with message body only if
the html starts with <p> tag since it won't look good
if the message starts with a code snippet, ul, etc.
If message starts with p tag we can safely assume that
it can go in-line with sender name.
As of commit 8c199fd44c (#12667) this
file is no longer generated. Handlebars compile errors are raised as
webpack errors.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
The decorator running at import time was causing directory
creation in the project's root.
One could imagine linting for this, but it seems unlikely that similar
code will be added in the future; the problem one would be trying to
solved is already addressed by default in the framework now.
In the unlikely event that someone edited the properties of a system
bot and then saved the result, we were still caching the old version
indefinitely in the get_system_bot cache.
This led to a confusing case where a newly installed Zulip server
didn't have is_api_super_user properly set on its EMAIL_GATEWAY_BOT in
memcached.
Co-authored-by: Mateusz Mandera <mateusz.mandera@protonmail.com>
This commit adds a new setting to the user's notification settings that
will change the behaviour of the unread count in the title bar and
desktop application.
When enabled, the title bar will show the count of unread private messages
and mentions. When disabled, the title bar will act as before, showing
the total number of unread messages.
Fixes#1736.
Modified by punchagan to:
* Replace URLs with titles only if the inline url embed previews are turned on
* Add a test for youtube titles replacing URLs
The titles for the videos are fetched asynchronously after the message has been
sent via the code that fetches metadata for open graph previews. So, the URLs
are replaced with titles only if the inline embed url previews feature is
enabled.
Ideally, YouTube previews should be shown only if inline url previews are
enabled, but this feature is in beta, while YouTube previews are pretty stable.
Once this feature is out of beta, YouTube previews should be shown only if the
url previews feature is turned on.
YouTube preview image is calculated as soon as the message is sent, while the
title needs to be fetched using a network request. This means that the URL is
replaced only after the data has been fetched from the request, and happens a
couple of seconds after the message has been rendered.
Closes#7549
Messages with links embedded in blockquotes turn out to be replies to
messages with links, more often than not. Showing previews for links in
replies seems like clutter, and it seems reasonable to turn off previews for
such links.
Modified by punchagan to:
* Add a separate markdown test for de-duplicating inline previews
* Check for number of unique URLs to see if per limit message is crossed
* Use a set for processed URLs instead of a list
Fixes#8379.
Extract some logical segments of test_openapi_arguments into
individual (helper) functions. E.g. extraction of the regex
to OpenAPI URL format conversion and testing.
The previous code for the validator test was fairly messy due to
checking for both formats of the openapi url, one with
<variable_name> and the other with {variable_name}. To eliminate
this, we have standardized the format and restricted it to
{variable_name} as per the official format at:
https://swagger.io/docs/specification/describing-parameters.
These updates are added as a direct result of the new strategy related
to the the following refactorings:
* Having `do_export_realm` return the value of the tarball path.
See 6e187e974a4e6282d3616312bdfa19d0d2a949d1.
* Moving the upload logic for s3 and local tarball storage out of
`export_realm_wrapper` and into `upload.py`.
See f1041e1fb6cb60f2c53b294695245e4c86a4d40b.
Add new custom profile field type, External account.
External account field links user's social media
profile with account. e.g. GitHub, Twitter, etc.
Fixes part of #12302
Rename URL type custom profile field in populate db to avoid confusion
with the "GitHub profile" custom external account profile field we'll
be adding shortly.
We can simply archive cross-realm personal messages according to the
retention policy of the recipient's realm. It requires adding another
message-archiving query for this case however.
What remains is to figure out how to treat cross-realm huddle messages.
In addition to the test which checks to to see if each endpoint in
code (urls.py) is documented in the openapi documentation (and with
the right arugments). We now also have a test to see if every
endpoint in the openapi documentation is a legitimate endpoint
also existing in code.
We do this by piggy-backing on the work done be the former test and
using set operations. This method avoid the need for an extra loop
and it uses set operations for additional speed and ease of reading.
The main things targeted by the refactor are the usage of comments and
moving the top-level variables to the scope of the class.
The movement of variables was to facilitate allowing us to perform
a reverse mapping test from OpenAPI URLs -> Code defined URLs.
By importing a few view modules in the validation test itself we
can remove a few endpoints which were marked as buggy. What was
happening was that the view functions weren't imported and hence
the arguments map was not filled. Thus the test complained that
there was documentation for request parameters that seemed to be
missing in the code. Also, for the events register endpoint, we
have renamed one of the documented request parameters from
"stream" to "topic" (the API itself was not modified though).
We add a new "documentation_pending" attribute to req variables
so that any arguments not currently documented but should be
documented can be properly accounted for.
The conditional block containing the tarball upload logic for both S3
and local uploads was deconstructed and moved to the more appropriate
location within `zerver/lib/upload.py`.
This reverts commit f476ec7fac (#10312)
and replaces it with a proper fix using Jinja2 raw blocks.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
We don’t need a hacked copy anymore. We run the installed version out
of node_modules in development, and a Webpack-bundled version of that
in production.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
In each url of urls.py, if we want to mark an endpoint as being
intentionally undocumented, then in the kwargs instead of directly
mapping like 'METHOD': 'zerver.views.package.foo', we can provide
a tag called 'intentionally_undocumented' and map like:
'METHOD': ('zerver.views.package.foo', {'intentionally_undocumented'}).
If an endpoint is marked as intentionally undocumented, but we find
some OpenAPI documentation for it then we'll throw an error, in which
case either the 'intentionally_undocumented' tag should be removed or
the faulty documentation should be removed.
This will allow us to mark a REQ variable as intentionally
undocumented. With this, we can remove some of the endpoints marked
as "buggy" even though they're not actually buggy, we just needed to
specify certain parameters as intentionally undocumented (e.g. the
stream_id for the /users/me/subscriptions/muted_topics endpoint.)
Any REQ variable with intentionally_undocumentated set to True
will not be added to the arguments_map data structure.
For some of the other "buggy" endpoints, we would want to mark the
entire endpoint as being undocumented intentionally via. the urls.py
file.
This is a dramatic redesign of the look and feel of our missed-message
emails, designed to decrease the feeling of clutter and just provide
the content users care about in a clear, visible fashion.
This cleans up the reply_warning feature in favor of a more coherent
explanation of whether or not one can reply.
(Also, critically, it now advertises the ability to enable
missed-message email replies with some administrative configuration
work.)
In 93914d8cd8, we intended to change our
markdown processor to add support for multi-line /me messages.
However, we neglected to change the backend processor, resulting in
the change only taking effect for the user sending the message :(.
We fix this by changing the backend processor too.
Fixes#12450.
We reuse the link regexes we use elsewhere inn markdown
for parsing links in topic names and add a button to open
them in new tabs similar to our behavior with linkifiers
in topic names.
Fixes#12391.
When archiving Messages, we stop relying on LEFT JOIN ... IS NULL to
avoid duplicates when INSERTing. Instead we use ON CONFLICT DO UPDATE
(added in postgresql 9.5) to, in case of archiving a Message that
already has a corresponding archived objects (this happens if a Message
gets archived, restored and then archived again), re-assign the existing
ArchivedMessage to the new transaction.
This also allows us to fix test_archiving_messages_second_time, which
was temporarily disable a few commits before.
Instead of having a bunch of custom code in the function, we make it use
run_message_batch_query and run_archiving_in_chunks to do the necessary
operations in a consistent way, using the same codepaths as the rest of
the archiving system.
This breaks test_archiving_messages_second_time temporarily, but we will
fix it and re-enable the test in the next commits, where we'll address
various other issues with re-archiving of messages.
We also remove the @transaction.atomic wrapper, because atomicity is
handled by the logic inside run_archiving_in_chunks.
For storing HTTP headers as a function of fixture name, previously
we required that the fixture_to_headers method should reside in a
separate module called headers.py.
However, as in many cases, this method will only take a few lines,
we decided to move this function into the view.py file of the
integration instead of requiring a whole new file called headers.py
This commit introduces the small change in the system architecture,
migrates the GitHub integration, and updates the docs accordingly.
The markup output changed but the rendering is the same, so modified
expected output in tests.
There is a regression introduced in one of the new versions of KaTeX,
which produces a warning in our node tests:
```
No character metrics for ' ' in style 'Main-Bold'
```
but the rendering is correct so we can ignore it.
Tracking issue: KaTeX/KaTeX#1994
Fixes#12472.
When parsing custom HTTP headers in the integrations dev panel, http
headers from fixtures system and the send_webhook_fixture_message
we now use a singular source of logic: standardize_headers which
will take care of converting a dictionary of input headers into a
standard form that Django expects.
Previously, our Github authentication backend just used the user's
primary email address associated with GitHub, which was a reasonable
default, but quite annoying for users who have several email addresses
associated with their GitHub account.
We fix this, by adding a new screen where users can select which of
their (verified) GitHub email addresses to use for authentication.
This is implemented using the "partial" feature of the
python-social-auth pipeline system.
Each email is displayed as a button. Clicking on that button chooses
the email. The email value is stored in a hidden input above the
button. The `primary_email` is displayed on top followed by
`verified_non_primary_emails`. Backend name is also passed as
`backend` to the template, which in our case is GitHub.
Fixes#9876.
Using this system, we can now associate any fixture of any integration
with a particular set of HTTP headers. A helper method called
determine_http_headers was introduced, and the test suite was upgraded
to use determine_http_headers.
Comments and documentation significantly edited by tabbott.
This function is an alternative to get_admin_users that we use in all
places where we explicitly want only human administrative users (not
administrative bots). The following commits will rename
get_admin_users for better clarity.
We also document support for user IDs in the pm-with narrow operator.
Edited by tabbott to document on /api rather than in the /help page.
Fixes part of #9474.
Namely, here we add the "plan_includes_wide_organization_logo" and
"upgrade_text_for_wide_organization_logo" to the page_params (which
is set in zerver/lib/events.py).
"plan_includes_wide_organization_logo" is True if the plan is not of
the Realm.LIMITED type. We need to add this extra boolean parameter
instead of just using "realm_plan_type" to make things a lot easier
to work with on the frontend side, especially considering that
handlebars won't allow checking for equality in its {{#if}} blocks.
When a realm's plan type is updated using "do_change_plan_type" we
notify active users of the realm. This way certain plan features
could be enabled instantaneously for active users.
This fixes an issue that caused LDAP synchronization to fail for
avatars. The problem occurred due to the lack of a 'name' attribute
on the BytesIO object that we pass to the upload backend (which is
only used in the S3 backend for computing Content-Type).
Fixes#12411.
To ensure the database retains a consistent state if archiving gets
interrupted, we process each Messages chunk together with related
objects in a single atomic transaction.
Rename notification property `enable_stream_sounds` to
`enable_stream_audible_notifications` to match with other
notification property patterns.
Fixes part of #12304
We batch queries that archive Messages, to limit the maximum amount of
Message objects archived in a single query. This leads to the archiving
of other related objects being batched as well, because we loop over
chunks of archived messages and archive their related objects per-chunk.
This validation is incomplete, in large part because of the long list
of TODOs in this code. But this test should provide a ton of support
for us in avoiding regressions as we work towards having complete API
documentation.
See https://github.com/zulip/zulip/issues/12521 for a bunch of
follow-up improvements.
We add the following behavior:
If stream has message_retention_days set to -1, archiving for it is
disabled.
If stream has message_retention_days set to null, use the realm's
policy. If the realm has no policy, we don't archive for this stream.
We change the archiving scheme to allow having stream based retention
policies. In the first step of the archiving process, we loop over
streams and archive their expired messages and related objects.
Then we separately archive all expired personal and huddle messages and
related objects. As the last step, we scan for redundant attachments
which can now be deleted.
To achieve this, we have to rewrite a significant portion of the
retention code and rework some of the database queries.
For the sake of simplicity, we neither archive nor delete cross-realm
messages, except cross-realm stream messages – in their case they can
be processed in the same manner as ordinary stream messages.
In the query for archiving personal and huddle messages we simply
exclude those sent by cross-realm bots.
We change the tests to adapt to these modifications.
Previously, we didn't have validation to prevent editing certain flags
that don't make sense for a client to edit, like whether a user was
mentioned in a given message.
This isn't a security issue -- the user could only mess up their own
personal search results (etc.), but it does seem worth fixing to avoid
confusion for folks developing Zulip clients.
While we're at it, clearly document the situation in comments.
This adds a setting to control Zulip's default behavior of sorting to
bottom and graying out inactive streams. The previous logic is still
the default "automatic", but this gives users more control. See the
models.py comment for details.
Fixes#11524.
We add RETURNING to fetch relevant message and usermessage ids in
archiving queries and use them to make other queries faster and slower.
A side-effect of this implementation is that with cross-realm messages,
the UserMessage of the recipient and the Message will not be deleted -
but cross-realm messages are rare, will still get correctly put in the
archive tables and so failing to delete should not be a problem for now.
They will be fully handled later.
In addition to the "+show-sender" option, we now add "+include-footers"
which disables stripping of the footer from the email body if this token
is included in the email address.
To enable a comfortable way of adding more optional tokens in the
address (like current '+show-sender') we change decode_email_address to
return a general dictionary containing options specified through adding
these optional tokens in the To: address. For now, we only have
"+show-sender", but more can be easily added using this change.
The RealmAuditLog object ID was stored in the event sent to the
deferred_work queue as a means to update the row's extra_data field.
The extra_data field then stores the location of the export.
Ensure that the html is safe, before using it. The html is considered if it is
in an iframe with a http/https src, based on the recommendations here:
https://oembed.com/#section3
We directly embed the `iframe` html into the lightbox overlay.
We add general code that will archive models that are tied to a specific
Message (such as Reactions and SubMessages). Certain details of the
model are grabbed from a list models_with_message_key, and then used to
create queries that will archive these database tables.
We put Reaction in that list in this commit, and add appropriate tests.
To have archiving of other analogical models (for example SubMessage),
one only needs to make an appropriate entry in the
models_with_message_key list.
Previously, if you exported a Zulip organization and then re-imported
it, we'd end up renumbering the user IDs and all direct foreign key
references to them in the database, but not the data-user-id
references in mentions. Fix this by parsing the message content and
doing that renumbering.
(Because we import raw markdown, not HTML, from third-party tools,
these changes won't affect data import from slack etc.)
Fixes the high-priority part of #11293.
Modifies the dict with the user info to include the key `bot_owner_id`
so it can be displayed in the user info popover.
Tests concerned with changing bot owner have been modified to have
number of events=2 because while updating the bot info, two events
are fired -- updating the `realm_bot` and `realm_user` since the
key `bot_owner_id` is a part of realm user info.
Since positional arguments are interpreted differently by different
backends in Django's authentication backend system, it’s safer to
disallow them.
This had been the motivation for previously declaring the parameters
with default values when we were on Python 2, but that was not super
effective because Python has no rule against positional default
arguments and that convention for our authentication backends was
solely enforced by code review.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
The `queue_data` variable is an intermediate step that's unnecessary.
Instead, the values from the queue event are assigned dierectly.
Also, the `worker` variable is not worth an assignment as it is only
referenced a single time per test case.
A FileNotFound error was set as the side-effect of the do_export_realm
mock and the DeferredWorker was made to consume the event explicitly.
Previously, the mock of do_export_realm was producing spammy output
as a result of a FileNotFound error coming from the queue processing of
`do_write_stats_file_for_realm_export`.
A unique path was created using the `LOCAL_UPLOADS_DIR` backend, similar
to the code used in `LocalUploadBackend`. The exported tarball was
copied to the directory, and an nginx url was created to serve the file
publicly.
Tweaked by tabbott to output an actual URL.
This cleans up the pattern for how we check which user is logged in
during Zulip's backend unit tests to be much more readable (replacing
the arcane session code that does this check).
test_retention.py had various issues - we opt for keeping its essence
(what should the tests do and verify), but rewriting a lot of it in
order to have more clarity in what's happening there.
We split archive_messages code into two functions: moving to archive and
cleanup. This allows cleaning up the tests - they can call
these functions directly instead of copying several lines of
archive_messages here and there in multiple tests.
test_cross_realm_messages_archiving_two_realm_expired doesn't run the
code path patched in commit 3d1aa98b2ea344fba7fbb2373a37d4cf30f53e08i,
so it can still fail. We apply the analogical change in the test as
in the cited commit.
This is probably a good idea for the production use case, since then
there's some consistency of behavior, and if we extend logging, one
knows exactly which realms were or were not executed before a logged
failure.
This fixes the nondeterministic test failures we've been seeing in CI:
if you use `-id` in that order_by, it happens consistently.
Sending PM from a hamlet(consented) to othello is a case
of sending message from a consented user to a non consented
user. This result in the generation of more than one message
files during realm export. To handle this case _export_realm
is updated.
The upload option will no longer be limited to strictly S3 uploads. This
commit serves as a preliminary step for supporting LOCAL_UPLOADS_DIR as
part of the public only export feature.
We've been seeing nondeterministic failures in this test suite in CI
that we can't reproduce locally; these print statements should help
track them down.
This is the only function in TestEmailMirrorLibrary, so we rename this
class to more appropriate TestGetMissedMessageToken, clean it up a bit
and add some extra checks to finally get email_mirror.py to 100% test
coverage.
log_and_report and its helper functions were mostly old code no longer
well adapted to how email mirror works currently, as well as having no
test coverage. We rewrite this part of the email to report errors in a
similar manner, and add tests for it. We're able to get rid of the
clunky and now useless debug_info dictionary in process message, as
log_and_report only needs the recipient email in its third argument.
Mostly rewritten by Tim Abbott to ensure it correctly implements the
desired security model.
Administrators should have access to users' real email address so that
they can contact users out-of-band.
Clients won't have access to user email addresses, and thus won't be
able to compute gravatars.
The tests for this are a bit messy, in large part because our tests
for get_events call subsections of it, rather than the main function.
This provides a clean warning and 40x error, rather than a 500, for
this corner case which is very likely user error.
The test here is awkward because we have to work around
https://github.com/zulip/zulip/issues/12362.
The `LocalUploadBackend` returns a relative URL, while the `S3UploadBackend`
returns an absolute URL. This commit switches to using `urljoin` to obtain the
absolute URL, instead of simply joining strings.
This commit also adds a small functionality change where the results of
each webhook fixture message sent is now displayed to the user.
With a small tweak by tabbott to fix a styling bug.
Fixes#12122.
Note: If you're going to send fixtures which are not JSON or of the
text/plain content type, make sure you set the correct content type
in the custom headers.
E.g. For the wordpress fixtures the "Content-Type" should be set to
"application/x-www-form-urlencoded".
This is a very old commit for #106, which has been on hiatus for a few
years. It was significantly modified by tabbott to:
* Improve coding style and variable names
* Update mypy annotations style
* Clean up the testing logic
* Update for API changes elsewhere in our system
But the actual runtime code is essentially unmodified from the
original work by Kirill.
It contains basic support for archiving Messages, UserMessages, and
Attachments with a nice test suite. It's still not usable in
production (e.g. it will probably break Reactions, SubMessages, etc.),
but upcoming commits will address that.
This commit introduces a simple field where the user can now specify custom
HTTP headers. This commit does not introduce an improved system for storing
HTTP headers as fixtures - such a change would modify both the existing unit
tests as well as this devtool.
This commit adds a new developer tool: The "integrations dev panel"
which will serve as a replacement for the send_webhook_fixture_message
management command as a way to test integrations with much greater ease.
This lets us handle directly in our tooling the user experience that
we document for exporting a realm with member consent (before, it
required unpleasant manual work).
We may be successfully able to get the page once, to get the content type, but
the server or network may go down and cause problems when fetching the page for
parsing its meta tags.
Currently, we only show previews for URLs which are HTML pages, which could
contain other media. We don't show previews for links to non-HTML pages, like
pdf documents or audio/video files. To verify that the URL posted is an HTML
page, we verify the content-type of the page, either using server headers or by
sniffing the content.
Closes#8358
We had some excessively tight rules about what characters were
allowed, which in particular prevented using `?foo=bar&baz=quux`
structures in the realm filters URLs.
Fixes#12239.
`youtube.com/playlist?list=<list-id>` incorrectly matches the regex since the
change in 8afda1c1bb. The regex was modified to
match URLs of the form `youtu.be/<id>` and this playlist URL incorrectly matches
with the `<id>` set to `playlist`.
This commit avoids this match by verifying that the ID is not playlist.
This renames Subscription.in_home_view field to is_muted, for greater
clarity as to what it does just from seeing the setting name, without
having to look it up.
Also disabled an obsolete test_migrations test.
Fixes#10042.
These tests have some code and comments that only used to apply when
these empty body scenarios used to raise the regular
ZulipEmailForwardError - now they raise ZulipEmailForwardUserError.
We adapt the tests to this fact and test by mocking logging.warning and
making sure it gets called with the intended warning message. This is
also needed to cover the ZulipEmailForwardUserError case with tests to
get to 100% coverage of email_mirror.py.
We add a test for the case "if not all(val is not None for val in result):"
on result returned by redis_client.hmget in send_to_missed_message_address.
A couple of tests asserted that the number of queries were within a range,
because they ran one additional query when they were run individually, as
compared to running all the tests in `TestDigestEmailMessages`. We now trigger
these additional queries within the tests, to make the tests deterministic and
assert that the number of queries is a number, instead of a range.
Digest emails were disabled for soft deactivated users, since UserMessage
objects are created for such users lazily when they return.
We now compute the message list for gathering hot conversations by looking at
all the messages sent to the streams where the user is subscribed, while they
were subscribed.
Fixes#6297
If the text part of an email message didn't specify the charset in the
Content-Type header, the text content wouldn't be found. We fix this, by
assuming us-ascii charset in those cases, as specified by RFC6657:
https://tools.ietf.org/html/rfc6657
This commit migrates the Subscription's notification fields from a
BooleanField to a NullBooleanField where a value of None means to
inherit the value from user's profile.
Also includes a migrations to set the corresponding settings to None
if they match the user profile's values. This migration helps us in
getting rid of the weird "Apply to all" widget that we offered on
subscription settings page.
The mobile apps can't handle None appearing as the stream-level
notification settings, so for backwards-compatibility we arrange to
only send True/False to the mobile apps by applying those defaults
server-side. We introduce a notification_settings_null value within a
client_capabilities structure that newer versions of the mobile apps
can use to request the new model.
This mobile compatibility code is pretty effectively tested by the
existing test_events tests for the subscriptions subsystem.
If MAX_FILE_UPLOAD_SIZE is set to 0, then UI elements like the upload
icon in the compose and message edit UI and "Attachments" menu in
"/#settings" are not displayed.
A different error message is also displayed if a user tries to drag and
drop or paste a file into the compose message box.
Fixes#12152.
Fixes#12273.
When running the test_query_email_attr test in reverse, the test failed
because self._LDAPUser.attrs was being modified and it was being shared
with other tests.
This makes the implementation of `get_realm` consistent with its
declared return type of `Realm` rather than `Optional[Realm]`.
Fixes#12263.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This commit replaces the `create_stream_by_admins_only` setting with a
new `create_stream_policy` setting, which mirroring the structure of
the existing `invite_to_stream_policy`.
This is important preparation for migrating the waiting period feature
to be its own independent setting.
Fixes#12236.
Using sys.exit in a management command makes it impossible
to unit test the code in question. The correct approach to do the same
thing in Django management commands is to raise CommandError.
Followup of b570c0dafa
Fixes#12251.
Previously when disabling name changes in server settings, instead
of realm settings, the name edit button did not get disabled.
Changing name resulted in a message stating `no changes made`.
Fixes#12132.
Realm setting to disable avatar changes is already present.
The `AVATAR_CHANGES_DISABLED` setting now follows the same
2-setting model as `NAME_CHANGES_DISABLED`.
This is useful when syncing avatars from an integrated LDAP/active
directory.
The upload avatar and delete avatar buttons are hidden if avatar
changes are disabled and the user is a non-admin.
If the user has a gravatar set, then the user will not be able to
upload an image as their avatar if avatar changes are disabled.
Part of #12132.
This module is used to render the HTML of pages like our user documentation
into text for use in open graph previews of those articles. It provided somewhat
confusing output in the case that there were paragraph breaks in the original message,
because text with multiple paragraphs and list items does't read very well. This commit
adds `|` as a delimiter between paragraphs, and prefixes list items with a `*`.
Closes#12228
When an emoji is nested inside another inline tag - like em or strong -
it was getting double processed because of the way the inlinePattern
TreeProcessor runs (it runs recursively). With this fix, we set the
inner text of the emoji span as an AtomicString, preventing us from
double processing the emoji's text.
Fixes#11621
Test Plan:
* Add test case for **😄**, verify it passes.
* Go into local dev server and send "**😄**" to self and verify the DOM
does not have double <span> tags for the emoji.
* Run zerver.tests.test_push_notifications and verify the markdown test case matches
the text_content field properly
We create rate_limit_entity as a general rate-limiting function for
RateLimitedObjects, from code that was possible to abstract away from
rate_limit_user and that will be used for other kinds of rate limiting.
We make rate_limit_user use this new general framework from now.
This enables the function to either return a valid UserProfile or raise
InvalidMirrorInput, which is clearer and more pythonic than the previous
approach of a tuple of a bool and Optional[UserProfile].
In making the type clearer, this improves checking with mypy.
Tests updated.
This commit creates a new organization setting that determines whether
a user can invite other users to streams. Previously this was linked
to the waiting period threshold, but this was both not documented and
overly limiting.
With significant tweaks by tabbott to change the database model to not
involve two threshhold fields, edit the tests, etc.
This requires follow-up work to make the create stream policy setting
work how this code implies it should.
Fixes#12042.
The github-services model for how GitHub would send requests to this
legacy integration is no longer available since earlier in 2019.
Removing this integration also allows us to finally remove
authenticated_api_view, the legacy authentication model from 2013 that
had been used for this integration (and other features long since
upgraded).
A few functions that were used by the Beanstalk webhook are moved into
that webhook's implementation directly.
An endpoint was created in zerver/views. Basic rate-limiting was
implemented using RealmAuditLog. The idea here is to simply log each
export event as a realm_exported event. The number of events
occurring in the time delta is checked to ensure that the weekly
limit is not exceeded.
The event is published to the 'deferred_work' queue processor to
prevent the export process from being killed after 60s.
Upon completion of the export the realm admin(s) are notified.
This slows down the tests by about 5-10% -- the tests go from 0.6s to 0.630s or
so. But, this seems like a change worth making to prevent open-graph metadata
breaking HTML.
The entire idea of doing this operation with unchecked string
replacement in a middleware class is in my opinion extremely
ill-conceived, but this fixes the most pressing problem with it
generating invalid HTML.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This reverts commit fd9dd51d16 (#1815).
The issue described does not exist in Python 3, where urllib.parse now
_only_ accepts (Unicode) str and does the right thing with it. The
workaround was not being triggered and would have failed if it were.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This contains email of the user to whom notification is being
send. This has not been used in any past mobile releases, so it is
safe to remove it.
As user_id will be stable for the user, but not email. So it's better to
start consuming `user_id` instead of email on mobile.
Calls to `render_markdown_path` weren't getting cached since the context
argument is unhashable, and the `ignore_unhashable_lru_cache` decorator ignores
such calls. This commit adds a couple of more decorators - one which converts
dict arguments to the function to a dict items tuple, and another which converts
dict items tuple arguments back to dicts. These two decorators used along with
the `ignore_unhashable_lru_cache` decorator ensure that the calls to
`render_markdown_path` with the context dict argument are also cached.
The time to run zerver.tests.test_urls.PublicURLTest.test_public_urls drops by
about 50% from 8.4s to 4.1s with this commit. The time to run
zerver.tests.test_docs.DocPageTest.test_doc_endpoints drops by about 20% from
3.2s to 2.5s.
This fixes an issue where the hanging unordered list was not
rendering in blockquote; the problem was that we were not
adding an empty line(to satisfy the markdown) for hanging
unordered list if it is in blockquote. Both blockquote
and code block is fenced but we want to avoid rendering
the list if it's in the code block but not in blockquote.
Fixes: #11916.
This is important because upcoming features will include slightly more
complex logic in post_process_state that we'd ideally like to be
included in what this suite tests.
This requires a few related changes:
* A small change to post_process_state to sort the realm_users objects
by user_id to ensure those data structures are stable.
* Improvements to the logic for checking if the initial state has
changed to use match_states for better output.
Extend the list of users that have to be notified when a message is
changed, so that in addition to users who have a UserMessage row, any
users who subscribed later to a stream with history public to
subscribers will also get the update.
Fixes: #8750.
This adds experimental support in /register for sending key
statistical data on the last 1000 private messages that the user is a
participant in. Because it's experimental, we require developers to
request it explicitly in production (we don't use these data yet in
the webapp, and it likely carries some perf cost).
We expect this to be extremely helpful in initializing the mobile app
user experience for showing recent private message conversations.
See the code comments, but this has been heavily optimized to be very
efficient and do all the filtering work at the database layer so that
we minimize network transit with the database.
Fixes#11944.
Previously, we had some expensive-to-calculate keys in
zulip_default_context, especially around enabled authentication
backends, which in total were a significant contributor to the
performance of various logged-out pages. Now, these keys are only
computed for the login/registration pages where they are needed.
This is a moderate performance optimization for the loading time of
many logged-out pages.
Closes#11929.
With the previous commit, fixes#1836.
As specified in the issue above, we make
get_email_gateway_message_string_from_address raise an exception if
it doesn't recognise the email gateway address pattern. Then, we make
appropriate adjustments in the codepaths which call this function.
These functions don't really belong in actions.py, so we move them out,
into email_mirror_helpers.py. They can't go directly into
email_mirror.py or we'd get circular imports resulting in ImportError.
The hope is that by having a shorter list of initial streams, it'll
avoid some potential confusion confusion about the value of topics.
At the very least, having 5 streams each with 1 topic was not a good
way to introduce Zulip.
This commit minimizes changes to the message content in
`send_initial_realm_messages` to keep the diff readable. Future commits will
reshape the content.
There were several problems with the old format:
* The sender was not necessarily the sender; it was the person who did
the deletion (which could be an organization administrator)
* It didn't include the ID of the sender, just the email address.
* It didn't include the recipient ID, instead having a semi-malformed
recipient_type_id under the weird name recipient_user_ids.
Since nothing was relying on the old behavior, we can just fix the
event structure.
Closes#2420
We add rate limiting (max X emails withing Y seconds per realm) to the
email mirror. By creating RateLimitedRealmMirror class, inheriting from
RateLimitedObject, and rate_limit_mirror_by_realm function, following a
mechanism used by rate_limit_user, we're able to have this
implementation mostly rely on the already existing, and proven over
time, rate_limiter.py code. The rules are configurable in settings.py in
RATE_LIMITING_MIRROR_REALM_RULES, analogically to RATE_LIMITING_RULES.
Rate limit verification happens in the MirrorWorker in
queue_processors.py. We don't rate limit missed message emails, as due
to using one time addresses, they're not a spam threat.
test_mirror_worker is adapted to the altered MirrorWorker code and a new
test - test_mirror_worker_rate_limiting is added in test_queue_worker.py
to provide coverage for these changes.
We clean up test_mirror_worker for more readability, as well as make it
verify that mirror_email gets called the correct amount of times and use
a correct rcpt_to address, so that the test doesn't fail when some
verification of the address is added in the following commits
implementing rate limiting in the email mirror.
Fixes#9840.
Old addresses caused bugs in some cases with non-latin characters in
stream names (see issue number above). We switch to using django's
slugify helper function to convert stream names to full ascii, while
also getting rid of problematic non-alphanumeric characters, in a
reasonable way. See Django's documentation for slugify to see more about
how this function works.
Tests extended by tabbott to cover cases where we do end up with ascii.
To prepare for changing how the stream name gets encoded into mirror
email addresses while making sure old addresses keep working, we ignore
the stream_name part when receiving emails into the mirror and we only
look at the email_token to identify into which stream to mirror the
email.
See the comment, but this is a significant performance optimization
for all of our pages using common_context, because this code path is
called more than a dozen times (recursively) by common_context.
We never intended to render them for this use case as the result would
not look good, and now we have a convenient bugdown option for
controlling this behavior.
Since we're not storing the markdown rendering anywhere, there's
conveniently no data migration required.
Fixes#11889.
This renames references to user avatars, bot avatars, or organization
icons to profile pictures. The string in the UI are updated,
in addition to the help files, comments, and documentation. Actual
variable/function names, changelog entries, routes, and s3 buckets are
left as-is in order to avoid introducing bugs.
Fixes#11824.
Follow up on 92dc363. This modifies the ScheduledEmail model
and send_future_email to properly support multiple recipients.
Tweaked by tabbott to add some useful explanatory comments and fix
issues with the migration.
Apparently, our invalid realm error page had HTTP status 200, which
could be confusing and in particular broken our mobile app's error
handling for this case.
When soft deactivation is run for in "auto" mode (no emails are
specified and all users inactive for specified number of days are
deactivated), catch-up is also run in the "auto" mode if
AUTO_CATCH_UP_SOFT_DEACTIVATED_USERS is True.
Automatically catching up soft-deactivated users periodically would
ensure a good user experience for returning users, but on some servers
we may want to turn off this option to save on some disk space.
Fixes#8858, at least for the default configuration, by eliminating
the situation where there are a very large number of messages to recover.
A user who has been soft deactivated for a long time might have 10Ks of message
history that was "soft deactivated". It might take a minute or more to add
UserMessage rows for all of these messages, causing timeouts. So, we paginate
the creation of these UserMessage rows.
This logic for passing through whether the user was logged in never
worked, because we were trying to read the client.
Fix this, and add tests to ensure it never breaks again.
Restructured by tabbott to have completely different code with the
same intent.
Fixes#11802.
Previously, the LDAP authentication model ignored the realm-level
settings for who can join a realm. This was sort of reasonable at the
time, because the original LDAP auth was an SSO solution that didn't
allow multiple realms, and so one could fully configure authentication
settings on the LDAP side. But now that we allow multiple realms with
the LDAP backend, one could easily imagine wanting different
restrictions on them, and so it makes sense to add this enforcement.
This field is primarily intended to support avoiding displaying the
"more topics" feature in new organizations and streams, where we might
know that all messages in the stream are already available in the
browser.
Based on original work by Roman Godov, and significantly modified by
tabbott.
The second migration involved here could be expensive on Zulip Cloud,
but is unlikely to be an issue on other servers.
The actual bug in #11791 was caused by code reverted in
3ed85f4cd7, so technically #11791 is
already fixed. However, it makes sense to add tests to ensure that it
doesn't regress in the future as part of closing out the issue.
Fixes#11791.
Apparently, our new validator for stream color having a valid format
incorrectly handled colors that had duplicate characters in them.
(This is caused in part by the spectrum.js logic automatically
converting #ffff00 to #ff0, which our validator rejected). Given that
we had old stream colors in the #ff0 format in our database anyway for
legacy, there's no benefit to banning these colors.
In the future, we could imagine standardizing the format, but doing so
will require also changing the frontend to submit colors only in the
6-character format.
Fixes an issue reported in
https://github.com/zulip/zulip/issues/11845#issuecomment-471417073
Addresses point 2 of #10612. We use a regex to detect if a form
of FWD indicator is present at the beginning of the subject, which
means the message has been forwarded.
remove_quotations argument is added to a couple of functions where
it's necessary.
In filter_footer, the criteria for a line to be a possible beginning
of a footer is changed to line.strip() == "--", instead of
line.strip().startswith("--"), because the former would remove
quotations from plaintext emails. This change makes sense, because
RFC 3676 specifies ""-- " as the separator line between the body
and the signature of a message":
https://tools.ietf.org/html/rfc3676
We remove the 'subject' argument of process_stream_message and make
subject processing happen inside the function, as it's a more
appropriate place than the general process_message function and is
needed to have a good way of disabling removing quotations in forwarded
emails sent into the mirror.
This used to have a single function test_email_subject_stripping which
would run through a sizeable list of example subjects from subjects.json
fixture, form an email with each subject, send it to the email mirror
and check if the resulting stream message has a correctly stripped
topic. That took too much time, because we run through the entire
process_message and most_recent_message codepaths a lot of times.
We change the way of testing to:
1. Ensure process_message applies subject stripping (only need to run
process_message twice here)
2. Test the strip_from_subject function separately, on all the example
from the subjects.json fixtures. This is very fast.
Some urls which end with image file extensions (eg .jpg) may link to
html pages. This adds handling for linx.li, wikipedia.org and
pasteboard.co. If it is possible, we redirect to the actual image url
otherwise we do not attempt to render it as an image.
Fixes#10438.
Fixes part 3 of #10612. When sending an email to the email mirror to a
stream address, if "+show-sender" is added in the address, the stream
message will now include "From: <sender>" at the top.
The test_events system was in several tests using get_realm to fetch a
realm object, rather than accessing self.user_profile.realm. This
created subtle problems where we were neither directly editing nor
refreshing the `realm` object associated with our UserProfile object
from the database after our the `do_*` methods.
The payoff for this is we can update the previously confused
`do_change_icon_source` test to actually change the state and have the
correct result.
This reverts commit ff90c0101c but keeps
the test cases added for reference.
This was reverted because it was both not a clean solution and created
other realm filters bugs involving dashes (etc.).
Earlier the behavior was to raise an exception thereby stopping the
whole sync. Now we log an error message and skip the field. Also
fixes the `query_ldap` command to report missing fields without
error.
Fixes: #11780.
This fixes an issue where invalid emoji name prevents following
emojis from rendering.
This reverts the code change in
8842349629, while still passing the
tests added in that commit (it seems the original commit had
misdiagnosed an ordering bug and thus introduced this issue).
Fixes: #11770.
The night logo synchronization on the settings page was perfect, but
the actual display logic had a few problems:
* We were including the realm_logo in context_processors, even though
it is only used in home.py.
* We used different variable names for the templating in navbar.html
than anywhere else the codebase.
* The behavior that the night logo would default to the day logo if
only one was uploaded was not correctly implemented for the navbar
position, either in the synchronization for updates code or the
logic in the navbar.html templates.
This commit leverages the ahocorasick algorithm to build a set of user_ids
that have their alert_words present in the message. It runs in linear time
of the order of length of the input message as opposed to number of
alert_words. This is after building a ahocorasick Automaton which runs
in O(number of alert_words in entire realm) which is usually cached.
This fixes an issue where blank lines between blocks were causing
auto-numbering of list to stop before the blank line resulting
in two separate numbered list instead of one.
Edited significantly by tabbott to explain the tricky details in the
comments.
Fixes: #11651.
Add `max_int_size` parameter to `to_non_negative_int()` in
decorator.py so it will be able to validate that the integer doesn't
exceed the integer maximum limit.
Fixes#11451
This is important for situations such as with our Zapier app,
where the requesting user may be a bot that would like to access
its owner's subscriptions.
Tweaked by tabbott to eliminate the 2^N growth of cases in
do_get_streams.
tests now ran in 7.649s from 9.297s. And this test works just as well
with 3 bots, since only 3 database queries with 3 bots confirms we're
not doing linear queries in the number of bots in the organization.
We want to use the baseline features of bugdown, but not fancy things
like inline URL previews, since the whole structure of stream
descriptions is to have a single-line thing supporting some
formatting.
The migration part of this change fixes a bug encountered by some
organizations upgrading from older versions of Zulip.
This allows us to have some features using bugdown rendering where
inline image previews will not be rendered (which would be problematic
for e.g. stream descriptions).
Guest users will just get an empty list of default streams; we also
hide the "Default streams" organization view from the guest users UI.
This is for consistency with not providing guest users the full list
of streams in an organization.
Fixing this involves fixing the backend to handle unchanged field
submissions of the Zoom credentials without trying to re-validate the
credentials (for performance) as well as to fetch the already-sent
secret.
Visually, #zoom_help_text acts like
.organization-settings-parent div:first-of-type when the Zoom option
is selected, but isn't treated as such.
No visual change with the #google_hangouts_domain change; just there to make
the code more readable/defensible.
When a bunch of messages with active notifications are all read at
once -- e.g. by the user choosing to mark all messages, or all in a
stream, as read, or just scrolling quickly through a PM conversation
-- there can be a large batch of this information to convey. Doing it
in a single GCM/FCM message is better for server congestion, and for
the device's battery.
The corresponding client-side logic is in zulip/zulip-mobile#3343 .
Existing clients today only understand one message ID at a time; so
accommodate them by sending individual GCM/FCM messages up to an
arbitrary threshold, with the rest only as a batch.
Also add an explicit test for this logic. The existing tests
that happen to cause this function to run don't exercise the
last condition, so without a new test `--coverage` complains.
We do not anticipate our UI for showing stream descriptions looking
reasonable for multi-line descriptions, so we should just ban creating
them.
Given the frontend changes, multi-line descriptions are only likely to
show up from importing content from other tools, in which case
replacing newlines with spaces is cleaner than the alternative.
This change should help people discover to distinguish
silent mentions in text as a part of Zulip syntax while
differentiating them from regular mentions.
ACCOUNT_ACTIVATION_DAYS doesn't seems to be used anywhere.
INVITATION_LINK_VALIDITY_DAYS seems to do it's job currently.
(It was only ever used in very early Zulip commits).
Since da8f4bc0e back in August, this control flow has caused
`flags.active_mobile_push_notification` to be cleared if we don't send
these `remove` messages at all, and if we send them directly to GCM...
but not if we send them via the Zulip notification bouncer.
As a result, on a server configured to send `remove` notification-messages
via the bouncer, we accumulate "active" messages and never clear them.
If the user then does `mark_all_as_read`, we end up sending a `remove`
for each of those messages again, and all in one giant burst. We've
seen puzzling bursts of hundreds of removals pass through the bouncer
since turning on removals on chat.zulip.org; it's likely many of them
are caused by this bug.
This issue was made more acute with f4478aad5, which unconditionally
enabled removals.
Test added by tabbott.
The client-side fix to make these not a problem was in release
16.2.96, of 2018-08-22. We've been sending them from the
development community server chat.zulip.org since 2018-11-29.
We started forcing clients to upgrade with commit fb7bfbe9a,
deployed 2018-12-05 to zulipchat.com.
(The mobile app unconditionally makes a request to a route on
zulipchat.com to check for this kind of forced upgrade, so that
applies to mobile users of any Zulip server.)
So at this point it's long past safe for us to unconditionally
send these. Hardwire the old `SEND_REMOVE_PUSH_NOTIFICATIONS`
setting to True, and simplify it out.
For Google auth, the multiuse invite key should be stored in the
csrf_state sent to google along with other values like is_signup,
mobile_flow_otp.
For social auth, the multiuse invite key should be passed as params to
the social-auth backend. The passing of the key is handled by
social_auth pipeline and made available to us when the auth is
completed.
For internal stream messages, most of the time, we have access to
a Stream object. For the few corner cases where we don't, it is a
much cleaner approach to have a separate function that accepts a
stream name than having one multi-option helper that accepts both
names and objects.
Our html collects extra spaces in a couple of places. The most prominent is
paragraphs that look like the following in the .md file:
* some text
continued
The html will have two spaces before "continued".
This changes the border-radius to 6px for the tabbed display, which is not
in line with the current Zulip style for border-radius (4px). However 6px
really looks a lot better for this (possibly because it's a bigger box than
most of our other boxes?)
I was hoping this would make things faster... it does, but sadly only
by about 70ms, 5% of this file's test runtime.
It sure does make this file rather less action-at-a-distance, though,
as well as fixing some duplication.
If we make a practice on the Zulip server of always explicitly setting
the desired priority, then when an old server doesn't set the priority
we can reasonably have the bouncer make a guess.
That is, this allows a Zulip server to now set the `priority`; but if
it doesn't, we use upstream's default value, which has the same effect
as we've always previously had by not setting it at all.
But when this is deployed to the push notifications bouncer server, it
does allow another server to set priority when pushing notifications
through the bouncer.
If the caller has access to a Stream object, it is wasteful to
query a database for a stream by ID or name. In addition, not
having to go through stream names eliminates various classes of
possible bugs involved with re-fetching the Stream object by name.
If the caller has access to a Stream object, it is wasteful to
query a database for a stream by ID or name. In addition, not
having to go through stream names eliminates various classes of
possible bugs involved with getting a Stream object back.
The name for_stream_name is more appropriate here. The name
for_stream is more suitable for a function that takes in a Stream
object, which we're about to add.
Extracts out common tests so that future social-auth backends can
be tested without duplicating tests. I have been careful to not
change any testing logic.
Add all the stop words to page_params, reading from the
`zulip_english.stop` database, with caching to avoid loading the file
on every page load.
Part of #10592.
This causes changing the email_address_visibility field to actually
modify what user_profile.email values are generated for users, both on
user creation and afterwards as email addresses are edited.
The overall feature isn't yet complete, but this brings us pretty close.
We had disabled reference style links in bugdown, however,
we hadn't disabled them in marked. This commit rectifies
that and adds test cases for the same.
Fixes#11350.
We eliminated use of this function in outgoing_webhook.py in
bdc95b5d72.
Tweaked by tabbott to also eliminate code only used for that mock.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
This commit does the following three things:
1. Update stream model to accomodate rendered description.
2. Render and save the stream rendered description on update.
3. Render and save stream descriptions on creation.
Further, the stream's rendered description is also sent whenever the
stream's description is being sent.
This is preparatory work for eliminating the use of the
non-authoritative marked.js markdown parser for stream descriptions.
This adds a new API for sending basic analytics data (number of users,
number of messages sent) from a Zulip server to the Zulip Cloud
central analytics database, which will make it possible for servers to
elect to have their usage numbers counted in published stats on the
size of the Zulip ecosystem.
This is primarily a feature for onboarding, where an organization
administrator might send a bunch of random test messages as part of
joining, but then want a pristine organization when their users later
join.
But it can theoretically be used for other use cases (e.g. for
moderation or removing threads that are problematic in some way).
Tweaked by tabbott to handle corner cases with
is_history_public_to_subscribers.
Fixes#10912.
This replaces the current usage of stream names with stream ids.
This commit also removes the `traditional` attribute from the invite
form as now we are sending stream_ids as an argument; this was the
only place in the codebase we used traditional=true, and it's great to
have it removed.
This commit also contains the following auxiliary changes:
* Adds a custom exception, StreamWithIDDoesNotExist for when
a stream with a given ID does not exist because the error
message returned by StreamDoesNotExist only makes with stream
names, not IDs.
* Adds a new helper, get_stream_by_id_in_realm, which is similar
to get_user_profile_by_id_in_realm (introduced in #10391).
* Adds a helper, validate_stream_id_with_pm_notification, which
returns the Stream object associated with a given ID and also
handles PM notifications to the bot owner if the message was
sent by a bot and if the stream does not exist or has no
subscribers.
* Modifies the message sent by send_pm_if_empty_stream to
accommodate stream IDs.
Note that all of the above changes are required before check_message
can be modified to support stream IDs.
As part of Google+ being removed, they've eliminated support for the
/plus/v1/people/me endpoint. Replace it with the very similar
/oauth2/v3/userinfo endpoint.
This additional logic to prevent resizing is certain circumstances
(file size, dimensions) is necessary because the pillow gif handling
code seems to be rather flaky with regards to handling gif color
palletes, causing broken gifs after resizing. The workaround is to
only resize when absolutely necessary (e.g. because the file is larger
than 128x128 or 128KB).
Fixes#10351.
We add a new syntax which converts the messages like the following:
```
/poll Who do you support?
Nadal
- Djokovic
```
to a poll with the two names as options. The list syntax is optional
since anyone making a poll is likely to want to create a list anyway.
Earlier, our realm filters didn't render for languages that do not
use spaces (eg: Japanese) since we used to check for the presence
of an actual space character. This commit replaces that logic with
a complex scheme to detect word boundaries.
Also, we convert the RealmFilterPattern to subclass InlineProcessor
and make use of the new no-op feature in py-markdown 3.0.1 where we
can tell py-markdown that our pattern didn't find a match despite
the initial regex getting matched.
Fixes#9883.
This fixes an annoying bug where clicking to subscribe to a stream
would change the color shown in the "manage streams" UI immediately
after you click.
Fixes#11072.
This adds a setting under "Notification" section of
"Organization settings" tab, which enables Organization administrator to
control whether the missed message emails include the message content or
not.
Fixes: #11123.
Multiple delete message requests for the same message sometimes caused
a 500 error. This happened via the normal IntegrityError being thrown
by delete message/archiving code.
This was manually reproduced by adding latency in function
move_messages_to_archive() in retention.py and
delete_message_backend() in views.py. This addresses the problem by
adding code to handle the exception and throw JsonableError to convert
500 to 400 errors, with an automated test.
This a check on server side to verify whether the user sending request
to create stream where only admins can post is an admin or not; Raises
a JsonableError when the user is not the realm admin.
You can now pass in an info field with a value
like "out to lunch" to the /users/me/status,
and the server will include that in its outbound
events.
The semantics here are that both "away" and
"status_text" have to have defined values in order
to cause changes. You can omit the keys or
pass in None when values don't change.
The way you clear info is to pass the empty
string.
We also change page_params to have a dictionary
called "user_status" instead of a set of user
ids. This requires a few small changes on the
frontend. (We will add "status_text" support in
subsequent commits; the changes here just keep
the "away" feature working correctly.)
We now have single function that handle both away
and not-away.
This refactoring sets us up to piggyback "info" more
easily onto status updates.
The only thing that changes here is that we don't
delete database rows any more when users revoke
their away status. Instead we just set the status
to NORMAL.
When I was initially writing the tests to solve issue #10131 in PR
2 schema checkers as I modified the code to send the rendered_value
only when required.
When I was using just 1 schema checker shared between two code paths,
we needed _allow_only_listed_keys. But after shifting to 2 schema
checkers for the two different cases, we no longer needed that flag,
and it's better to remove it for a stronger check.
This reverts the temporary fix done in commit
46f4e58782 and replaced it with the fix that
non-admins should be able to see a dropdown to select a non-admin type of
invited user i.e. normal member or guest user.
`fakeldap` assumes every attribute to be a multi-value attribute
while making comparison in `_comapare_s()` and so while making
comparisons for password it gives a false positive. The result
of this was that it was possible to login in the dev environment
using LDAP using a substring of the password. For example, if the
LDAP password is `ldapuser1` even entering `u` would log you in.
On the backend, we extend the BlockQuoteProcessor's clean function that
just removes '>' from the start of each line to convert each mention to
have the silent mention syntax, before UserMentionPattern is invoked.
The frontend, however, has an edge case where if you are mentioned in
some message and you quote it while having mentioned yourself above
the quoted message, you wouldn't see the red highlight till we get the
final rendered message from the backend.
This is such a subtle glitch that it's likely not worth worrying about.
Fixes#8025.
These mentions look like regular mentions except they do not
trigger any notification for the person mentioned. These are
primarily to be used when you make a bot take an action and
the bot mentions you, or when you quote a message that mentions
you.
Fixes#11221.
Apparently, zoom's API will (sometimes?) return a 201 (not 200)
created in response to the API request to create a call. We fix this
by using the proper requests check for whether or not the request
failed.
This commit fixes an error in the logic for allowing admins to edit any
user's CPF (custom profile field) values. The logic allowing users to
edit their own CPF values is however sound. What happens is that of all
the CPF types, for "choice fields" as well as "URL" and "date fields",
when the value is reset/deleted/cleared by the admin in the Admin UI
(organization settings), the frontend would send a null (empty string)
value to the backend for that custom profile field (as this is, after
all, the new value in this case). This would then triggers the backend
validators to return an error message.
We fix this by using the method check_remove_custom_profile_field_value,
that both code paths (user editing their own CPFs and admin editing a
user's CPF) can call.
We make this change because setting up reminders in PM's didn't
play really well with our current infrastructure. Basically the
reminder messages from the bot can't appear in the same narrow as
that of a PM between two people and therefore we disable it.
Though we make an exception here where a person wants to set up
reminder for himself.
Previously, the subscription color attribute had a validator of
check_string, but this is insufficient. Hence this commit update the
validator used to check_color. Fixes#11268.