Fixed case close events appearing repeatedly in case timelines

casepro/cases/models.py

@@ -23,7 +23,7 @@
 from redis_cache import get_redis_connection
 from temba_client.base import TembaNoSuchObjectError, TembaException
 from casepro.email import send_email
-from . import parse_csv, normalize, match_keywords, SYSTEM_LABEL_FLAGGED
+from . import parse_csv, normalize, match_keywords, safe_max, SYSTEM_LABEL_FLAGGED
 
 
 # only show unlabelled messages newer than 2 weeks
@@ -549,6 +549,53 @@ def get_or_open(cls, org, user, labels, message, summary, assignee, update_conta
 
         return case
 
+    def get_timeline(self, after, before):
+        label_map = {l.name: l for l in Label.get_all(self.org)}
+
+        # if this isn't a first request for the existing items, we check on our side to see if there will be new
+        # items before hitting the RapidPro API
+        do_api_fetch = True
+        if after != self.message_on:
+            last_event = self.events.order_by('-pk').first()
+            last_outgoing = self.outgoing_messages.order_by('-pk').first()
+            last_event_time = last_event.created_on if last_event else None
+            last_outgoing_time = last_outgoing.created_on if last_outgoing else None
+            last_message_time = safe_max(last_event_time, last_outgoing_time)
+            do_api_fetch = last_message_time and after <= last_message_time
+
+        if do_api_fetch:
+            # fetch messages
+            remote = self.org.get_temba_client().get_messages(contacts=[self.contact.uuid],
+                                                              after=after,
+                                                              before=before)
+
+            local_outgoing = self.outgoing_messages.filter(created_on__gte=after, created_on__lte=before)
+            local_by_broadcast = {o.broadcast_id: o for o in local_outgoing}
+
+            # merge remotely fetched and local outgoing messages
+            messages = []
+            for m in remote:
+                local = local_by_broadcast.pop(m.broadcast, None)
+                if local:
+                    m.sender = local.created_by
+                messages.append({'time': m.created_on, 'type': 'M', 'item': Message.as_json(m, label_map)})
+
+            for m in local_by_broadcast.values():
+                messages.append({'time': m.created_on, 'type': 'M', 'item': m.as_json()})
+
+        else:
+            messages = []
+
+        # fetch actions in chronological order
+        actions = self.actions.filter(created_on__gte=after, created_on__lte=before)
+        actions = actions.select_related('assignee', 'created_by').order_by('pk')
+
+        # merge actions and messages and JSON-ify both
+        timeline = messages
+        timeline += [{'time': a.created_on, 'type': 'A', 'item': a.as_json()} for a in actions]
+        timeline = sorted(timeline, key=lambda event: event['time'])
+        return timeline
+
     @case_action()
     def update_summary(self, user, summary):
         self.summary = summary

casepro/cases/tests.py

@@ -467,6 +467,39 @@ def test_timeline(self, mock_create_broadcast, mock_get_messages):
         # back to having no reason to hit the RapidPro API
         self.assertEqual(mock_get_messages.call_count, 0)
 
+        # user closes case
+        case.close(self.user1)
+
+        # contact sends new message after that
+        d5 = timezone.now()
+        msg5 = TembaMessage.create(id=105, contact='C-001', created_on=d5, text="But wait", labels=[], direction='I')
+        mock_get_messages.return_value = [msg5]
+
+        # page again looks for new timeline activity
+        response = self.url_get('unicef', '%s?after=%s' % (timeline_url, datetime_to_microseconds(t5)))
+        t6 = microseconds_to_datetime(response.json['max_time'])
+
+        # should show the close event but not the message after it
+        self.assertEqual(len(response.json['results']), 1)
+        self.assertEqual(response.json['results'][0]['type'], 'A')
+        self.assertEqual(response.json['results'][0]['item']['action'], 'C')
+
+        # no reason to hit the API
+        self.assertEqual(mock_get_messages.call_count, 0)
+
+        # another look for new timeline activity
+        response = self.url_get('unicef', '%s?after=%s' % (timeline_url, datetime_to_microseconds(t6)))
+        t7 = microseconds_to_datetime(response.json['max_time'])
+
+        # nothing to see
+        self.assertEqual(len(response.json['results']), 0)
+
+        # and one last look for new timeline activity
+        response = self.url_get('unicef', '%s?after=%s' % (timeline_url, datetime_to_microseconds(t7)))
+
+        # nothing to see
+        self.assertEqual(len(response.json['results']), 0)
+
 
 class ContactTest(BaseCasesTest):
     @patch('dash.orgs.models.TembaClient.get_contact')

casepro/cases/views.py

@@ -15,7 +15,7 @@
 from smartmin.users.views import SmartCRUDL, SmartListView, SmartCreateView, SmartReadView, SmartFormView
 from smartmin.users.views import SmartUpdateView, SmartDeleteView, SmartTemplateView
 from temba_client.utils import parse_iso8601
-from . import parse_csv, json_encode, normalize, safe_max, str_to_bool, MAX_MESSAGE_CHARS, SYSTEM_LABEL_FLAGGED
+from . import parse_csv, json_encode, normalize, str_to_bool, MAX_MESSAGE_CHARS, SYSTEM_LABEL_FLAGGED
 from .models import AccessLevel, Case, Group, Label, Message, MessageAction, MessageExport, Partner, Outgoing
 from .tasks import message_export
 from .utils import datetime_to_microseconds, microseconds_to_datetime
@@ -227,63 +227,28 @@ class Timeline(OrgPermsMixin, SmartReadView):
 
         def get_context_data(self, **kwargs):
             context = super(CaseCRUDL.Timeline, self).get_context_data(**kwargs)
-            org = self.request.org
+            now = timezone.now()
+            empty = False
 
             after = self.request.GET.get('after', None)
             if after:
                 after = microseconds_to_datetime(int(after))
             else:
                 after = self.object.message_on
 
-            before = self.object.closed_on if self.object.closed_on else timezone.now()
-
-            label_map = {l.name: l for l in Label.get_all(self.request.org)}
-
-            # if this isn't a first request for the existing items, we check on our side to see if there will be new
-            # items before hitting the RapidPro API
-            do_api_fetch = True
-            if after != self.object.message_on:
-                last_event = self.object.events.order_by('-pk').first()
-                last_outgoing = self.object.outgoing_messages.order_by('-pk').first()
-                last_event_time = last_event.created_on if last_event else None
-                last_outgoing_time = last_outgoing.created_on if last_outgoing else None
-                last_message_time = safe_max(last_event_time, last_outgoing_time)
-                do_api_fetch = last_message_time and after <= last_message_time
-
-            if do_api_fetch:
-                # fetch messages
-                remote = org.get_temba_client().get_messages(contacts=[self.object.contact.uuid],
-                                                             after=after, before=before)
-
-                local_outgoing = Outgoing.objects.filter(case=self.object,
-                                                         created_on__gte=after, created_on__lte=before)
-                local_by_broadcast = {o.broadcast_id: o for o in local_outgoing}
-
-                # merge remotely fetched and local outgoing messages
-                messages = []
-                for m in remote:
-                    local = local_by_broadcast.pop(m.broadcast, None)
-                    if local:
-                        m.sender = local.created_by
-                    messages.append({'time': m.created_on, 'type': 'M', 'item': Message.as_json(m, label_map)})
-
-                for m in local_by_broadcast.values():
-                    messages.append({'time': m.created_on, 'type': 'M', 'item': m.as_json()})
+            if self.object.closed_on:
+                if after > self.object.closed_on:
+                    empty = True
 
+                # don't return anything after a case close event
+                before = self.object.closed_on
             else:
-                messages = []
-
-            # fetch actions in chronological order
-            actions = self.object.actions.filter(created_on__gte=after, created_on__lte=before)
-            actions = actions.select_related('assignee', 'created_by').order_by('pk')
+                before = now
 
-            # merge actions and messages and JSON-ify both
-            timeline = messages
-            timeline += [{'time': a.created_on, 'type': 'A', 'item': a.as_json()} for a in actions]
-            timeline = sorted(timeline, key=lambda event: event['time'])
+            timeline = self.object.get_timeline(after, before) if not empty else []
 
             context['timeline'] = timeline
-            context['max_time'] = datetime_to_microseconds(before)
+            context['max_time'] = datetime_to_microseconds(now)
             return context
 
         def render_to_response(self, context, **response_kwargs):