This commit asserts that parse_user_agent never returns None. The
RegEx will match any string, so that `match` is never None. This
brings test coverage of lib/user_agent.py to 100%. Changes were also
made in test/test_decorators.py and views/compatibility.py to reflect
that parse_user_agent cannot return None.
Improves: #7089.
Fixes: #8779.
Details in comment. Together with a few previous commits, this should
completely eliminate sending error mail to admins when the RabbitMQ
server is simply restarted and comes back up normally.
There were two instances of `ensure_stream` being called and assigned to
a variable with the variable not being used elsewhere. pyflakes picked
up on this (where it didn't in the previous version likely due to tuple
unpacking), so the the variable assignment has been replaced with a call
to `ensure_stream`.
Issue #2088 asked for a wrapper to be created for
`create_stream_if_needed` (called `ensure_stream`) for the 25 times that
`create_stream_if_needed` is called and ignores whether the stream was
created. This commit replaces relevant occurences of
`create_stream_if_needed` with `ensure_stream`, including imports.
The changes weren't significant enough to add any tests or do any
additional manual testing.
The refactoring intended to make the API easier to use in most cases.
The majority of uses of `create_stream_if_needed` ignored the second
parameter.
Fixes: #2088.
This commit migrates realm emoji to be addressed by their `id` rather
than their name. This fixes a long standing issue which was causing
an error on uploading an emoji with same name as a deactivated realm
emoji.
Fixes: #6977.
Because the base class's __init__ calls `_connect`, when we set the
value after that call has already returned, our new value only takes
effect if the first connection fails and we have to reconnect.
Make it take effect from the beginning.
This parameter isn't used anywhere. A good thing, because if it were,
the code would immediately raise an exception -- `self._on_open_cbs`
hasn't been initialized yet when we first call `_connect`, from the
base class's `__init__`.
So, just cut it. If we later need something like this, it's easy to
add a working version then.
We disabled the original "colorized HTML edit-history" feature way
back in 2013 in c51056ff8e.
That original feature involved showing what had been edited inline in
message bodies, so one could easily see what had been changed.
That old feature has since been replaced with the "view edit history"
menu option, and we're unlikely to ever want the old feature back.
So, we can just remove its code. There's a few supporting variables
that were created to help implement this; we can clean those up and
simplify the `update_message` code now that this feature is fully
removed.
This applies only on a server open for anyone to create a realm.
Moreover, if the server admins have granted any given realm a
max_invites greater than the default, that realm is exempt too.
This fixes an unpleasant regression in
f5edeb01ae, where we stopped correctly
filtering users who have an open browser session that's idle. These
users are tagged as "UserPresence.IDLE" with an current timestamp in
the database, and should be treated as idle for presence purposes.
As a result, if you had an open Zulip browser session, you incorrectly
wouldn't get missed-message emails for PMs and mentions before this fix.
This fixes a regression in 93678e89cd
and a4979410f9, where the webhooks using
authenticated_rest_api_view were migrated to a new model that didn't
include setting a custom Client string for the webhook.
When restoring these webhooks' client strings, we also fix places
where the client string was not capitalized the same was as the
product's name.
This changes the followup_day2 emails delay from one day later to two days
later if it is getting delivered on any working days(i.e. Mon - Fri).
For Thursday it is compromised to next day as it would be too late to
postponed to Monday and for Friday it should be Monday.
At last actually, emails should send one hour before the above calculated so
that user can catch them when they are dealing with these kinds of stuff.
Fixes: #7078.
These changes are in one commit, since the previous typing of check_url
does not match the centralized strict definition (object/Any vs Text),
actually already used elsewhere in validator.py, and also had a different
API.
check_url is updated here to match the API of the other check_* functions,
ie. val is an object (not Text) & returns Optional[str]. It also now checks
the value is text explicitly at run-time, which was only type-checked
previously. Tests are updated accordingly.
Currently, when other private stream subscriber add realm admin to
stream, new copy private stream is created in realm admin's streams.
Which resulted in error, cause there are two similar stream element
in stream settings.
If new subscriber is added to private stream, we first send them
stream `create` event, cause private stream are not visible until
user don't get subscribed at least once. But realm admins can now
always access private stream, so when realm admin is subscribed to
stream, realm admin get stream `create` event even if stream already
exist in on realm admin client side.
Fix this by extracting realm admins from stream `create` event on
`add` subscription operation and sending private stream `create`
event to all realm admins on stream creation operation.
Fixes#8695
These are the straightforward ones.
Note that there is a line in zerver.lib.test_classes.build_webhook_url
that lost test coverage. That's because most of our tests test using
stream messages so the webhook URLs being tested always have a query
parameter. So the line that accounts for there being no query
parameters never gets called, which is fine, but we should still
keep it.
This commit adds a generic function called check_send_webhook_message
that does the following:
* If a stream is specified in the webhook URL, it sends a stream
message, otherwise sends a PM to the owner of the bot.
* In the case of a stream message, if a custom topic is specified
in the webhook URL, it uses that topic as the subject of the
stream message.
Also, note that we need not test this anywhere except for the
helloworld webhook. Since helloworld is our default example for
webhooks, it is here to stay and it made sense that tests for a
generic function such as check_send_webhook_message be tested
with an actual generic webhook!
Fixes#8607.
We solved the problem the TODO raised by using a different type
annotation syntax, and I'm not sure whether that refactor would
actually improve the code.
The previous system would crash with some files (because for some
reason the comment count was 1 but there was no "initial comment") and
also the file comment and file name were sorta redundant.
The 'make_new_dir' bool value was used to create a new directory
every time True is passed. Now that avatars and uploads directory
are being created seperately, we don't need this anymore.
The domain name is being set in the helper function
'slack_workspace_to_realm', but it should be set in the main function
'do_convert_data', as we need it in other child functions of
'do_convert_data'.
This code was originally written when we were using the old South
system, and hasn't been used in a few years. It probably doesn't
work, and thus only serves to clutter the codebase.
Originally was going to centralize this in zerver/lib/request.pyi, but this
file is not visible at run-time, being only a stub. The matching request.py
file seemed inappropriate, as it doesn't actually use ViewFuncT.
Namely, annotate as best as possible, and add notes to indicate preference,
if QuerySet develops generic typing.
Note that the return values of functions with annotations changed in this
commit are used elsewhere as QuerySets, so the Sequence[T] approach used
for some functions in models.py is not applicable.
This will allow realm admins to access subscribers of unsubscribed
private stream. This is a preparatory commit for letting realm admins
remove those users.
This will allow realm admins to update the names and descriptions of
private streams even if they are not subscribed, which fixes the buggy
behavior that previously nobody could(!).
Apparently, we did essentially all the work to support showing full
topic history to newly subscribed users from a data flow perspective,
but didn't actually enable this feature by having the topic history
endpoint grant access to historical topics. This fixes that gap.
I'm not altogether happy with how the code and tests read for this
feature; the code itself has more duplication than I'd like, and the
tests do too, but it works.
This commit refactors the bugdown to perform a lookup only on active
realm emojis. This was needed because once we migrate realm emojis
to be addressed by `id` rather than name, it will be costly to
perform a lookup on all the realm emojis.
If user has disabled message content in missed email notifications,
we shouldn't send any informations about missed messages i.e. sender,
stream, message text, etc. to email servers.
We are already hiding this informations in email templates, but we
shoudln't expose any information about message content if user
has disabled this setting.
If new private stream is created by realm admin without realm admin
subscribed to it, then it doesn't automatically add created stream to
realm admin's stream list. We have to reload the browser to get newly
created stream in stream list. Cause private stream creation event is
only sent to the subscribed users to private stream, so even if realm
admin is acting user, they don't get creation event.
We should send private stream creation event to realm admin users along
with subscribed user to stream, as realm admins can access unsubscribed
private streams.
Tweaked by tabbott to fix various typos and clean up the code.
Till now, we had been storing realm emoji's name in emoji code field
in reactions' model. This commit migrates it to store realm emoji's id.
It is a part of effort to migrate realm emojis to be referenced by their
id and not by name.
"incorrect" here means rejected by a bot's validate_config() method.
A common scenario for this is validating API keys before the bot is
created. If validate_config() fails, the bot will not be created.
Adds realm_bot delete event. On bot ownership change, add event is
sent to the bot_owner(if not admin) and delete event to the
previous bot owner(if not admin). For admin, update event is sent.
The messages were first being read and passed to the helper
functions channel wise.
This function makes a list of all the messages in the all the channels
beforehand which would be used to pass in the helper functions.
This field has been unused by clients for some time, and isn't great
for our public archive feature plans (where we'll not want to be
including email addresses in messages).
Add `translate_emoticons` to `prop_types` and `expected_keys`.
Furthermore, create a emoji-translating Markdown inline pattern.
Also use a JavaScript version of `translate_emoticons` and then use
this function during Markdown previews and as a preprocessor. This
is only needed for previews, because usually emoticon translation
happens on the backend after sending.
Add tests for emoticon translation, a settings UI, and a /help/ page
as well.
Tweaked by tabbott to fix various test failurse as well as how this
handles whitespace, requiring emoticons to not have adjacent
characters.
Fixes#1768.
This sets up a new test class with a simple
test, mostly for increasing coverage. The class
should in the future be extended to properly
verify the handle_feedback() logic.
Previously, when a user updated the config data of an
embedded bot, only the updated fields were dispatched
back to the client. Dispatching all config data fields
makes the client updating code less brittle.
During a slack import, we don't have medium-size avatars already
available in the export data set (and possibly also with a normal
import/export?). The medium size avatar can be created by the
'ensure_medium_avatar_image' function, which checks if the medium
image exists, and if it doesn't, it creates the image.
This commit was substantially edited by tabbott to get rid of an
undefined variable bug, avoid initializing the upload backend classes
in a loop, and add some TODO notes on things that could be improved
later.
slack avatar urls have the format:
'https://ca.slack-edge.com/<team_id>-<user_id>-<avatar_hash>-<size>'
For any url of this form, if the user hasn't uploaded an image,
Slack uses default gravatar, but we don't have a way of knowing if Slack
has used the uploaded image or the custom gravatar
eg: https://ca.slack-edge.com/T5YFFM2QY-U6006P1CN-gd41c3c33cbe-512.
Hence, avatar_source should be mapped to 'U'.
models.py should only contain thin wrapper functions. Furthermore,
this move allows us to remove the circular imports. The two moved
functions are interdependent and are thus moved in one commit.
The check for the channel ('general' and 'random') must be added before
'build_defaultstream' function is called and then the id is incremented.
Otherwise, the id appended at the end of second defaultstream object, which would be
greater than the total number of defaultstream objects would crash at
'defaultstream_id_list[defaultstream_id]' which is a paramater of 'build_defaultstream'.
Added tests to prevent the same.
This commit:
* Removes the unnecessary screenshot.
* Reorders the instructions and combines them in to 4 steps.
* Improves the contents of the webhook-url-with-bot-email-indented.md
macro and makes it more consistent with create-bot-construct-url.md.
* Sets the recommended stream name to "commits", since that's what
the webhook function for Beanstalk expects in
zerver/webhooks/beanstalk/view.py. This allows us to use the
create-stream.md macro.
Creating the very first organization administrator user and
subscribing them to streams before any messages were sent resulted in
RealmAuditLog entries being created with a `event_last_message_id` of
None, because that's the maximum ID in the empty set.
We correct this by fixing the incorrectly created RealmAuditLog
entries, both for new servers and also fixing old broken entries on
existing servers.
This fixes an issue where if a user setup a Zulip server with just the
organization administrator, and then forgot about it (so that the
initial user became soft-deactivated), trying to sign in 3 weeks later
would throw an exception.
This fixes the issue reported here:
https://chat.zulip.org/#narrow/stream/9-issues/subject/500.20error.20on.20login/near/511981
The total number of stream objects are allocated to
total_users. They should be allocated to the total_channels.
This passed the tests as the total number of users in the test
where greater than the total number of channels.
The Markdown extension that lives inside
zerver/lib/bugdown/api_code_example.py previously used ujson.
ujson's `dumps` function doesn't accept a `separators` argument,
which means we have no control over how the JSON is pretty-printed.
This resulted in JSON fixtures with no spaces after the colon, which
looks unnecessarily convoluted.
So now, we use the built-in `json` module to get around this.
For further reading, this issue
<https://github.com/esnme/ultrajson/issues/82> opened on ujson's
repo explains why they are reluctant to support such formatting
due to performance considerations.
This commit adds a test for the sample fixture for when an invalid
stream name is passed to a query that expects a valid stream name
as an argument. This is the case with almost all of our queries
documented under the sidebar heading "Streams".
EDIT: Actually, I was wrong. This payload is highly specific to
get-stream-id, so it shouldn't be a part of common-error-payloads
at all.
In templates/zerver/api/delete-queue.md, we have a sample fixture
for when the queue_id passed to client.deregister_queue is not
valid or the event queue in question has already been deleted.
This commit tests that fixture.
Note that this error payload is specific to client.deregister_queue.
In templates/zerver/api/create-user.md, we have a sample fixture
for when a client attempts to create a user with the same email
as an existing user. This commit adds a test for that fixture.
Note that this error payload is specific to client.create_user
and this error payload isn't generated anywhere else.
In templates/zerver/api/add-subscriptions, we have a sample fixture
for when the user being subscribed is already subscribed to a
stream. This commit tests that fixture against a running server.
This commit adds tests for the fixture for when a user is not
authorized (perhaps because the query requires the use of admin
privileges) for a particular query.
In templates/zerver/api/update-message.md, we have a sample fixture
for when a zulip.Client does not have the permission to update/edit
a particular message. This commit adds a test for that fixture.
Also, tools/test-api now also uses a non-admin client for this test,
which might come in handy in the future.
This commit adds tests for the sample fixture for when a required
request argument is missing. Also, it moves the sample fixture
to common-error-payloads.md, since this is an error payload that
is common to most requests (except the ones that don't take any
arguments).
In templates/zerver/api/private-message.md, we have a sample fixture
for when the email address of the PM's recipient is invalid. This
commit makes sure that fixture is tested against a running server.
In templates/zerver/api/stream-message.md, we have a sample fixture
for when the target stream does not exist. This commit adds a test
for that sample fixture.
This changes the missed-message logic to use the new encoding
scheme for stream url fragments that prefixes them with
"{stream_id}-".
For simplicity sake, we just get a Stream object to pass to
the helper functions, and we only call get_display_recipient()
now for the HUDDLE case. (We were calling it wastefully before
for the PM case.)
This commit prefixes stream names in urls with stream ids,
so that the urls don't break when we rename streams.
strean name: foo bar.com%
before: #narrow/stream/foo.20bar.2Ecom.25
after: #narrow/stream/20-foo-bar.2Ecom.25
For new realms, everything is simple under the new scheme, since
we just parse out the stream id every time to figure out where
to narrow.
For old realms, any old URLs will still work under the new scheme,
assuming the stream hasn't been renamed (and of course old urls
wouldn't have survived stream renaming in the first place). The one
exception is the hopefully rare case of a stream name starting with
something like "99-" and colliding with another stream whose id is 99.
The way that we enocde the stream name portion of the URL is kind
of unimportant now, since we really only look at the stream id, but
we still want a safe encoding of the name that is mostly human
readable, so we now convert spaces to dashes in the stream name. Also,
we try to ensure more code on both sides (frontend and backend) calls
common functions to do the encoding.
Fixes#4713
We use the command
'select nextval('sequence') from generate_series(1, increment_number)'
which returns a list of allocated values for the ids.
This list is used to assign ids to the to be converted objects.
This adds button under "Organization profile" settings, which
deactivates the organization and sends an "event" to all the
active user and log out them.
Fixes: #8212.
This enforces `**` around all the mentions including "at-all" and
"at-everyone" mentions. Hence this makes `@all` and `@everyone`
invalid mentions, resulting into proper syntax for these mentions as
`@**all**` and `@**everyone**` respectively.
Note from tabbott: This removes an old feature/syntax, which made
sense back when @Tim was also a way to mention a user with Tim as
their first name. Given how nice typeahead is now, the user part of
the feature was removed a while ago; this should have gone at the same
time.
Fixes: #8143.
This commit modifies the Markdown extension in bugdown/api_code_examples.py
to support rendering code examples in multiple languages by specifying
the language like so:
{generate_code_example(python)|doc.md|example}
This makes us one step closer towards adding support for testable
JavaScript code examples.
This commit simply adds a comment in zerver/lib/api_test_helpers.py
explaining why it isn't worth the effort to explicitly test the
code example in api/get-events-from-queue.md.
This may be helpful for some API clients, since it avoids them needed
to do somewhat messy post-processing on the results (the data was
always available via scanning for the first unread message in the result).
Fixes#6244.
External bots may call bot_handler.quit() when
they wish to terminate, e.g. due to a misconfiguration.
Currently, embedded bots ignore calls to quit(), even
though they signal a problem. This commit does the first
step in handling quit() calls by logging a warning.
This is based on usage in bulk_change_user_names.py, and that
the RealmAuditLog acting_user field is Optional[UserProfile].
This could be more meaningfully changed in future, perhaps to
indicate that the command was run by a specific zulip user.
When the answer is False, this will allow the mobile app to show a
warning that push notifications will not work and the server admin
should set them up.
Based partly on Kunal's PR #7810. Provides the necessary backend API
for zulip/zulip-mobile#1507.
I got distracted, came back later to a successful test run in my
terminal, and thought I remembered finishing the change and just
kicking off a final test run to check.
In fact, there was an `assert False` right in the normal case for
production, and I just hadn't finished a test for that path. (m.-)
Definitely the most grateful I've been for our coverage checks,
which highlighted this for me.
Remove the `assert False`, and also finish writing the test it was
there to help me write. Those lines are covered now.
This is a wrapper over lru_cache function. It adds following features on
top of lru_cache:
* It will not cache result of functions with unhashable arguments.
* It will clear cache whenever zerver.lib.cache.KEY_PREFIX changes.
The fresh imported data shows that the users emails are not included
in the data. However, the data received from the older method of slack
(which is using legacy tokens) contains the email data of the users.
If some bug in Bugdown results in a rendered message content that is
bigger than twice the message size, we now just throw an exception
from Bugdown. This is considerably better than the old behavior,
which might result in an enormous message being placed in the database
(potentially, bigger than the 1MB limit to store in memcached), which
would in turn result in tragic consequences.
This fixes#8322, in that it prevents the super bad outcome seen there
(where basically Zulip became unusable for everyone on the stream
where the message is posted). Now, the failure mode is just the
message failing to send. Still not ideal (and requires further work
on the URL embed feature), but not a minor problem, not a major one.
This commit adds tests (and thus, an extra code example) for
unsubscribing another user from a particular stream by passing in
the `principals` argument to client.remove_subscriptions. The
ability to pass in `principals` was added in the latest release
of the zulip API PyPI package.
We now have a separate page for common error payloads, for example,
the payload for when the client's API key is invalid. All error
payloads that are presented on this page will be tested similarly
to our other non-error sample fixtures.
This commit adds support for passing in an argument to the macro
"call" to explicitly specify a fixture to render, like so:
{generate_code_example|doc_name|fixture(stream_message_with_args)}
api/get-all-streams - make use of the api_code_example Markdown extension's
feature of recursively extracting multiple code examples from a single
test method.
To generate a code exammple,
{generate_code_example|<api_doc_md>|example} sounds better and
more intuitive than,
{generate_code_example|<api_doc_md>|method}
This code duplicated the code in setup_realm_internal_bots, with some
added logic to avoid trying to create the same bot twice. That logic
was buggy so that it would never work at all -- it subtracted a set of
UserProfile objects from a set of email strings -- so it looked like
the command might blow up when run after the users already existed.
In fact, the buggy logic wasn't necessary, because the work the
command does after it is idempotent -- in particular `create_users`,
within its subroutine `bulk_create_users`, already filters out users
that already exist. So just cut the buggy stuff out, deduplicate the
rest with `setup_realm_internal_bots`, and document that invariant on
the latter.
While we're here, in the common case bail early without doing any
per-realm work in Python, since we're running this on every upgrade.
Users having only account in one realm will not be distracted by realm
name in subject lines of every email. Users who have multiple
accounts in realms can turn this setting on and receive a
corresponding realm name in email's subject.
Tweaked by tabbott to rebase and address a few small issues.
Fixes#5489.
For messages where the entire rendered body is a message_inline_image
object, we actually don't display any text and just display the
image. These messages may have links to images which might or might
not be internal to Zulip but in both cases there is a chance of this
links being broken when accessed by an email server like Gmail that
doesn't possess the recipient user's cookies.
We don't want to have ugly looking broken images displayed in email
notifications. So we patch this by inserting a replacement for the
`message_inline_image` block in which we essentially replace the
content with the textual link.
Edited for clarity by tabbott.
This is to break the bigger functions into smaller
ones and hence, makes unit testing easier.
Also includes renaming the stream's topic name to 'from slack'.
This fixes an issue where the user's own avatar was being sent down
the wire as None. We could have fixed it, as in #8265, by adding code
in the webapp and mobile apps to compute medium-size gravatar URLs as
well, but that would be messy, and there's little benefit to that
complexity (saving at most 2 URLs from the payload).
Fixes#8253.
This commit uses the Markdown extension defined in
zerver/lib/bugdown/api_generate_examples to generate the
example fixture and code example, so that both are tested
in zerver/lib/api_test_helpers.
This commit uses the Markdown extension defined in
zerver/lib/bugdown/api_generate_examples to generate the
example fixture and code example, so that both are tested
in zerver/lib/api_test_helpers.
Some of our code examples can only be run with administrator
credentials (such as create-user). Thus, the Markdown extension
for generating code examples should have an option to include
the lines that recommend using an admin zuliprc instead of a
non-admin one.
This commit uses the Markdown extension defined in
zerver/lib/bugdown/api_generate_examples to generate the
example fixture and code example, so that both are tested
in zerver/lib/api_test_helpers.
This commit uses the Markdown extension defined in
zerver/lib/bugdown/api_generate_examples to generate the
example fixture and code example, so that both are tested
in tools/lib/api_tests.
This commit uses the Markdown extension defined in
zerver/lib/bugdown/api_generate_examples to generate the
example fixture and code example, so that both are tested
in tools/lib/api_tests.
This commit uses the Markdown extension defined in
zerver/lib/bugdown/api_generate_examples to generate the
example fixture and code example, so that both are tested in
tools/lib/api_tests.
This commit uses the Markdown extension defined in
zerver/lib/bugdown/api_generate_examples to generate the example
fixture and code example, so that both are tested in
tools/lib/api_tests.
This commit uses the Markdown extension defined in
zerver/lib/bugdown/api_generate_examples to generate the example
fixture and code example, so that both are tested in
tools/lib/api_tests.
Now that the Markdown extension defined in
zerver/lib/bugdown/api_generate_examples depended on code in the
tools/lib/* directory, it caused the production tests to fail since
the tools/ directory wouldn't exist in a production environment.
This commit adds a Markdown extension that allows the following
syntax,
{generate_code_example|<md_file_name>|<fixture or method>}
to generate code examples and fixtures found in tools/lib/api_tests.py
and templates/zerver/api/fixtures.json, respectively.
We'll replace this primarily with per-realm quotas (plus the simple
per-file limit of settings.MAX_FILE_UPLOAD_SIZE, 25 MiB by default).
We do want per-user quotas too, but they'll need some more management
apparatus around them so an admin has a practical way to set them
differently for different users. And the error handling in this
existing code is rather confused. Just clear this feature out
entirely for now; then we'll build the per-realm version more cleanly,
and then we can later add back per-realm quotas modelled after that.
The migration to actually remove the field is in a subsequent commit.
Based in part on work by Vishnu Ks (hackerkid).
Jack Weatherilt
Greg Price
YJDave
Harshit Bansal
Tim Abbott
Rhea Parekh
Nikhil Kumar Mishra
Shubham Dhama
neiljp (Neil Pilgrim)
Eeshan Garg
Vishnu Ks
Ricky
Robert Hönig
Shubham Padia
Aditya Bansal
Yago González
Marco Burstein
Archana BS
Umair Khan
rht
Alena Volkova
Aastha Gupta
nyan-salmon
Steve Howell
Roman Godov
Privisus
Rohitt Vashishtha