These docstrings hadn't been properly updated in years, and bad an
awkward mix of a bad version of the user-facing documentation and
details that are no longer true (e.g. references to "Voyager").
(One important detail is that we have real documentation for this
system now).
Closes#13736.
zerver.lib.server_initialization.create_internal has precisely the same
code (you can copy-and-paste swap them, with one level of indentation
adjustment, without generating any diff) so they can be trivially
deduplicated.
zerver.lib.server_initialization.create_users has precisely the same
code (you can copy-and-paste swap them without generating any diff) so
they can be trivially deduplicated.
This doesn't change any behavior, the purpose of this is to make the
function identical to what we have in server_initialization.py so that
it can be deduplicated in follow-up commits.
In addition to making our schema check stricter, it also makes it
possible for us to extend check_events_dict to do additional
validation that's only expected for the full event object.
Now that we have the type situation of having anchor support passing a
string, this is a much more natural way to implement
use_first_unread_anchor.
We still support the old interface to avoid breaking compatibility
with legacy versions of the mobile apps.
A wart that has long been present inin Zulip's get_messages API is how
to request "the latest messages" in the API. Previously, the
recommendation was basically to pass anchor=10000000000000000 (for an
appropriately huge number). An accident of the server's implementation
meant that specific number of 0s was actually important to avoid a
buggy (or at least wasteful) value of found_newest=False if the query
had specified num_after=0 (since we didn't check).
This was the cause of the mobile issue
https://github.com/zulip/zulip-mobile/issues/3654.
The solution is to allow passing a special value of anchor='newest',
basically a special string-type value that the server can interpret as
meaning the user precisely just wants the most recent messages. We
also add an analogous anchor='oldest' or similar to avoid folks
needing to write a somewhat ugly anchor=0 for fetching the very first
messages.
We may want to also replace the use_first_unread_anchor argument to be
a "first_unread" value for the anchor parameter.
While it's not always ideal to make a value have a variable type like
this, in this case it seems like a really clean way to express the
idea of what the user is asking for in the API.
This fixes a bug where that clients using the legacy approach of a
"very large anchor" value with the intent to only get the most recent
messages would only get found_newest=True if they used the specific
value LARGER_THAN_MAX_MESSAGE_ID. Now any value at least that large
will work.
In upcoming commits, we plan to replace this with passing the string
"last", but it seems worth removing the buggy "special value" behavior
while we're touching this code.
This is required for the upcoming type behavior of the "anchor"
parameter.
This change is the minimal work required to have our OpenAPI code not
fail when checking a union-type value of this form. We'll likely want
to, in the future, do something nicer, but it'd require more extensive
infrastructure for parsing of OpenAPI data that it's worth with our
current approach (we may want to switch to using a library).
The proximal issue here is that in upcoming commits, we're going to
change the type of the `anchor` field in `get_messages_backend` to
support passing either an integer or a string.
Many of our tests using POSTRequestMock currently define a query
object that uses integer values for the integer fields we're going to
pass into it, e.g. {'num_after': 0}. That is the correct type for
that field in the Zulip API, before HTTP encoding turns it into a
string. However, because POSTRequestMock didn't use HTTP encoding at
all (which will convert the 0 into a '0'), it ended up passing an
integer to a function that can't possible receive one as an argument.
Ideally, we'd just get rid of POSTRequestMock, since it's a hack, and
just do real HTTP requests instead.
But since it's used in a lot of places making doing so somewhat
impractical, we can get past this issue by just making POSTRequestMock
convert integers to strings.
Using logging.info() rather than logger.info() meant that our
zulip.soft_deactivation logger configuration (which, in particular,
included not logging to the console) was not active on this log line,
resulting in the `manage.py soft_deactivate_users` cron job sending
emails every time it ran.
Fixes#13750.
In Django 2.0, request.user.is_authenticated stops supporting
`.is_authenticated()` and becomes just a property. In 1.11, it's a
CallableProperty (i.e. can be used either way), and we already use it
as a property in several other places, so we should just switch to
using it consistently now to get it off of our Django 2.x migration
checklist.
Previously, we didn't track opening and closing fences separately,
with led to bugs like not parsing a list that was immediately after
a quoted fence; we treated each ``` as a new fence.
This commit rewrites the function to maintain a stack of currently
open fences. If any of the parent fences is a code fence, we do not
insert a new line before a list.
We also add some test cases specifically to test this behavior with
complexly nested lists.
Fixes#13745.
Adding invited users to the notifications stream unconditionally isn't
a correct behaviour for guest users, where the previous behavior of
including the notifications stream no longer makes sense. Therefore,
while inviting a new user, the notifications stream is listed along
with other streams with a message "recieves notifications for new
streams" in order to distinguish it from other streams.
Fixes#13645.
The desktop otp flow (to be added in next commits) will want to generate
one-time tokens for the app that will allow it to obtain an
authenticated session. log_into_subdomain will be the endpoint to pass
the one-time token to. Currently it uses signed data as its input
"tokens", which is not compatible with the otp flow, which requires
simpler (and fixed-length) token. Thus the correct scheme to use is to
store the authenticated data in redis and return a token tied to the
data, which should be passed to the log_into_subdomain endpoint.
In this commit, we replace the "pass signed data around" scheme with the
redis scheme, because there's no point having both.
This extracts a function for computing show_invites and
show_add_streams, for better readability and testability.
This commit was substantially cleaned up by tabbott.
This legacy cross-realm bot hasn't been used in several years, as far
as I know. If we wanted to re-introduce it, I'd want to implement it
as an embedded bot using those common APIs, rather than the totally
custom hacky code used for it that involves unnecessary queue workers
and similar details.
Fixes#13533.
authenticate_remote_user already takes care of calling the authenticate
with the dummy backend. Also, return_data is not used and catching
DoesNotExist exception is not needed, as the dummy backend just returns
None if user isn't found.
In other places where we set request._email, we set it to the
delivery_email, as that's more informative in orgs with hidden email
settings, where user.email will be useless.
This updates our error handling of invalid Slack API tokens (and other
networking error handling) to mostly make sense:
* A token that doesn't start with `xoxp-` gives an extended error early.
* An AssertionError for the codebase is correctly declared as such.
* We check for token shape errors before querying the Slack API.
We could still do useful work to raise custom exception classes here.
Thanks to @stavrospat for raising this issue.
responses is an module analogous to httpretty for mocking external
URLs, with a very similar interface (potentially cleaner in that it
makes use of context managers).
The most important (in the moment) problem with httpretty is that it
breaks the ability to use redis in parts of code where httpretty is
enabled. From more research, the module in general has tendency to
have various troublesome bugs with breaking URLs that it shouldn't be
affecting, caused by it working at the socket interface layer. While
those issues could be fixed, responses seems to be less buggy (based
on both third-party reports like ckan/ckan#4755 and our own experience
in removing workarounds for bugs in httpretty) and is more actively
maintained.
If an email is sent with the .prefer-html option, but it has no html
body, it's better to fall back to plaintext content instead of treating
it as a user error.