Commit Graph

12896 Commits

Author SHA1 Message Date
Anders Kaseorg 70aa9903b9 list_realms: Convert percent formatting to "".format.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-01-26 13:27:22 -08:00
Anders Kaseorg c36a66cc1b redis_utils: Convert percent formatting to f-strings.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-01-26 13:27:22 -08:00
Mateusz Mandera 1432067959 dependencies: Upgrade to Django 3.1.
https://docs.djangoproject.com/en/3.1/releases/3.1/

- django.contrib.postgres.fields.JSONField is deprecated and should be
  replaced with models.JSONField
-  The internals of the implementation in the postgresql backend have
   changed a bit in
   f48f671223
   and thus we need to make an ugly tweak in test_runner.
- app_directories.Loader.get_dirs() now returns a list of PosixPath so
  we need to make a small tweak in TwoFactorLoader for that (PosixPath
  is not iterable)

Fixes #16010.
2021-01-26 10:20:00 -08:00
Mateusz Mandera bf9e5e52ce dependencies: Upgrade to Django 3.0.
Adjustments made due to changes in Django 3.0:
(https://docs.djangoproject.com/en/3.0/releases/3.0/)

- test_signup: INTERNAL_RESET_URL_TOKEN was moved to
  PasswordResetConfirmView.reset_url_token
- test_message_fetch:
  "add_never_cache_headers() and never_cache() now add the private
  directive to Cache-Control headers."
- "django.utils.html.escape() now uses html.escape() to escape HTML.
  This converts ' to &#x27; instead of the previous equivalent decimal
  code &#39;." - this requires adjusting the expected decimal code
  in some of the string fixtures in tests.
2021-01-26 10:20:00 -08:00
Aman Agrawal 961d1d0a68 community_topic_edit: Increase time limit to 3 days.
24hrs is a small time in an asynchronous conversation. Increased
time limit of topic editing for non-admins to 3 days.
2021-01-25 14:55:33 -08:00
Anders Kaseorg a873a68cc7 eslint: Fix unicorn/no-new-array.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
2021-01-25 14:53:19 -08:00
akshatdalton 5f8a10124e url preview: Update Zulip User-Agent.
This commit updates the Zulip User-Agent to
'Mozilla/5.0 (compatible; ZulipURLPreview/{version}; +{external_host})'
as the older User-Agent was rendering Markdown YouTube titles as
'YouTube - YouTube'.

Fixes #16970.
2021-01-25 14:24:48 -08:00
Alex Vandiver 3381fad258 registration: Stop enqueueing to the signups queue.
c2526844e9 removed the `signups` queue
worker, and the command-line tool that enqueues to it -- but not the
automated process that enqueues during signups itself.

Remove the signup, since it is no longer in use.
2021-01-24 09:42:55 -08:00
Suyash Vardhan Mathur f4cf5166bb api docs: Display data type of parameters in API documentation.
Previously, the data type of parameters wasn't displayed in the API
Documentation, even though that OpenAPI data is carefully validated
against the implementation.  Here we add a recursive function to
render the data types visibly in the API documentation.

This only covers the request parameters; we'll want to do something
similar for response parameters in a follow-up PR.

Fixes part of #15967.
2021-01-21 15:56:07 -08:00
Steve Howell 1498b2ef69 apply_event: Fix broken deepcopy attempt for subs.
When we were getting an apply_event call for
a subscription/add event, we were trying not to
mutate the event itself, but this clumsy code
was still mutating the actual event:

    # Avoid letting 'subscribers' entries end up in the list
    for i, sub in enumerate(event['subscriptions']):
        event['subscriptions'][i] = \
            copy.deepcopy(event['subscriptions'][i])
        del event['subscriptions'][i]['subscribers']

This is only a theoretical bug.

The only person who receives a subscription/add
event is the current user.

And it wouldn't have affected the current user,
since the apply_event was correctly updating the
state, and we wouldn't actually deliver the event
to the client (because the whole point of apply_event
is to prevent us from having to piggyback the
super-recent events on to our payload or put
them into the event queue and possibly race).

The new code just cleanly makes a copy of each
sub, if necessary, as we add them to state["subscriptions"].

And I updated the event schemas to reflect that
subscribers is always present in subscription/add
event.

Long term we should probably avoid sending subscribers
on this event when the clients don't set something
like include_subscribers.  That's a fairly complicated
fix that involves passing in flags to ClientDescriptor.
Alternatively, we could just say that our policy is
that we never send subscribers there, but we instead
use peer_add events.  See issue #17089 for more
details.
2021-01-21 15:04:07 -08:00
Steve Howell c6acde9c63 apply_event: Use stream_ids, not names, for add/remove.
It's always cleaner to work in id space.  It probably
would have required a perfect storm to have broken
the existing code, but using ids is obviously more
robust in theory, and just as simple.
2021-01-21 15:04:07 -08:00
Steve Howell 0519f2d2b9 minor: Move include_subscribers guards in apply_event.
This sets us up for a cleaner diff in an
upcoming commit.
2021-01-21 15:04:07 -08:00
Steve Howell 3fa595ef85 minor: Clean up args for apply_event.
We now require keywords, so that there is no
pitfall for mixing up boolean parameters.
Positional parameters are basically evil
when you have a bunch of bools.

I also make user_profile the first argument.

Finally, the code is more diff-friendly.
2021-01-21 15:04:07 -08:00
Steve Howell e42baf9e13 minor: Clean up args for apply_events.
I eliminate the defaults, since the existing code
was already specificying values for most things.

I move all the booleans to the bottom for both
parameters and arguments.

I require explicit keywords for everything but
user_profile (which is now first).

And, finally, I format the code in a more
diff-friendly manner.
2021-01-21 15:04:07 -08:00
Steve Howell f2586d2f9b refactor: Introduce SubscriptionInfo dataclass.
We use this as the return type for
gather_subscriptions_helper and
get_web_public_subs, instead of tuples.
2021-01-21 15:04:07 -08:00
Steve Howell 768117f0ff refactor: Unify include_subscribers logic. 2021-01-21 15:04:07 -08:00
Steve Howell e735ce3f01 refactor: Move subscribers logic up to caller.
The gather_subscriptions_helper function now updates
subscribers instead of delegating.
2021-01-21 15:04:07 -08:00
Steve Howell d9740045a5 refactor: Eliminate checks in build_stream_dict_for_sub.
We eliminate some redundant checks.

We also consistently provide a `subscribers` field
in our stream data with `[]`, even if our users
can't access subscribers.  We therefore bump
the API version and tweak the docs.  (See further
down for a detailed justification of the change.)

Even though it is sometimes fine to have redundant code
that is defensive in nature, some upcoming changes are gonna
move subscriber-related logic out of build_stream_dict_for_sub
for certain codepaths as part of our effort to streamline
the payload for subscribers within page_params.

So we can't rely on the code that I removed here
inside of build_stream_dict_for_sub.

Anyway, it makes more sense to do these checks explicitly
in the validate function.

The code in build_stream_dict_for_sub was almost effectively
a noop, since the validation function was already preventing
us from getting subscriber info.  The only difference it
made was sometimes converting `[]` to `None`, and then
subsequently omitting the subscribers field.

Neither ZT nor the webapp make any distinction between
`[]` or <missing key> for the `subscribers` data in
`page_params`.

The webapp has had this code for a long time (and now
equivalent code elsewhere in this PR):

    if (!Object.prototype.hasOwnProperty.call(sub, "subscribers")) {
        sub.subscribers = new LazySet([]);
    }

The webapp calculates access based on booleans, anyway:

    sub.can_access_subscribers =
        page_params.is_admin || sub.subscribed ||
        (!page_params.is_guest && !sub.invite_only);

And ZT would choke if `subscribers` were missing, except that
it never gets to the relevant code due to other checks:

    def get_other_subscribers_in_stream(<snip>):
        assert stream_id is not None or stream_name is not None

        if stream_id:
            assert self.is_user_subscribed_to_stream(stream_id)

            return [sub
                    for sub in self.stream_dict[stream_id]['subscribers']
                    if sub != self.user_id]
        else:
            return [sub
                    for _, stream in self.stream_dict.items()
                    for sub in stream['subscribers']
                    if stream['name'] == stream_name
                    if sub != self.user_id]

You could make a semantic argument that we should prefer
<missing key> to `[]` when subscribers aren't even available, but
we have precedent from the way that `bulk_get_subscriber_user_ids`
has traditionally populated its result:

    result: Dict[int, List[int]] =
        {stream["id"]: [] for stream in stream_dicts}

If we changed `stream_dicts` to `target_stream_dicts` we
would faciliate a move toward `None`, but it would just cause
headaches for other server code as well as the frontends
(which, to reiterate, already prefer the empty array
for convenience).
2021-01-21 15:04:07 -08:00
Steve Howell 40b0c36d21 minor: Update comment for guest subscription access.
As my comment indicates, I would prefer to handle
this explicitly by raising JsonableError in an
else statement here, but it's not a big deal.

This function can probably be simplified with a
bit of work, mostly on the testing side to make
sure we are covering all edge cases, but that
is out of the scope of my current PR.
2021-01-21 15:04:07 -08:00
Mateusz Mandera fcc8debc3a users: Use realm.host in dummy user addresses without email visibility.
By moving the relevant logic from realm.get_bot_domain to
get_fake_email_domain we will make realm.host be used (if possible) for
dummy user addresses. That is, instead of user11@zulipchat.com, the
address will become user11@subdomain.zulipchat.com.
2021-01-21 13:04:38 -08:00
Mateusz Mandera 2283aa8a62 bots: Use realm.host for bot email domain if possible.
With the change in d70e1bcdb7,
bots get email like bot@zulip.com with EXTERNAL_HOST="zulip.com",
rather than bot@subdomain.zulip.com, which was the old format. That's
not desirable, so with this commit, realm.host will be used when
possible and only falling back to FAKE_EMAIL_DOMAIN if needed.
2021-01-21 13:04:38 -08:00
Mateusz Mandera b15dd9147d create_user: Remove redundant argument of get_display_email_address. 2021-01-21 13:04:38 -08:00
Steve Howell c693ae8982 event tests: Cover do_update_user_status better.
We often send only one field (away or status_text)
to be updated.

So we have to make our schema support optional
keys.

As a result of the more flexible schema, we no
longer need to exempt the node fixtures from
our schema checks.
2021-01-20 13:17:32 -08:00
Steve Howell 36b1794c1d user_status: Fix bug with resetting away status.
The fix is pretty simple here--if the client
doesn't send an away status, then don't change
it.

I improved the tests to cover this case.

Fixes #17071
2021-01-20 13:59:35 -05:00
Mateusz Mandera a9242d6dfc retention: Eliminate redundant recipient JOIN from cross-realm query.
Since recipient_id (id of the PERSONAL Recipient of the user) was
denormalized into the UserProfile model, this query can be simplified by
getting rid of the zerver_recipient JOIN.
2021-01-18 21:40:37 -08:00
Mateusz Mandera e3be6db73a retention: Eliminate redundant userprofile JOIN from cross-realm query. 2021-01-18 21:40:37 -08:00
Tim Abbott 5a02b33f2e digest: Add a large block comment on correctness. 2021-01-17 11:37:59 -08:00
Steve Howell 1040fb7219 email digests: Remove handle_digest_email shim.
The previous commit made it so we only call the
shim in tests, so now we completely remove it.
2021-01-17 11:28:30 -08:00
Steve Howell bfa0bdf3d6 email digests: Process users in chunks of 30.
This should make the queue empty more quickly,
because we do bulk queries to prevent database
hops.
2021-01-17 11:28:30 -08:00
Steve Howell e0b451730a email digests: Extract get_new_streams.
This makes us more efficient when handling
multiple users.  We don't have to keep
sending the same two queries to the database.

Note that as part of this we eliminated
a failure mode for the obscure population
of users from whom both `user.is_guest` and
`user.can_access_public_streams()` returns
False.  We know this would have only affected
Zephyr users (by looking at the code), and
we know we don't actually process Zephyr
users for email digests (or else we would
have raised exceptions in the old code).
2021-01-17 11:28:30 -08:00
Steve Howell 23de94504f email digests: Query streams for messages up front.
This should save us many hops to the database when
we process users in bulk.
2021-01-17 11:28:30 -08:00
Steve Howell 3662bf2dcb minor: Rename stream_map -> user_stream_map. 2021-01-17 11:28:30 -08:00
Steve Howell 11c93aced5 minor: Rename user_profile -> user and avoid shadowing. 2021-01-17 11:28:30 -08:00
Steve Howell f8bbb7fea9 email digests: Use select_related("realm").
We mostly need realm_id, but when we go to build
message lists, we need realm.uri.

We could probably be more aggresive about using
`only` here, but for now I am just trying to
reduce hops to the database.
2021-01-17 11:28:29 -08:00
Steve Howell bb56f0ec0e minor: Move get_stream_map to module level.
This is a pure code move.
2021-01-17 11:28:29 -08:00
Steve Howell 52e2d5a733 email digests: Avoid long_term_idle check.
We want to exclude users with recent subscription
activity from emails, regardless of whether
the long_term_idle flag is set.
2021-01-17 11:28:29 -08:00
Steve Howell 162b372b93 email digests: Do one query for recent streams.
This is another way to limit hops to the database
when we process users in bulk.
2021-01-17 11:28:29 -08:00
Alex Vandiver c2526844e9 worker: Remove SignupWorker and friends.
ZULIP_FRIENDS_LIST_ID and MAILCHIMP_API_KEY are not currently used in
production.

This removes the unused 'signups' queue and worker.
2021-01-17 11:16:35 -08:00
Alex Vandiver 01658e39a9 sentry: Verify version is supported, first.
Raven SDK does not send a `title` field.
2021-01-17 11:15:40 -08:00
Alex Vandiver d688e18de2 errors: Remove references to "deployment", use "host".
The `deployment` key was only set in `do_report_error`, which is now
only used in one codepath (the queue worker).  The logging handlers on
staging call notify_server_error directly, which omits the
`deployment` key.

Remove the odd one-of key, and instead simply do dispatch in
`do_report_error`.
2021-01-17 11:08:12 -08:00
Mateusz Mandera 3623681d30 message_edit: Don't rely on .recipient_id change not affecting recipient.
The codepath for moving a topic changes the message.recipient_id to the
id of the new recipient, but later, in update_messages_for_topic_edit,
it uses message.recipient when querying for messages with the matching
topic in the *old* stream (because those are the other messages that
need to be moved). This is a bug which happens to work fine, because in
Django 2, if message.recipient gets fetched first and then
message.recipient_id is mutated, message.recipient will not be altered
and thus will retain the outdated, previously fetched value.

In Django 3 changing .recipient_id causes .recipient to be updated to
the new Recipient objects, which is the Recipient of the *new* stream.
That will cause the bug to manifest.

This is a bugfix preparing for the upgrade to Django 3.
2021-01-17 10:39:46 -08:00
Mateusz Mandera f76202dd59 django3: Save language preference in a cookie rather than the session.
Support for saving it in the session is dropped in django3, the cookie
is the mechanism that needs to be used. The relevant i18n code doesn't
have access to the response objects and thus needs to delegate setting
the cookie to LocaleMiddleware.

Fixes the LocaleMiddleware point of #16030.
2021-01-17 10:38:58 -08:00
Steve Howell 04b6108e71 minor: Require keywords for verify_action. 2021-01-17 12:31:04 -05:00
Steve Howell 3df507be73 refactor: Clean up args for fetch_initial_state_data.
We now require explicit keywords for all arguments
to fetch_initial_state_data except user_profile.

We provide reasonable defaults to keep the test
code concise.
2021-01-17 12:31:04 -05:00
Alex Vandiver 08d716c741 registration: Re-use the redirect_to_email_login_url helper.
In the case of reusing a registration link, reuse the
redirect_to_email_login_url helper.  This does have the side effect of
now showing a "you've already registered" note, which did not happen
previously, but that seems probably for the best, since the user did
just click a "register" link.
2021-01-13 11:28:32 -08:00
Alex Vandiver ad3d25103b registration: Pre-fill the email when redirecting to login.
ecfafc05c0 shifted to using a different paramter name to hint that
the user had previously signed up -- and in so doing also stopped
pre-filling the "email" box.  Also send along the email box, to save
users time.
2021-01-13 11:28:32 -08:00
Tushar912 c60f48c889 registration: Move "already in realm" check outside of validation.
Checking for `validate_email_not_already_in_realm` again (after the
form already did so), but only in the case that the form fails to
validate, means that we may be spending time pushing totally invalid
emails to the DB to check.  In the case of emails containing nulls,
this can even trigger a 500 error from PostgreSQL.

Stop calling `validate_email_not_already_in_realm` in the form
validation. The form is currently only used in two places -- in
`accounts_home` and in `maybe_send_to_registration`.  The latter is
only called if the address is known to not currently have an account,
so checking in there is unnecessary; and in the former case, we wish
different behaviour (the redirect) than just validation failure, which
is all the validator can do.

Fixes #17015.

Co-authored-by: Alex Vandiver <alexmv@zulip.com>
2021-01-13 11:28:32 -08:00
Tushar912 410bb8ad89 imports: Add better checking for subdomains.
Add a `--allow-reserved-subdomain` flag which allows creation of
reserved keyword domains.  This also always enforces that the domain
is not in use, which was removed in 0258d7d.

Fixes #16924.
2021-01-12 17:54:01 -08:00
sushant52 6f0e8a9888 auth: Handle the case of invalid subdomain at various points.
Fixes #16770.
2021-01-11 22:29:50 -08:00
Siddharth Asthana 6c888977a6 change_subdomain: Create a deactivated realm on updating subdomain.
When changing the subdomain of a realm, create a deactivated realm with
the old subdomain of the realm, and set its deactivated_redirect to the
new subdomain.
Doing this will help us to do the following:
- When a user visits the old subdomain of a realm, we can tell the user
that the realm has been moved.
- During the registration process, we can assure that the old subdomain
of the realm is not used to create a new realm.

If the subdomain is changed multiple times, the deactivated_redirect
fields of all the deactivated realms are updated to point to the new
uri.
2021-01-07 14:15:22 -08:00