Commit Graph

177 Commits

Author SHA1 Message Date
Steve Howell 5e2a32c936 tests: Use users in send_*_message.
This commit mostly makes our tests less
noisy, since emails are no longer an important
detail of sending messages (they're not even
really used in the API).

It also sets us up to have more scrutiny
on delivery_email/email in the future
for things that actually matter.  (This is
a prep commit for something along those
lines, kind of hard to explain the full
plan.)
2020-03-07 18:30:13 -08:00
Tim Abbott 8ff5d8ca89 test_classes: Clean up API_KEYS cache.
Since the intent of our testing code was clearly to clear this cache
for every test, there's no reason for it to be a module-level global.

This allows us to remove an unnecessary import from test_runner.py,
which in combination with DEFAULT_REALM's definition was causing us to
run models code before running migrations inside test-backend.

(That bug, in turn, caused test-backend's check for whether migrations
needs to be run to happen sadly after trying to access a Realm,
trigger a test-backend crash if the Realm model had changed since the
last provision).
2020-01-16 13:07:26 -08:00
Mateusz Mandera 7d0444f903 push_notifs: Improve handling of errors when talking to the bouncer.
We use the plumbing introduced in a previous commit, to now raise
PushNotificationBouncerRetryLaterError in send_to_push_bouncer in case
of issues with talking to the bouncer server. That's a better way of
dealing with the errors than the previous approach of returning a
"failed" boolean, which generally wasn't checked in the code anyway and
did nothing.
The PushNotificationBouncerRetryLaterError exception will be nicely
handled by queue processors to retry sending again, and due to being a
JsonableError, it will also communicate the error to API users.
2019-12-04 09:58:22 -08:00
Mateusz Mandera 20b30e1503 push_notifs: Set up plumbing for retrying in case of bouncer error.
We add PushNotificationBouncerRetryLaterError as an exception to signal
an error occurred when trying to communicate with the bouncer and it
should be retried. We use JsonableError as the base class, because this
signal will need to work in two roles:
1. When the push notification was being issued by the queue worker
PushNotificationsWorker, it will signal to the worker to requeue the
event and try again later.
2. The exception will also possibly be raised (this will be added in the
next commit) on codepaths coming from a request to an API endpoint (for
example to add a token, to users/me/apns_device_token). In that case,
it'll be needed to provide a good error to the API user - and basing
this exception on JsonableError will allow that.
2019-12-04 09:58:22 -08:00
Mateusz Mandera 717e90dfeb test_push_notifications: Adjust mocking of requests.request.
requests.request is called in zerver/lib/remote_server.py, so these
mocks should be mocking it there, not in zerver.lib.push_notifications.
2019-12-04 09:58:22 -08:00
Mateusz Mandera ae8656e2c1 test_get_apns_client: Do cleanup in a finally: block. 2019-12-04 09:58:21 -08:00
Tim Abbott 6407d0b1f9 push_notifications: Clear PushDeviceToken on API key change.
This includes adding a new endpoint to the push notification bouncer
interface, and code to call it appropriately after resetting a user's
personal API key.

When we add support for a user having multiple API keys, we may need
to add an additional key here to support removing keys associated with
just one client.
2019-11-19 15:37:43 -08:00
Tim Abbott ddd1a0eb00 actions: Convert do_delete_messages to take a Realm.
The function only used the user's realm anyway, so this is a cleaner
API.

This should also make it more convenient to permanently delete
messages manually, since one doesn't have to fetch a random user in
the realm in order to delete a message using the management shell.

No functional change.
2019-11-12 12:20:31 -08:00
Mateusz Mandera bbf2474bd0 tests: setUp overrides should call super().setUp().
MigrationsTestCase is intentionally omitted from this, since migrations
tests are different in their nature and so whatever setUp()
ZulipTestCase may do in the future, MigrationsTestCase may not
necessarily want to replicate.
2019-10-19 17:27:01 -07:00
Rishi Gupta 360cd7f147 remote data: Send RealmAuditLog data. 2019-10-08 17:27:29 -07:00
Rishi Gupta 48dc1d1128 remote data: Refactor remote_server_post_analytics to be more generic.
One small change in behavior is that this creates an array with all the
row_objects at once, rather than creating them 1000 at a time.

That should be fine, given that the client batches these in units of
10000 anyway, and so we're just creating 10K rows of a relatively
small data structure in Python code here.
2019-10-06 16:55:41 -07:00
Mateusz Mandera dbe508bb91 models: Migration of Message.pub_date to date_sent, part 2.
Fixes #1727.

With the server down, apply migrations 0245 and 0246. 0246 will remove
the pub_date column, so it's essential that the previous migrations
ran correctly to copy data before running this.
2019-10-05 19:01:34 -07:00
Wyatt Hoodes dbaf6ac7e7 test_push_notifications: Remove fixtures print statement. 2019-09-13 11:54:14 -07:00
Tim Abbott 70c513a640 analytics: Fix logging for errors connecting to push bouncer.
There's no reason for this to be a category of error that emails the
server administrator, since there's a good chance that fixing it will
need to be done in the Zulip codebase, not administrator action.
2019-09-02 18:47:10 -07:00
Tim Abbott 0ed0bb6828 messages: Add email/push notifications for wildcard mentions.
Historically, Zulip's implementation of wildcard mentions never
triggered either email or push notifications, instead being limited to
desktop notifications and the "mentions" counter.

We fix this just by plumbing the "wildcard_mentioned" flag through our
system.

Implements much of
https://github.com/zulip/zulip/issues/6040#issuecomment-510157264.
We're also now ready to seriously work on #3750.
2019-08-26 14:39:53 -07:00
Mateusz Mandera 52d4583987 test_push_notifs: Eliminate hard-coded user ids. 2019-08-21 21:28:09 -07:00
Rohitt Vashishtha 400d0367dc tests: Improve logging for fixture tests in push_notifications. 2019-08-21 16:34:40 -07:00
okmanl 2a1305de9f lint: Add a rule to avoid msgid as a Python variable name.
This is for consistency with our usual patterns, see #12995.  We will
need a similar commit for JavaScript to complete #12995.
2019-08-17 12:47:13 -07:00
Anders Kaseorg becef760bf cleanup: Delete leading newlines.
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>
2019-08-06 23:29:11 -07:00
Mateusz Mandera 8c0e5c5fde test_push_notifs: Prepare for moving system bots to zulipinternal. 2019-07-24 16:44:16 -07:00
Mateusz Mandera 11862e5ce0 test_classes: Use subdomain kwarg in api_* functions instead of realm.
It's more appropriate for the kwarg to be named subdomain. We also
update the functions, so that this kwarg is used in all of them
consistently.
2019-07-23 15:05:39 -07:00
Tim Abbott bcc6949461 zilencer: Add better error handling for IntegrityError.
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.
2019-05-20 17:53:43 -07:00
Tim Abbott cc421d4415 tests: Fix bad use of mock local variable name.
This ended up masking the mock module.
2019-04-25 15:28:10 -07:00
Vishwesh Jainkuniya c007b9ea4a notifcations: Remove `user` from the payload.
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.
2019-04-22 14:50:04 -07:00
Vishwesh Jainkuniya 447a517e6f notifications: Add `user_id` in the GCM & APNS payload.
This makes it easy to uniquely identify the user account associated
with a notification by, for example, the (realm_uri, user_id) pair.

This helps improve notifications in the mobile apps.
See https://github.com/zulip/zulip-mobile/pull/3407#discussion_r266196616

Fixes #11961.
2019-04-22 14:49:03 -07:00
Greg Price 9869153ae8 push notif: Send a batch of message IDs in one `remove` payload.
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.
2019-02-26 16:41:54 -08:00
Greg Price 28ff9670de push notif: Push `gcm_options` logic inside "payload" helpers.
These are logically closely related.
2019-02-26 16:41:54 -08:00
Greg Price 8f26e12c85 push notif: Clarify get_*_payload, and factor another out.
This is a pure refactor; adding docstrings, making some names more
explicit, and pulling out one small helper.
2019-02-26 16:41:54 -08:00
Greg Price 69ded8b1b4 push notif: Drop irrelevant fields in `remove` payloads.
These fields don't make much sense in this case; and the client
doesn't look at them and never has.  Stop including them.
2019-02-26 16:41:54 -08:00
Anders Kaseorg 649235cfec python: Remove unused imports.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
2019-02-22 16:54:36 -08:00
Greg Price 0213aa0b16 push notif: Don't forget to clear "active" flag on sending to bouncer.
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.
2019-02-14 14:52:53 -08:00
Greg Price 84e0b68b16 push notif: Rename `gcm` to less confusing `gcm_client`.
This opens up space in the namespace for, say, the library
module itself.
2019-02-13 13:57:57 -08:00
Greg Price f4478aad54 push notif: Unconditionally remove notifications on message read.
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.
2019-02-13 13:13:45 -08:00
Tim Abbott d6140b684f notifications: Don't send error emails on bouncer 500s.
Since the individual server administrator can't do anything about
these, this should not trigger an email notification.
2019-02-11 21:19:28 -08:00
Greg Price a6c2c16666 push notif tests: Clean up how we set up device token fixtures.
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.
2019-02-08 15:18:12 -08:00
Greg Price ccc1f3cd85 push notif tests: Stop importing module as bizarre name `apn`. 2019-02-08 15:18:12 -08:00
Greg Price c372ceb7a2 push notif tests: Use mock.patch instead of `apn.gcm = ...`. 2019-02-08 15:18:12 -08:00
Greg Price e289c5835d push notif tests: Use mock.patch instead of `apn.gcm = None`.
This is what it's for -- and it cleans up after itself, too.
2019-02-08 15:18:12 -08:00
Greg Price ff12072f18 push notif: Skip four layers of setup for some pure unit tests.
This saves about 50ms, for just four test cases.
2019-02-08 15:18:12 -08:00
Greg Price 1e11e929ec push notif: Guess a GCM `priority` on behalf of old servers. 2019-02-08 15:18:12 -08:00
Greg Price a293aeee23 push notif: Explicitly set GCM priority `normal` for remove.
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.
2019-02-08 15:18:12 -08:00
Greg Price ffabebd7f3 push notif: Set GCM priority `high` for real notifications.
This is the payoff of this branch!  Fixes zulip/zulip-mobile#3185.
2019-02-08 15:18:12 -08:00
Greg Price 699bf262ca push notif: Test GCM options parsing more comprehensively.
The payoff from making these into real unit tests.
2019-02-08 15:18:12 -08:00
Greg Price 02ba302009 push notif: Simplify tests for parsing GCM options.
Huzzah for unit tests!
2019-02-08 15:18:12 -08:00
Greg Price f1cb9be79c push notif: Consolidate and simplify GCM tests. 2019-02-08 15:18:12 -08:00
Greg Price 674b254b65 push notif: Accept GCM `priority` option.
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.
2019-02-08 09:42:59 -08:00
Greg Price 49fd2e65de push notif: Add GCM options to bouncer API; empty for now.
The first use case for this will be setting `priority`,
coming up shortly.
2019-02-08 09:40:43 -08:00
Anders Kaseorg 3127fb4dbd zerver/tests: Remove unused imports.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
2019-02-02 17:43:03 -08:00
Anders Kaseorg b80a095196 python: Stop importing PushNotificationBouncerException from wrong file.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
2019-02-02 17:09:01 -08:00
Tim Abbott 022c8beaf5 analytics: Add APIs for submitting analytics to another server.
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.
2019-02-01 22:03:52 -08:00