Commit Graph

108 Commits

Author SHA1 Message Date
Steve Howell 0cef7c9fd5 Refactor: Extract get_client_info_for_message_event().
This removes some clutter from process_message_event, and it
makes process_message_event() a bit easier to test.
2017-10-26 16:35:28 -07:00
Tim Abbott e98ca0714b notifications: Simplify how triggers are passed around.
This removes the utterly unnecessary `triggers` dict (which always was
a dict with exactly one value True) in favor of a single field,
'trigger'.

Inspired by Kunal Gupta's work in #6659.
2017-10-18 21:42:05 -07:00
Tim Abbott 513b6d624f notifications: Fix double-sending in missedmessage_hook.
While the missedmessage_hook logic originally did a reasonably good
job of avoiding double-sending notifications, there was a corner case
it didn't handle, namely a user who had been presence-idle when a
message was sent and became also event-queue-idle as well within the
next 10 minutes.  For those users, they got a notification at message
send time, and the missedmessage_hook would deliver it a second time.

We fix this by just checking the conveniently available push_notified
and email_notified variables that indicate whether the message already
had a notification triggered.

Fixes #7031.
2017-10-17 21:54:03 -07:00
derAnfaenger af699500b7 tests: Add option to call queue processor consumer.
This makes tests of queue processors more realistic,
by adding a parameter to `queue_json_publish` that
calls a queue's consumer function if accessed in a test.

Fixes part of #6542.
2017-10-16 23:20:13 -07:00
Steve Howell f0194f1821 Reset client descriptors at the start of event tests.
Fixes #6958
2017-10-12 10:27:16 -07:00
Tim Abbott 539ff20ec2 socket: Add logging for CSRF issue in logging code.
This should help make it possible to debug #6961.
2017-10-11 22:39:14 -07:00
Steve Howell 9ecf41980c Remove message.is_mentioned in message events.
This field would get overwritten with an improper value when
we looped over multiple clients, due to not making full copies
of the message dictionary.  This failure would be somewhat
random depending on how clients were ordered in the loop.

The only consumers of this field were the mobile app and the
apply-events-to-unread-counts logic.  Both of these will now
use `flags` instead.
2017-10-11 16:55:34 -07:00
Steve Howell 10a30bece1 Rename presence_idle_userids -> presence_idle_user_ids. 2017-10-07 12:16:45 -07:00
Tim Abbott f11832536c event_queue: Fix a now-incorrect mypy annotation.
It's not clear tagging things like this as an Iterable is a good idea,
when it's in fact definitely a list.
2017-10-05 23:11:16 -07:00
Tim Abbott fbdeb63bc9 tornado: Add debug logging for time consumed processing an event.
I've occasionally found this logging helpful.
2017-10-05 22:08:29 -07:00
Steve Howell aae0b2a826 Notify offline users about edited stream messages.
We now do push notifications and missed message emails
for offline users who are subscribed to the stream for
a message that has been edited, but we short circuit
the offline-notification logic for any user who presumably
would have already received a notification on the original
message.

This effectively boils down to sending notifications to newly
mentioned users.  The motivating use case here is that you
forget to mention somebody in a message, and then you edit
the message to mention the person.  If they are offline, they
will now get pushed notifications and missed message emails,
with some minor caveats.

We try to mostly use the same techniques here as the
send-message code path, and we share common code with the
send-message path once we get to the Tornado layer and call
maybe_enqueue_notifications.

The major places where we differ are in a function called
maybe_enqueue_notifications_for_message_update, and the top
of that function short circuits a bunch of cases where we
can mostly assume that the original message had an offline
notification.

We can expect a couple changes in the future:

    * Requirements may change here, and it might make sense
      to send offline notifications on the update side even
      in circumstances where the original message had a
      notification.

    * We may track more notifications in a DB model, which
      may simplify our short-circuit logic.

In the view/action layer, we already had two separate codepaths
for send-message and update-message, but this mostly echoes
what the send-message path does in terms of collecting data
about recipients.
2017-10-03 15:57:06 -07:00
Tim Abbott 3a286756ff tornado: Document missedmessage_hook in more detail. 2017-10-03 10:55:38 -07:00
Steve Howell e660428c21 Rename missed_message_userids to presence_idle_userids. 2017-10-02 15:19:28 -07:00
rht 426548097e zerver/tornado: Remove from __future__ import *. 2017-09-28 10:14:03 -07:00
rht 241e318eba zerver/tornado: remove `import six`. 2017-09-27 19:10:28 -07:00
rht b8aa92194c zerver/tornado: Remove print_function. 2017-09-27 18:05:45 -07:00
rht a7803cbf1b zerver/tornado: remove `import six`.
Tweaked by tabbott to remove the changes in
zerver/tornado/handlers.py, which are hard to test.
2017-09-27 17:07:08 -07:00
Tim Abbott 06e6b3abe1 event_queue: Fixed stream_push_notify code path in missedmessage_hook.
This finishes implementing the stream_push_notify feature, which
previously didn't work if you were not idle when the message was sent.
2017-09-27 15:57:48 -07:00
Tim Abbott 2b1ea1555c event_queue: Simplify API of maybe_enqueue_notifications. 2017-09-27 15:57:47 -07:00
Tim Abbott fa461c6a4f missedmessage_hook: Fix name of ClientDescription argument.
This was previously incorrectly named to suggest it was the EventQueue
object.
2017-09-27 15:57:47 -07:00
Tim Abbott d6732d0781 event_queue: Fix handling of private messages in missedmessage_hook.
Previously, we didn't correctly trigger email/push notifications for
PMs in these cases.
2017-09-27 15:57:47 -07:00
Tim Abbott 71fc8363b4 tests: Add a test suite for maybe_enqueue_notifications.
This ensures that as we expand the logic for under what circumstances
email and push notifications should be sent, we can be confident about
this code path always doing the right thing.
2017-09-27 15:55:29 -07:00
Tim Abbott 830d3e8d07 event_queue: Use maybe_enqueue_notifications in missedmessage_hook.
This fixes a problem introduced in the recent refactoring where
`triggers` would not be set correctly when a push or email
notification was triggered by missedmessage_hook.

Fixes #6612.
2017-09-27 15:55:29 -07:00
Tim Abbott 9749826e9c event_queue: Refactor code for checking if a message was mentioned.
Now, the two code paths do the same thing for this check.

It seems like there may be more work to do here, in that
wildcard_mentioned messages seem to not be eligible for sending
email/push notifications.  We probably want to add some logic there
for the user doing the mention to control whether or not it does.
2017-09-27 15:55:29 -07:00
Tim Abbott f68089fbe1 event_queue: Extract maybe_enqueue_notifications.
Conceptually, this code path should be shared between the
process_message handler and the garbage-collection hook.
2017-09-27 15:55:29 -07:00
rht 74fd3d9f31 zerver/tornado: Remove absolute_import. 2017-09-27 10:00:39 -07:00
Tim Abbott d06cb4e4fd event_queue: Make path forward for push/email notifications clearer.
This is a nonfunctional refactor, designed primarily to make it
simpler to extend this code path when we later add support for
controlling whether email notifications go out on stream messages.
2017-09-15 01:01:11 -07:00
Tim Abbott d0e8163f13 event_queue: Remove some unnecessary parenthesis. 2017-09-15 01:01:11 -07:00
Tim Abbott e085af3324 Fix stream_push_notify feature to not send emails.
Previously, due to a logic bug, this feature would also send email
notifications for all messages on the stream, which is definitely not
the intent.  The recent refactoring we just did makes the logic more
obvious.
2017-09-15 01:01:11 -07:00
Tim Abbott 22ea2a5858 event_queue: Separate email and push notification loops. 2017-09-15 01:01:11 -07:00
Tim Abbott 7fa0325fb5 event_queue: Refactor notified logic. 2017-09-15 01:01:11 -07:00
Greg Price c4b506998f tornado: Disable routine logging in dev.
This creates a lot of logging noise, and also causes confusion
for new contributors when something isn't working as they expect
and they aren't sure if this message is normal or an error.
2017-09-14 12:38:57 -07:00
Tim Abbott 5722237f59 push: Rename received_pm to private_message.
This is a clearer name for this now more broadly used interface.
2017-09-14 05:41:37 -07:00
Sarah 97571a203d push: Add new formatting for stream message push and add tests.
This should make the push notifications for messages to streams with
the new stream push notifications setting enabled make sense.
2017-09-14 05:41:37 -07:00
Sarah c3a8138f74 user_settings: Add push notifications for all stream messages.
Add setting to enable push notifications for all stream messages.
2017-09-14 05:41:37 -07:00
Steve Howell f5edeb01ae Calculate idle users more efficiently when sending messages.
Usually a small minority of users are eligible to receive missed
message emails or mobile notifications.

We now filter users first before hitting UserPresence to find idle
users.  We also simply check for the existence of recent activity
rather than borrowing the more complicated data structures that we
use for the buddy list.
2017-09-07 06:59:44 -07:00
Steve Howell 981f557422 Extract receiver_is_off_zulip().
We are splitting out this logic from the more complicated
UserPresence-related logic, so that we can simplify the latter.
2017-09-07 06:59:44 -07:00
Steve Howell 776bdc59db Avoid unnecessary steps in process_message_event().
There is no reason to compute receiver_is_idle() unless a user
is actually PM'ed or mentioned.
2017-09-07 06:59:44 -07:00
Greg Price b63e995e82 tornado: Fix a Redis-caused str/bytes bug in `status_inquiries`.
Because the Redis client returns exclusively bytes -- even for
hash keys -- even on Python 3, the test `'response' in status`
was always returning false, and the line that tries to decode
as JSON was never running, so we were passing `response`
through as a `bytes` object encoding some JSON.

I'm not sure what the impact of this bug was, and in particular
whether something downstream would have fudged it to make up for
this error.
2017-08-25 16:14:33 -07:00
Tim Abbott b8e7369dee mypy: Remove type: ignores not needed in Python 3. 2017-08-25 11:04:20 -07:00
Tim Abbott 80caa5c66f socket: Use get_user_profile_by_id.
This should have a slight performance benefit.
2017-08-24 23:16:42 -07:00
Tim Abbott c8f53e55db websocket_client: Use get_system_bot.
This messy code is used only for our websocket Nagios tests, so using
get_system_bot makes sense.
2017-08-24 23:16:18 -07:00
neiljp (Neil Pilgrim) b782db48e1 mypy: Remove superfluous older 'type: ignore' annotations. 2017-08-08 11:27:51 -07:00
neiljp (Neil Pilgrim) d9e36c8156 mypy: strict-optional fixes in zerver/tornado/socket.py. 2017-08-07 21:27:50 -07:00
Greg Price 0d2f7e8bcd errors: Eliminate redundant `json_response_to_error` calls. 2017-07-25 15:33:11 -07:00
Greg Price 709c3b50fc tornado: Use a machine-readable error code when an event queue is gone.
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.
2017-07-24 16:41:22 -07:00
Greg Price 45b718a2a2 tornado: Use the new structured JsonableError API for /json/events errors. 2017-07-24 16:41:22 -07:00
Greg Price 124d623d05 tornado/socket: Replace an ad-hoc error class with JsonableError.
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.
2017-07-24 16:41:22 -07:00
Greg Price 9faa44af60 JsonableError: Optionally carry error codes and structured data.
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.
2017-07-24 16:41:22 -07:00
Greg Price ff5013c619 JsonableError: Add types, and eliminate duck-typing.
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.
2017-07-24 16:41:22 -07:00