Add support for updating messages after they've been received.

Currently the interface for editing messages is limited to a
command-line API tool; it's great for testing with e.g.:

./api/examples/edit-message --message=348135 --content="test $(date +%s)" --site=http://localhost:9991 --subject="test"

The next commit will add a user interface for actually doing the editing.

(imported from commit bdd408cec2946f31c2292e44f724f96ed5938791)

api/examples/edit-message

@@ -0,0 +1,52 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+# Copyright © 2012 Humbug, Inc.
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+# THE SOFTWARE.
+
+import sys
+from os import path
+import optparse
+
+usage = """edit-message [options] --message=<msg_id> --content=<new content> --subject=<new subject>
+
+Edits a message
+
+Example: edit-message --message="348135" --subject="my subject" --message="test message"
+"""
+parser = optparse.OptionParser(usage=usage)
+parser.add_option('--subject', default="")
+parser.add_option('--message', default="")
+parser.add_option('--site',   default='https://humbughq.com')
+parser.add_option('--content',   default="")
+(options, args) = parser.parse_args()
+
+sys.path.insert(0, path.join(path.dirname(__file__), '..'))
+import humbug
+client = humbug.Client(site=options.site)
+
+message_data = {
+    "message_id": options.message,
+}
+if options.subject != "":
+    message_data["subject"] = options.subject
+if options.content != "":
+    message_data["content"] = options.content
+print client.update_message(message_data)

api/humbug/__init__.py

@@ -272,6 +272,7 @@ def _mk_events(event_types=None):
     return dict(event_types=event_types)
 
 Client._register('send_message', url='messages', make_request=(lambda request: request))
+Client._register('update_message', method='PATCH', url='messages', make_request=(lambda request: request))
 Client._register('get_messages', method='GET', url='messages/latest', longpolling=True)
 Client._register('get_events', url='events', method='GET', longpolling=True, make_request=(lambda **kwargs: kwargs))
 Client._register('register', make_request=_mk_events)

humbug/urls.py

@@ -112,6 +112,7 @@ urlpatterns += patterns('zephyr.views',
     url(r'^json/create_bot$',               'json_create_bot'),
     url(r'^json/get_bots$',                 'json_get_bots'),
     url(r'^json/update_onboarding_steps$',  'json_update_onboarding_steps'),
+    url(r'^json/update_message$',           'json_update_message'),
 
     # These are json format views used by the API.  They require an API key.
     url(r'^api/v1/get_profile$',            'api_get_profile'),
@@ -132,6 +133,7 @@ urlpatterns += patterns('zephyr.views',
     # GET returns messages, possibly filtered, POST sends a message
     url(r'^api/v1/messages$', 'rest_dispatch',
             {'GET':  'get_old_messages_backend',
+             'PATCH': 'update_message_backend',
              'POST': 'send_message_backend'}),
     url(r'^api/v1/streams$', 'rest_dispatch',
             {'GET':  'get_public_streams_backend'}),

zephyr/lib/actions.py

@@ -7,7 +7,8 @@ from zephyr.models import Realm, Stream, UserProfile, UserActivity, \
     Subscription, Recipient, Message, UserMessage, valid_stream_name, \
     DefaultStream, UserPresence, MAX_SUBJECT_LENGTH, \
     MAX_MESSAGE_LENGTH, get_client, get_stream, get_recipient, get_huddle, \
-    get_user_profile_by_id, PreregistrationUser, get_display_recipient
+    get_user_profile_by_id, PreregistrationUser, get_display_recipient, \
+    to_dict_cache_key
 from django.db import transaction, IntegrityError
 from django.db.models import F
 from django.core.exceptions import ValidationError
@@ -27,7 +28,7 @@ from django.utils import timezone
 from zephyr.lib.create_user import create_user
 from zephyr.lib import bugdown
 from zephyr.lib.cache import cache_with_key, user_profile_by_id_cache_key, \
-    user_profile_by_email_cache_key, status_dict_cache_key
+    user_profile_by_email_cache_key, status_dict_cache_key, cache_set_many
 from zephyr.decorator import get_user_profile_by_email, json_to_list, JsonableError, \
      statsd_increment
 from zephyr.lib.event_queue import request_event_queue, get_user_events
@@ -826,6 +827,57 @@ def do_update_onboarding_steps(user_profile, steps):
                   users=[user_profile.id])
     tornado_callbacks.send_notification(notice)
 
+def do_update_message(user_profile, message_id, subject, content):
+    try:
+        message = Message.objects.select_related().get(id=message_id)
+    except Message.DoesNotExist:
+        raise JsonableError("Unknown message id")
+
+    event = {'type': 'update_message',
+             'sender': user_profile.email,
+             'message_id': message_id}
+
+    if message.sender != user_profile:
+        raise JsonableError("Message was not sent by you")
+
+    if content is not None:
+        rendered_content = bugdown.convert(content)
+        if rendered_content is None:
+            raise JsonableError("We were unable to render your updated message")
+
+        event['orig_content'] = message.content
+        event['orig_rendered_content'] = message.rendered_content
+        message.content = content
+        message.rendered_content = rendered_content
+        message.rendered_content_version = bugdown.version
+        event["content"] = content
+        event["rendered_content"] = rendered_content
+
+    if subject is not None:
+        event["orig_subject"] = message.subject
+        message.subject = subject
+        event["subject"] = subject
+
+    log_event(event)
+    message.save(update_fields=["subject", "content", "rendered_content",
+                                "rendered_content_version"])
+
+    # Update the message as stored in both the (deprecated) message
+    # cache (for shunting the message over to Tornado in the old
+    # get_messages API) and also the to_dict caches.
+    cache_save_message(message)
+    items_for_memcached = {}
+    items_for_memcached[to_dict_cache_key(message, True)] = \
+        (message.to_dict_uncached(apply_markdown=True,
+                                  rendered_content=message.rendered_content),)
+    items_for_memcached[to_dict_cache_key(message, False)] = \
+        (message.to_dict_uncached(apply_markdown=False),)
+    cache_set_many(items_for_memcached)
+
+    recipients = [um.user_profile_id for um in UserMessage.objects.filter(message=message_id)]
+    notice = dict(event=event, users=recipients)
+    tornado_callbacks.send_notification(notice)
+
 def do_finish_tutorial(user_profile):
     user_profile.tutorial_status = UserProfile.TUTORIAL_FINISHED
     user_profile.save()
@@ -950,6 +1002,11 @@ def do_events_register(user_profile, user_client, apply_markdown=True,
                                               ret['subscriptions'])
         elif event['type'] == "presence":
                 ret['presences'][event['email']] = event['presence']
+        elif event['type'] == "update_message":
+            # The client will get the updated message directly
+            pass
+        else:
+            raise ValueError("Unexpected event type %s" % (event['type'],))
 
     if events:
         ret['last_event_id'] = events[-1]['id']

zephyr/static/js/message_list.js

@@ -213,6 +213,11 @@ MessageList.prototype = {
             return false;
         }
 
+        this._rerender_preserving_scrolltop();
+        return true;
+    },
+
+    _rerender_preserving_scrolltop: function MessageList__rerender_preserving_scrolltop() {
         // scrolltop_offset is the number of pixels between the top of the
         // viewable window and the newly selected message
         var scrolltop_offset;
@@ -233,8 +238,6 @@ MessageList.prototype = {
         if (selected_in_view) {
             viewport.scrollTop(rows.get(this._selected_id, this.table_name).offset().top + scrolltop_offset);
         }
-
-        return true;
     },
 
     _render: function MessageList__render(messages, where) {
@@ -505,7 +508,7 @@ MessageList.prototype = {
         this._render_win_end += messages.length;
     },
 
-    add_and_rerender: function MessageList_interior(messages) {
+    add_and_rerender: function MessageList_add_and_rerender(messages) {
         // To add messages that might be in the interior of our
         // existing messages list, we just add the new messages and
         // then rerender the whole thing.
@@ -521,6 +524,15 @@ MessageList.prototype = {
                                        this._render_win_end), 'bottom');
     },
 
+    rerender: function MessageList_rerender() {
+        // We need to clear the rendering state, rather than just
+        // doing _clear_table, since we want to potentially recollapse
+        // things.
+        this._clear_rendering_state();
+        this._rerender_preserving_scrolltop();
+        this.select_id(this._selected_id);
+    },
+
     all: function MessageList_all() {
         return this._items;
     }

zephyr/static/js/zephyr.js

@@ -545,9 +545,10 @@ function case_insensitive_find(term, array) {
     }).length !== 0;
 }
 
-function process_message_for_recent_subjects(message) {
+function process_message_for_recent_subjects(message, remove_message) {
     var current_timestamp = 0;
     var max_subjects = 5;
+    var count = 0;
 
     if (! recent_subjects.hasOwnProperty(message.stream)) {
         recent_subjects[message.stream] = [];
@@ -557,6 +558,7 @@ function process_message_for_recent_subjects(message) {
                 var is_duplicate = (item.subject.toLowerCase() === message.subject.toLowerCase());
                 if (is_duplicate) {
                     current_timestamp = item.timestamp;
+                    count = item.count;
                 }
 
                 return !is_duplicate;
@@ -564,8 +566,18 @@ function process_message_for_recent_subjects(message) {
     }
 
     var recents = recent_subjects[message.stream];
-    recents.push({subject: message.subject,
-                  timestamp: Math.max(message.timestamp, current_timestamp)});
+
+    if (remove_message !== undefined) {
+        count = count - 1;
+    } else {
+        count = count + 1;
+    }
+
+    if (count !== 0) {
+        recents.push({subject: message.subject,
+                      count: count,
+                      timestamp: Math.max(message.timestamp, current_timestamp)});
+    }
 
     recents.sort(function (a, b) {
         return b.timestamp - a.timestamp;
@@ -788,6 +800,54 @@ function maybe_add_narrowed_messages(messages, msg_list) {
         }});
 }
 
+function update_message_unread_subjects(msg, event) {
+    if (event.subject !== undefined &&
+        unread_subjects[msg.stream] !== undefined &&
+        unread_subjects[msg.stream][msg.subject] !== undefined &&
+        unread_subjects[msg.stream][msg.subject][msg.id]) {
+        // Move the unread subject count to the new subject
+        delete unread_subjects[msg.stream][msg.subject][msg.id];
+        if (unread_subjects[msg.stream][msg.subject].length === 0) {
+            delete unread_subjects[msg.stream][msg.subject];
+        }
+        if (unread_subjects[msg.stream][event.subject] === undefined) {
+            unread_subjects[msg.stream][event.subject] = {};
+        }
+        unread_subjects[msg.stream][event.subject][msg.id] = true;
+    }
+}
+
+function update_messages(events) {
+    $.each(events, function (idx, event) {
+        var msg = all_msg_list.get(event.message_id);
+        if (event.rendered_content !== undefined) {
+            msg.content = event.rendered_content;
+        }
+
+        if (event.subject !== undefined) {
+            // Remove the recent subjects entry for the old subject;
+            // must be called before we update msg.subject
+            process_message_for_recent_subjects(msg, true);
+            // Update the unread counts; again, this must be called
+            // before we update msg.subject
+            update_message_unread_subjects(msg, event);
+
+            msg.subject = event.subject;
+            // Add the recent subjects entry for the new subject; must
+            // be called after we update msg.subject
+            process_message_for_recent_subjects(msg);
+        }
+    });
+
+    home_msg_list.rerender();
+    if (current_msg_list === narrowed_msg_list) {
+        narrowed_msg_list.rerender();
+    }
+    compose.update_faded_messages();
+    update_unread_counts();
+    stream_list.update_streams_sidebar();
+}
+
 var get_updates_xhr;
 var get_updates_timeout;
 function get_updates(options) {
@@ -820,6 +880,7 @@ function get_updates(options) {
             $('#connection-error').hide();
 
             var messages = [];
+            var messages_to_update = [];
             var new_pointer;
 
             $.each(data.events, function (idx, event) {
@@ -839,6 +900,9 @@ function get_updates(options) {
                 case 'onboarding_steps':
                     onboarding.set_step_info(event.steps);
                     break;
+                case 'update_message':
+                    messages_to_update.push(event);
+                    break;
                 case 'realm_user':
                     if (event.op === 'add') {
                         add_person(event.person);
@@ -912,6 +976,10 @@ function get_updates(options) {
                 home_msg_list.select_id(home_msg_list.first().id, {then_scroll: false});
             }
 
+            if (messages_to_update.length !== 0) {
+                update_messages(messages_to_update);
+            }
+
             get_updates_timeout = setTimeout(get_updates, 0);
         },
         error: function (xhr, error_type, exn) {

zephyr/views.py

@@ -30,7 +30,7 @@ from zephyr.lib.actions import do_add_subscription, do_remove_subscription, \
     update_user_presence, bulk_add_subscriptions, update_message_flags, \
     recipient_for_emails, extract_recipients, do_events_register, do_finish_tutorial, \
     get_status_dict, do_change_enable_offline_email_notifications, \
-    do_update_onboarding_steps
+    do_update_onboarding_steps, do_update_message
 from zephyr.forms import RegistrationForm, HomepageForm, ToSForm, CreateBotForm, \
     is_unique, is_inactive, isnt_mit
 from django.views.decorators.csrf import csrf_exempt
@@ -1034,6 +1034,20 @@ def json_tutorial_status(request, user_profile, status=REQ('status')):
 
     return json_success()
 
+@authenticated_json_post_view
+def json_update_message(request, user_profile):
+    return update_message_backend(request, user_profile)
+
+@has_request_variables
+def update_message_backend(request, user_profile,
+                           message_id=REQ(converter=to_non_negative_int),
+                           subject=REQ(default=None),
+                           content=REQ(default=None)):
+    if subject is None and content is None:
+        return json_error("Nothing to change")
+    do_update_message(user_profile, message_id, subject, content)
+    return json_success()
+
 # We do not @require_login for send_message_backend, since it is used
 # both from the API and the web service.  Code calling
 # send_message_backend should either check the API key or check that