Merge pull request #102 from rapidpro/timelines

Simpler timeline rendering

casepro/backend/__init__.py

@@ -195,8 +195,7 @@ def fetch_contact_messages(self, org, contact, created_after, created_before):
         :param contact: the contact
         :param created_after: include messages created after this time
         :param created_before: include messages created before this time
-        :return: the messages as JSON objects in reverse chronological order. JSON format should match that returned by
-                 Message.as_json() for incoming messages and Outgoing.as_json() for outgoing messages.
+        :return: the messages as transient Message and Outgoing instances
         """
 
     @abstractmethod

casepro/backend/rapidpro.py

@@ -325,27 +325,18 @@ def fetch_contact_messages(self, org, contact, created_after, created_before):
         """
         Used to grab messages sent to the contact from RapidPro that we won't have in CasePro
         """
-        contact_json = contact.as_json(full=False)
-
         # fetch remote messages for contact
         client = self._get_client(org, 2)
         remote_messages = client.get_messages(contact=contact.uuid, after=created_after, before=created_before).all()
 
-        def remote_as_json(msg):
-            # should match schema of Outgoing.as_json()
-            return {
-                'id': msg.broadcast,
-                'contact': contact_json,
-                'urn': None,
-                'text': msg.text,
-                'time': msg.created_on,
-                'direction': Outgoing.DIRECTION,
-                'case': None,
-                'sender': None
-            }
-
-        return [remote_as_json(m) for m in remote_messages if m.direction == 'out']
+        def remote_as_outgoing(msg):
+            return Outgoing(backend_broadcast_id=msg.broadcast, contact=contact, text=msg.text,
+                            created_on=msg.created_on)
+
+        return [remote_as_outgoing(m) for m in remote_messages if m.direction == 'out']
 
     def get_url_patterns(self):
-        """The Rapidpro backend doesn't have any urls to register."""
+        """
+        No urls to register as everything is pulled from RapidPro
+        """
         return []

casepro/backend/tests/test_rapidpro.py

@@ -15,7 +15,7 @@
 from unittest import skip
 
 from casepro.contacts.models import Contact, Field, Group
-from casepro.msgs.models import Label, Message
+from casepro.msgs.models import Label, Message, Outgoing
 from casepro.test import BaseCasesTest
 
 from ..rapidpro import RapidProBackend, ContactSyncer, MessageSyncer
@@ -825,22 +825,12 @@ def test_fetch_contact_messages(self, mock_get_messages):
 
         messages = self.backend.fetch_contact_messages(self.unicef, self.ann, d1, d3)
 
-        self.assertEqual(messages, [
-            {
-                'id': 201,  # id is the broadcast id
-                'contact': {'id': self.ann.pk, 'name': "Ann"},
-                'urn': None,
-                'text': "Welcome",
-                'time': d3,
-                'direction': 'O',
-                'case': None,
-                'sender': None,
-            }
-        ])
-
-        # check that JSON schemas match local outgoing model
-        outgoing = self.create_outgoing(self.unicef, self.admin, 201, 'B', "Hello", self.ann)
-        self.assertEqual(messages[0].keys(), outgoing.as_json().keys())
+        self.assertEqual(len(messages), 1)
+        self.assertIsInstance(messages[0], Outgoing)
+        self.assertEqual(messages[0].backend_broadcast_id, 201)
+        self.assertEqual(messages[0].contact, self.ann)
+        self.assertEqual(messages[0].text, "Welcome")
+        self.assertEqual(messages[0].created_on, d3)
 
     def test_get_url_patterns(self):
         """

casepro/cases/models.py

@@ -15,6 +15,7 @@
 from casepro.backend import get_backend
 from casepro.contacts.models import Contact
 from casepro.msgs.models import Label, Message, Outgoing
+from casepro.utils import TimelineItem
 from casepro.utils.export import BaseSearchExport
 
 
@@ -259,8 +260,7 @@ def get_timeline(self, after, before, merge_from_backend):
         local_incoming = local_incoming.order_by('-created_on')
 
         # merge local incoming and outgoing
-        local_messages = chain(local_outgoing, local_incoming)
-        messages = [{'time': msg.created_on, 'type': 'M', 'item': msg.as_json()} for msg in local_messages]
+        timeline = [TimelineItem(msg) for msg in chain(local_outgoing, local_incoming)]
 
         if merge_from_backend:
             # if this is the initial request, fetch additional messages from the backend
@@ -272,16 +272,16 @@ def get_timeline(self, after, before, merge_from_backend):
                 local_broadcast_ids = {o.backend_broadcast_id for o in local_outgoing if o.backend_broadcast_id}
 
                 for msg in backend_messages:
-                    if msg['id'] not in local_broadcast_ids:
-                        messages.append({'time': msg['time'], 'type': 'M', 'item': msg})
+                    if msg.backend_broadcast_id not in local_broadcast_ids:
+                        timeline.append(TimelineItem(msg))
 
-        # fetch actions in chronological order
+        # fetch and append actions
         actions = self.actions.filter(created_on__gte=after, created_on__lte=before)
-        actions = actions.select_related('assignee', 'created_by').order_by('pk')
-        actions = [{'time': a.created_on, 'type': 'A', 'item': a.as_json()} for a in actions]
+        actions = actions.select_related('assignee', 'created_by')
+        timeline += [TimelineItem(a) for a in actions]
 
-        # merge actions and messages and sort by time
-        return sorted(messages + actions, key=lambda event: event['time'])
+        # sort timeline by reverse chronological order
+        return sorted(timeline, key=lambda item: item.get_time())
 
     def add_reply(self, message):
         message.case = self
@@ -462,6 +462,8 @@ class CaseAction(models.Model):
                       (CLOSE, _("Close")),
                       (REOPEN, _("Reopen")))
 
+    TIMELINE_TYPE = 'A'
+
     case = models.ForeignKey(Case, related_name="actions")
 
     action = models.CharField(max_length=1, choices=ACTION_CHOICES)

casepro/cases/tests.py

@@ -654,16 +654,8 @@ def test_timeline(self, mock_fetch_contact_messages):
         CaseAction.create(case, self.user1, CaseAction.OPEN, assignee=self.moh)
 
         # backend has a message in the case time window that we don't have locally
-        remote_message1 = {
-            'id': 102,
-            'contact': {'id': self.ann.pk, 'name': "Ann"},
-            'urn': None,
-            'text': "Non casepro message...",
-            'time': d2,
-            'direction': 'O',
-            'case': None,
-            'sender': None,
-        }
+        remote_message1 = Outgoing(backend_broadcast_id=102, contact=self.ann, text="Non casepro message...",
+                                   created_on=d2)
         mock_fetch_contact_messages.return_value = [remote_message1]
 
         timeline_url = reverse('cases.case_timeline', args=[case.pk])
@@ -676,14 +668,12 @@ def test_timeline(self, mock_fetch_contact_messages):
         t0 = microseconds_to_datetime(response.json['max_time'])
 
         self.assertEqual(len(response.json['results']), 3)
-        self.assertEqual(response.json['results'][0]['type'], 'M')
+        self.assertEqual(response.json['results'][0]['type'], 'I')
         self.assertEqual(response.json['results'][0]['item']['text'], "What is AIDS?")
         self.assertEqual(response.json['results'][0]['item']['contact'], {'id': self.ann.pk, 'name': "Ann"})
-        self.assertEqual(response.json['results'][0]['item']['direction'], 'I')
-        self.assertEqual(response.json['results'][1]['type'], 'M')
+        self.assertEqual(response.json['results'][1]['type'], 'O')
         self.assertEqual(response.json['results'][1]['item']['text'], "Non casepro message...")
         self.assertEqual(response.json['results'][1]['item']['contact'], {'id': self.ann.pk, 'name': "Ann"})
-        self.assertEqual(response.json['results'][1]['item']['direction'], 'O')
         self.assertEqual(response.json['results'][2]['type'], 'A')
         self.assertEqual(response.json['results'][2]['item']['action'], 'O')
 
@@ -724,9 +714,8 @@ def test_timeline(self, mock_fetch_contact_messages):
         t3 = microseconds_to_datetime(response.json['max_time'])
 
         self.assertEqual(len(response.json['results']), 1)
-        self.assertEqual(response.json['results'][0]['type'], 'M')
+        self.assertEqual(response.json['results'][0]['type'], 'O')
         self.assertEqual(response.json['results'][0]['item']['text'], "It's bad")
-        self.assertEqual(response.json['results'][0]['item']['direction'], 'O')
 
         # contact sends a reply
         d4 = timezone.now()
@@ -738,9 +727,8 @@ def test_timeline(self, mock_fetch_contact_messages):
         t4 = microseconds_to_datetime(response.json['max_time'])
 
         self.assertEqual(len(response.json['results']), 1)
-        self.assertEqual(response.json['results'][0]['type'], 'M')
+        self.assertEqual(response.json['results'][0]['type'], 'I')
         self.assertEqual(response.json['results'][0]['item']['text'], "OK thanks")
-        self.assertEqual(response.json['results'][0]['item']['direction'], 'I')
 
         # page again looks for new timeline activity
         response = self.url_get('unicef', '%s?after=%s' % (timeline_url, datetime_to_microseconds(t4)))
@@ -774,16 +762,7 @@ def test_timeline(self, mock_fetch_contact_messages):
 
         # backend has the message sent during the case as well as the unrelated message
         mock_fetch_contact_messages.return_value = [
-            {
-                'id': 202,
-                'contact': {'id': self.ann.pk, 'name': "Ann"},
-                'urn': None,
-                'text': "It's bad",
-                'time': d3,
-                'direction': 'O',
-                'case': None,
-                'sender': None,
-            },
+            Outgoing(backend_broadcast_id=202, contact=self.ann, text="It's bad", created_on=d3),
             remote_message1
         ]
 
@@ -792,24 +771,21 @@ def test_timeline(self, mock_fetch_contact_messages):
         items = response.json['results']
 
         self.assertEqual(len(items), 7)
-        self.assertEqual(items[0]['type'], 'M')
+        self.assertEqual(items[0]['type'], 'I')
         self.assertEqual(items[0]['item']['text'], "What is AIDS?")
         self.assertEqual(items[0]['item']['contact'], {'id': self.ann.pk, 'name': "Ann"})
-        self.assertEqual(items[0]['item']['direction'], 'I')
-        self.assertEqual(items[1]['type'], 'M')
+        self.assertEqual(items[1]['type'], 'O')
         self.assertEqual(items[1]['item']['text'], "Non casepro message...")
         self.assertEqual(items[1]['item']['contact'], {'id': self.ann.pk, 'name': "Ann"})
-        self.assertEqual(items[1]['item']['direction'], 'O')
         self.assertEqual(items[1]['item']['sender'], None)
         self.assertEqual(items[2]['type'], 'A')
         self.assertEqual(items[2]['item']['action'], 'O')
         self.assertEqual(items[3]['type'], 'A')
         self.assertEqual(items[3]['item']['action'], 'N')
-        self.assertEqual(items[4]['type'], 'M')
-        self.assertEqual(items[4]['item']['direction'], 'O')
+        self.assertEqual(items[4]['type'], 'O')
         self.assertEqual(items[4]['item']['sender'], {'id': self.user1.pk, 'name': "Evan"})
-        self.assertEqual(items[5]['type'], 'M')
-        self.assertEqual(items[5]['item']['direction'], 'I')
+        self.assertEqual(items[5]['type'], 'I')
+        self.assertEqual(items[5]['item']['text'], "OK thanks")
         self.assertEqual(items[6]['type'], 'A')
         self.assertEqual(items[6]['item']['action'], 'C')
 

casepro/msgs/models.py

@@ -180,11 +180,11 @@ class Message(models.Model):
 
     TYPE_CHOICES = ((TYPE_INBOX, _("Inbox")), (TYPE_FLOW, _("Flow")))
 
-    DIRECTION = 'I'
-
     SAVE_CONTACT_ATTR = '__data__contact'
     SAVE_LABELS_ATTR = '__data__labels'
 
+    TIMELINE_TYPE = 'I'
+
     org = models.ForeignKey(Org, verbose_name=_("Organization"), related_name='incoming_messages')
 
     backend_id = models.IntegerField(unique=True, help_text=_("Backend identifier for this message"))
@@ -424,7 +424,6 @@ def as_json(self):
             'labels': [l.as_json(full=False) for l in self.labels.all()],
             'flagged': self.is_flagged,
             'archived': self.is_archived,
-            'direction': self.DIRECTION,
             'flow': self.type == self.TYPE_FLOW,
             'case': self.case.as_json(full=False) if self.case else None
         }
@@ -491,7 +490,7 @@ class Outgoing(models.Model):
     ACTIVITY_CHOICES = ((BULK_REPLY, _("Bulk Reply")), (CASE_REPLY, "Case Reply"), (FORWARD, _("Forward")))
     REPLY_ACTIVITIES = (BULK_REPLY, CASE_REPLY)
 
-    DIRECTION = 'O'
+    TIMELINE_TYPE = 'O'
 
     org = models.ForeignKey(Org, verbose_name=_("Organization"), related_name='outgoing_messages')
 
@@ -619,6 +618,16 @@ def search_replies(cls, org, user, search):
     def is_reply(self):
         return self.activity in self.REPLY_ACTIVITIES
 
+    def get_sender(self):
+        """
+        Convenience method for accessing created_by since it can be null on transient instances returned from
+        Backend.fetch_contact_messages
+        """
+        try:
+            return self.created_by
+        except User.DoesNotExist:
+            return None
+
     def as_json(self):
         """
         Prepares this outgoing message for JSON serialization
@@ -629,9 +638,8 @@ def as_json(self):
             'urn': self.urn,
             'text': self.text,
             'time': self.created_on,
-            'direction': self.DIRECTION,
             'case': self.case.as_json(full=False) if self.case else None,
-            'sender': self.created_by.as_json(full=False)
+            'sender': self.get_sender().as_json(full=False) if self.get_sender() else None
         }
 
     def __str__(self):

casepro/msgs/tests.py

@@ -689,7 +689,6 @@ def test_as_json(self):
             'labels': [{'id': self.aids.pk, 'name': "AIDS"}],
             'flagged': False,
             'archived': False,
-            'direction': 'I',
             'flow': False,
             'case': None
         })
@@ -1114,7 +1113,6 @@ def test_as_json(self):
             'urn': None,
             'text': "That's great",
             'time': outgoing.created_on,
-            'direction': 'O',
             'case': None,
             'sender': {'id': self.user1.pk, 'name': "Evan"}
         })
@@ -1149,7 +1147,6 @@ def test_search(self):
                 'contact': {'id': self.ann.pk, 'name': "Ann"},
                 'urn': None,
                 'text': "Hello 2",
-                'direction': 'O',
                 'case': None,
                 'sender': {'id': self.user1.pk, 'name': "Evan"},
                 'time': format_iso8601(out2.created_on)
@@ -1159,7 +1156,6 @@ def test_search(self):
                 'contact': {'id': self.ann.pk, 'name': "Ann"},
                 'urn': None,
                 'text': "Hello 1",
-                'direction': 'O',
                 'case': None,
                 'sender': {'id': self.admin.pk, 'name': "Kidus"},
                 'time': format_iso8601(out1.created_on)
@@ -1176,7 +1172,6 @@ def test_search(self):
                 'contact': {'id': self.ann.pk, 'name': "Ann"},
                 'urn': None,
                 'text': "Hello 2",
-                'direction': 'O',
                 'case': None,
                 'sender': {'id': self.user1.pk, 'name': "Evan"},
                 'time': format_iso8601(out2.created_on)
@@ -1212,7 +1207,6 @@ def test_search_replies(self):
                 'contact': {'id': self.bob.pk, 'name': "Bob"},
                 'urn': None,
                 'text': "Hello 2",
-                'direction': 'O',
                 'case': None,
                 'sender': {'id': self.admin.pk, 'name': "Kidus"},
                 'time': format_iso8601(out2.created_on),
@@ -1228,7 +1222,6 @@ def test_search_replies(self):
                 'contact': {'id': self.ann.pk, 'name': "Ann"},
                 'urn': None,
                 'text': "Hello 1",
-                'direction': 'O',
                 'case': {'id': case.pk, 'assignee': {'id': self.moh.pk, 'name': "MOH"}},
                 'sender': {'id': self.user1.pk, 'name': "Evan"},
                 'time': format_iso8601(out1.created_on),

casepro/utils/__init__.py

@@ -141,6 +141,20 @@ def date_range(start, stop):
         yield start + timedelta(n)
 
 
+class TimelineItem(object):
+    """
+    Wraps a message or action for easier inclusion in a merged timeline
+    """
+    def __init__(self, item):
+        self.item = item
+
+    def get_time(self):
+        return self.item.created_on
+
+    def to_json(self):
+        return {'time': self.get_time(), 'type': self.item.TIMELINE_TYPE, 'item': self.item.as_json()}
+
+
 def uuid_to_int(uuid):
     """
     Converts a UUID hex string to an int within the range of a Django IntegerField, and also >=0, as the URL regexes

casepro/utils/tests.py

@@ -14,7 +14,7 @@
 
 from casepro.test import BaseCasesTest
 
-from . import safe_max, normalize, match_keywords, truncate, str_to_bool, json_encode, uuid_to_int
+from . import safe_max, normalize, match_keywords, truncate, str_to_bool, json_encode, TimelineItem, uuid_to_int
 from . import date_to_milliseconds, datetime_to_microseconds, microseconds_to_datetime, month_range, date_range
 from .email import send_email
 from .middleware import JSONMiddleware
@@ -106,6 +106,12 @@ def test_date_range(self):
         ])
         self.assertEqual(list(date_range(date(2015, 1, 29), date(2015, 1, 29))), [])
 
+    def test_timeline_item(self):
+        d1 = datetime(2015, 10, 1, 9, 0, 0, 0, pytz.UTC)
+        ann = self.create_contact(self.unicef, 'C-101', "Ann")
+        msg = self.create_message(self.unicef, 102, ann, "Hello", created_on=d1)
+        self.assertEqual(TimelineItem(msg).to_json(), {'time': d1, 'type': 'I', 'item': msg.as_json()})
+
     def test_uuid_to_int_range(self):
         """
         Ensures that the integer returned will always be in the range [0, 2147483647].