We are adding a new list of unread message ids grouped by
conversation to the queue registration result. This will allow
clients to show accurate unread badges without needing to load an
unbound number of historic messages.
Jason started this commit, and then Steve Howell finished it.
We only identify conversations using stream_id/user_id info;
we may need a subsequent version that includes things like
stream names and user emails/names for API clients that don't
have data structures to map ids -> attributes.
In anticipation of have all unread message ids available to the
web app in page_params (via a separate effort), we are simplifying
the /topics endpoint to no longer return unread counts.
Instead we have a list of tiny dictionaries with these fields:
name - name of the topic
max_id - max message id for the topic (aka most recent)
The items in the list are order by most-recent-topic-first.
This route is called only in `js/compose.js`, to handle autosubscribe.
That code doesn't check this "exists" field, because there's no need
-- the same information is already carried in whether the result was
success or failure. So just eliminate it.
This makes the logic here a little simpler. It also eliminates
another usage of the `data` parameter to `json_error`. I have half a
mind to eliminate that parameter, in favor of making `JsonableError`
subclasses whenever there's structured data to include, in particular
to get the benefits of typing. There are a couple of places where
that change isn't locally a clear win, but this is not one of them.
This error isn't saying that any kind of authentication or
authorization failed -- it's just a validation error like
any other validation error in the values the user is asking to
set. The thought of authentication comes into it only because
the setting happens to be *about* authentication.
Fix the error to look like the other validation errors around it,
rather than give a 403 HTTP status code and a "reason" field that
mimics the "reason" fields in `api_fetch_api_key`.
This allows us to reliably parse the error in code, rather than
attempt to parse the error text. Because the error text gets
translated into the user's language, this error-handling path
wasn't functioning at all for users using Zulip in any of the
seven non-English languages for which we had a translation for
this string.
Together with 709c3b50f which fixed a similar issue in a
different error-handling path, this fixes#5598.
Process the unicode emojis in twitter link previews and render them
properly. Before this we were not processing the unicode emojis in
twitter link previews and hence on the systems which don't have
fonts for displaying them they were rendered as blank boxes.
Fixes: #5427.
This commit renames list named `to_linkify` in twitter link processor
to `to_process` and adds a `type` field to each entry in it to
indicate the type of data represented by that particular entry.
Add test to check if the embedded bot service being used is in the
registry or not.
Add test to check if the bot being added to the registry has a valid
bot corresponding to it.
Move 'get_bot_handler' to 'zerver/lib/bot_lib.py' as it is an independent
function, not related to the 'EmbeddedBotWorker' class that it was
previously a part of.
This fixes the original issue that #5598 was the root cause of; when
the user returns to a Zulip browser tab after they've been idle past
the timeout (10 min, per IDLE_EVENT_QUEUE_TIMEOUT_SECS), we now
correctly reload the page even if they're using Zulip in German or
another non-English language where we have a translation for the
relevant error message.
The one purpose this exception was serving was to carry a message
in `msg`. We can do that with `JsonableError`, and as a bonus replace
a repetition of the familiar "'result': 'error', ..." JSON pattern
with a call to a common implementation.
Also wrap the error messages for translation -- we hadn't been doing
that, oops. Our linter notices that issue now that it's the familiar
JsonableError class.
There's one other potential change in behavior here: this
except-clause might now catch a JsonableError raised from some other
code. That seems like a bonus, if so; the handler isn't doing
anything actually specific to this code, and the more exceptions it
successfully turns into proper error responses to the client and lines
in the log, the better.
All JsonableError subclasses now have corresponding ErrorCode values
of their own, reducing the number of different patterns for using
the new JsonableError API.
This provides the main infrastructure for fixing #5598. From here,
it's a matter of on the one hand upgrading exception handlers -- the
many except-blocks in the codebase that look for JsonableError -- to
look beyond the string `msg` and pass on the machine-readable full
error information to their various downstream recipients, and on the
other hand adjusting places where we raise errors to take advantage
of this mechanism to give the errors structured details.
In an ideal future, I think all exception handlers that look (or
should look) for a JsonableError would use its contents in structured
form, never mentioning `msg`; but the majority of error sites might
continue to just instantiate JsonableError with a string message. The
latter is the simplest thing to do, and probably most error types will
never have code looking for them specifically.
Because the new API refactors the `to_json_error_msg` method which was
designed for subclasses to override, update the 4 subclasses that did
so to take full advantage of the new API instead.
This simplifies things for all codepaths not involving this feature.
Using this feature becomes slightly easier when you're already
defining a subclass, but now requires you to define a subclass.
Currently we use it just once out of >100 uses of JsonableError, and
that use already has a subclass, so this seems like a win.
With #5598 there will soon be an application-level error code
optionally associated with a `JsonableError`, so rename this
field to make clear that it specifically refers to an
HTTP status code.
Also take this opportunity to eliminate most of the places
that refer to it, which only do so to repeat the default value.
The file `zerver/lib/request.py` doesn't have type annotations
of its own; if they did, they would duplicate the annotations that
exist in its stub file `zerver/lib/request.pyi`. The latter exists
so that we can provide types for the highly dynamic `REQ` and
`has_request_variables`, which are beyond the type-checker's ken
to type-check, but we should minimize the scope of code that gets
that kind of treatment and `JsonableError` is not at all the sort of
code that needs it.
So move the definition of `JsonableError` into a file that does
get type-checked.
In doing so, the type-checker points out one issue already:
`__str__` should return a `str`, but we had it returning a `Text`,
which on Python 2 is not the same thing. Indeed, because the
message we pass to the `JsonableError` constructor is generally
translated, it may well be a Unicode string stuffed full of
non-ASCII characters. This is potentially a bit of a landmine.
But (a) it can only possibly matter in Python 2 which we intend to
be off before long, and (b) AFAIK it hasn't been biting us in
practice, so we've probably reasonably well worked around it where
it could matter. Leave it as is.
The whole thing is an error, so "message" is a more apt word for the
error message specifically. We abbreviate that as `msg` in the actual
HTTP responses and in the signatures of `json_error` and friends, so
do the same here.
In order to benefit from the modern conveniences of type-checking,
add concrete, non-Any types to the interface for JsonableError.
Relatedly, there's no need at this point to duck-type things at
the places where we receive a JsonableError and try to use it.
Simplify those by using straightforward standard typing.
This fixes some error message strings and skips converting request_data
into json. From now, conversion would be the responsibility of interface.
Also, base_url is now not passed into event structure.
Tornado reloads the app whenever there is a change in code. Due to this,
new connection is created to the client which also results in a new
channel. To avoid creating two channels for the queue in the RabbitMQ
broker we should close the old channel. Otherwise messages sent to the
queue will be distributed among these two channels in a round robin
scheme and we will end up losing one message since one of the channels
doesn't have an active consumer.
This commit closes the connection to the queue whenever Tornado reloads
the application using add_reload_hook().
Fixes#5824.
We do not need to test the exception message being logged in every
test case where an exception is raised by a webhook function.
Testing it once should be enough; this makes the tests less
verbose.
If an incoming payload contained a unicode character, it raised
a UnicodeEncodeError, because the message template was an str. Now,
the message template is unicode, so it can be formatted to include
unicode characters, should the incoming payloads contain any.
Splitting bot_lib.py file into 2 files led to unnecessary
redirection of the code workflow. For an embedded bot/service to
send a reply, it was being redirected 3 times.
First, the code flow comes to "EmbeddedBotHandler" class to send
reply, then it goes to the common function in "zulip_bots/lib.py",
then it would come back to "EmbeddedBotHandler". Later on, if we
create an abstract class, from where the bot work flow would
directly hit and then from there it is classified into
EmbeddedBotHandler or ExternalBotHandler and accordingly it would
get redirected.
Now, first the bot flow goes to it's handler class External or
Embedded (where we pass that this is External or Embedded bot as
parameter) and then goes to a common point and then comes back to
the same class.
Exception logging within api_key_only_webhook_view fails when
ValueError is raised if the request.body passed to ujson.loads
isn't valid JSON. In this case, we now just convert the payload
to a string and log that. This allows us to inspect JSON payloads
that aren't being decoded properly.
This fixes a performance issue that caused this migration to run for a
really long time.
It still takes about 1 minute to run with the 75K Subscription objects
we have on chat.zulip.org, but that's within the realm of acceptable.
This is required, since we just reorganized the python-zulip-api
repository into 3 packages.
A nice side effect is that we get to eliminate some now-unnecessary
code for editing sys.path.
Also adds Confirmation.type, and cleans up the rest of Confirmation to look
more like the model definitions in zerver.
In the migration, all existing confirmations adopt the type
USER_REGISTRATION, to be conservative. In a few commits, different
confirmation types will have different validity periods, and
USER_REGISTRATION will have the shortest default.
In most cases, we do have the data for which other user was
responsible for subscribing the target user to new streams.
The main case where we don't is when the user is created and gets the
default streams.
ScheduledJob was written for much more generality than it ended up being
used for. Currently it is used by send_future_email, and nothing
else. Tailoring the model to emails in particular will make it easier to do
things like selectively clear emails when people unsubscribe from particular
email types, or seamlessly handle using the same email on multiple realms.
Both the queue processor and ScheduledJob emails need to sometimes pass a
to_user_id and sometimes pass a to_email, and it's more convenient to just
have one function that they can call that can handle either.
Also removes the now redundant send_email_to_user.
Better to see "noreply@..." when replying to a message that you can't reply
to than to see "Zulip" (for email clients that hide the email address when
there is a display name).
This is required for test fixtures which contain `stream_id`. Prior
to python 3.3 hashes were not randomized but after a security fix
hash randomization was enabled by default in python 3.3 which made
iteration of dictionaries and sets completely unpredictable.
This new setting controls whether or not users are allowed to see the
edit history in a Zulip organization. It controls access through 2
key mechanisms:
* For long-ago edited messages, get_messages removes the edit history
content from messages it sends to clients.
* For newly edited messages, clients are responsible for checking the
setting and not saving the edit history data. Since the webapp was
the only client displaying it before this change, this just required
some changes in message_events.js.
Significantly modified by tabbott to fix some logic bugs and add a
test.
I pushed a bunch of commits that attempted to introduce
the concept of `client_message_id` into our server, as
part of cleaning up our codepaths related to messages you
sent (both for the locally echoed case and for the host
case).
When we deployed this, we had some strange failures involving
double-echoed messages and issues advancing the pointer that appeared
related to #5779. We didn't get to the bottom of exactly why the PR
caused havoc, but I decided there was a cleaner approach, anyway.
We are deprecating local_id/local_message_id on the Python server.
Instead of the server knowing about the client's implementation of
local id, with the message id = 9999.01 scheme, we just send the
server an opaque id to send back to us.
This commit changes the name from local_id -> client_message_id,
but it doesn't change the actual values passed yet.
The goal for client_key in future commits will be to:
* Have it for all messages, not just locally rendered messages
* Not have it overlap with server-side message ids.
The history behind local_id having numbers like 9999.01 is that
they are actually interim message ids and the numerical value is
used for rendering the message list when we do client-side rendering.
Prior to this commit, 7 megabytes of images (through 253 individual requests)
were heavily slowing down the initial load. With this commit, we load only the
logos (60 or so images).
Documentation and images for the individual integration sub-pages is requested
separately using the /integrations/doc/ endpoint, which returns HTML.
Use bool_change if the user_display setting property_type is bool, so that no additional code needs to be added to test_events for new boolean user display settings.
Brock Whittaker
Tim Abbott
Steve Howell
Jason Michalski
Abhijeet Kaur
Harshit Bansal
Eeshan Garg
digi0ps
James Rowan
Vishnu Ks
Rishi Gupta
Greg Price
Elliott Jin
vaibhav
Jack Zhang
Umair Khan
Sarah
Joshua Pan
Aditya Bansal
neiljp (Neil Pilgrim)
Durga Akhil Mundroy
Vaida Plankyte
Cory Lynch