Ticket bulk actions endpoint should report failures like message bulk actions endpoint

temba/api/v2/serializers.py

@@ -1676,15 +1676,18 @@ def save(self):
         topic = self.validated_data.get("topic")
 
         if action == self.ACTION_ASSIGN:
-            Ticket.bulk_assign(org, user, tickets, assignee=assignee)
+            changed = Ticket.bulk_assign(org, user, tickets, assignee=assignee)
         elif action == self.ACTION_ADD_NOTE:
-            Ticket.bulk_add_note(org, user, tickets, note=note)
+            changed = Ticket.bulk_add_note(org, user, tickets, note=note)
         elif action == self.ACTION_CHANGE_TOPIC:
-            Ticket.bulk_change_topic(org, user, tickets, topic=topic)
+            changed = Ticket.bulk_change_topic(org, user, tickets, topic=topic)
         elif action == self.ACTION_CLOSE:
-            Ticket.bulk_close(org, user, tickets)
+            changed = Ticket.bulk_close(org, user, tickets)
         elif action == self.ACTION_REOPEN:
-            Ticket.bulk_reopen(org, user, tickets)
+            changed = Ticket.bulk_reopen(org, user, tickets)
+
+        failed = [t.uuid for t in tickets if t not in changed]
+        return BulkActionFailure(failed) if failed else None
 
 
 class TopicReadSerializer(ReadSerializer):

temba/api/v2/tests/test_ticket_actions.py

@@ -125,13 +125,16 @@ def test_endpoint(self, mr_mocks):
         self.assertEqual(sales, ticket2.topic)
 
         # close tickets
-        self.assertPost(
+        response = self.assertPost(
             endpoint_url,
             self.agent,
             {"tickets": [str(ticket1.uuid), str(ticket2.uuid)], "action": "close"},
-            status=204,
+            status=200,
         )
 
+        # ticket1 reported as failure because it is already closed
+        self.assertEqual({"failures": [str(ticket1.uuid)]}, response.json())
+
         ticket1.refresh_from_db()
         ticket2.refresh_from_db()
         self.assertEqual("C", ticket1.status)

temba/mailroom/client/client.py

@@ -312,15 +312,10 @@ def ticket_change_topic(self, org, user, tickets, topic):
             },
         )
 
-    def ticket_close(self, org, user, tickets, force: bool):
+    def ticket_close(self, org, user, tickets):
         return self._request(
             "ticket/close",
-            {
-                "org_id": org.id,
-                "user_id": user.id,
-                "ticket_ids": [t.id for t in tickets],
-                "force": force,
-            },
+            {"org_id": org.id, "user_id": user.id, "ticket_ids": [t.id for t in tickets]},
         )
 
     def ticket_reopen(self, org, user, tickets):

temba/mailroom/client/tests.py

@@ -714,18 +714,13 @@ def test_ticket_close(self, mock_post):
         ticket2 = self.create_ticket(bob)
 
         mock_post.return_value = MockJsonResponse(200, {"changed_ids": [ticket1.id]})
-        response = self.client.ticket_close(self.org, self.admin, [ticket1, ticket2], force=True)
+        response = self.client.ticket_close(self.org, self.admin, [ticket1, ticket2])
 
         self.assertEqual({"changed_ids": [ticket1.id]}, response)
         mock_post.assert_called_once_with(
             "http://localhost:8090/mr/ticket/close",
             headers={"User-Agent": "Temba", "Authorization": "Token sesame"},
-            json={
-                "org_id": self.org.id,
-                "user_id": self.admin.id,
-                "ticket_ids": [ticket1.id, ticket2.id],
-                "force": True,
-            },
+            json={"org_id": self.org.id, "user_id": self.admin.id, "ticket_ids": [ticket1.id, ticket2.id]},
         )
 
     @patch("requests.post")

temba/tests/mailroom.py

@@ -390,10 +390,13 @@ def org_deindex(self, org):
     @_client_method
     def ticket_assign(self, org, user, tickets, assignee):
         now = timezone.now()
-        tickets = Ticket.objects.filter(org=org, id__in=[t.id for t in tickets]).exclude(assignee=assignee)
-        tickets.update(assignee=assignee, modified_on=now, last_activity_on=now)
+        tickets = list(Ticket.objects.filter(org=org, id__in=[t.id for t in tickets]).exclude(assignee=assignee))
 
         for ticket in tickets:
+            ticket.assignee = assignee
+            ticket.modified_on = now
+            ticket.last_activity_on = now
+            ticket.save(update_fields=("assignee", "modified_on", "last_activity_on"))
             ticket.events.create(
                 org=org,
                 contact=ticket.contact,
@@ -407,10 +410,12 @@ def ticket_assign(self, org, user, tickets, assignee):
     @_client_method
     def ticket_add_note(self, org, user, tickets, note: str):
         now = timezone.now()
-        tickets = Ticket.objects.filter(org=org, id__in=[t.id for t in tickets])
-        tickets.update(modified_on=now, last_activity_on=now)
+        tickets = list(Ticket.objects.filter(org=org, id__in=[t.id for t in tickets]))
 
         for ticket in tickets:
+            ticket.modified_on = now
+            ticket.last_activity_on = now
+            ticket.save(update_fields=("modified_on", "last_activity_on"))
             ticket.events.create(
                 org=org,
                 contact=ticket.contact,
@@ -424,10 +429,13 @@ def ticket_add_note(self, org, user, tickets, note: str):
     @_client_method
     def ticket_change_topic(self, org, user, tickets, topic):
         now = timezone.now()
-        tickets = Ticket.objects.filter(org=org, id__in=[t.id for t in tickets]).exclude(topic=topic)
-        tickets.update(topic=topic, modified_on=now, last_activity_on=now)
+        tickets = list(Ticket.objects.filter(org=org, id__in=[t.id for t in tickets]).exclude(topic=topic))
 
         for ticket in tickets:
+            ticket.topic = topic
+            ticket.modified_on = now
+            ticket.last_activity_on = now
+            ticket.save(update_fields=("topic", "modified_on", "last_activity_on"))
             ticket.events.create(
                 org=org,
                 contact=ticket.contact,
@@ -439,21 +447,25 @@ def ticket_change_topic(self, org, user, tickets, topic):
         return {"changed_ids": [t.id for t in tickets]}
 
     @_client_method
-    def ticket_close(self, org, user, tickets, force: bool):
-        tickets = Ticket.objects.filter(org=org, id__in=[t.id for t in tickets], status=Ticket.STATUS_OPEN)
-        tickets.update(status=Ticket.STATUS_CLOSED, closed_on=timezone.now())
+    def ticket_close(self, org, user, tickets):
+        tickets = list(Ticket.objects.filter(org=org, id__in=[t.id for t in tickets], status=Ticket.STATUS_OPEN))
 
         for ticket in tickets:
+            ticket.status = Ticket.STATUS_CLOSED
+            ticket.closed_on = timezone.now()
+            ticket.save(update_fields=("status", "closed_on"))
             ticket.events.create(org=org, contact=ticket.contact, event_type=TicketEvent.TYPE_CLOSED, created_by=user)
 
         return {"changed_ids": [t.id for t in tickets]}
 
     @_client_method
     def ticket_reopen(self, org, user, tickets):
-        tickets = Ticket.objects.filter(org=org, id__in=[t.id for t in tickets], status=Ticket.STATUS_CLOSED)
-        tickets.update(status=Ticket.STATUS_OPEN, closed_on=None)
+        tickets = list(Ticket.objects.filter(org=org, id__in=[t.id for t in tickets], status=Ticket.STATUS_CLOSED))
 
         for ticket in tickets:
+            ticket.status = Ticket.STATUS_OPEN
+            ticket.closed_on = None
+            ticket.save(update_fields=("status", "closed_on"))
             ticket.events.create(org=org, contact=ticket.contact, event_type=TicketEvent.TYPE_REOPENED, created_by=user)
 
         return {"changed_ids": [t.id for t in tickets]}

temba/tickets/models.py

@@ -216,23 +216,27 @@ def add_note(self, user: User, *, note: str):
 
     @classmethod
     def bulk_assign(cls, org, user: User, tickets: list, assignee: User):
-        return mailroom.get_client().ticket_assign(org, user, tickets, assignee)
+        return cls._bulk_response(mailroom.get_client().ticket_assign(org, user, tickets, assignee), tickets)
 
     @classmethod
     def bulk_add_note(cls, org, user: User, tickets: list, note: str):
-        return mailroom.get_client().ticket_add_note(org, user, tickets, note)
+        return cls._bulk_response(mailroom.get_client().ticket_add_note(org, user, tickets, note), tickets)
 
     @classmethod
     def bulk_change_topic(cls, org, user: User, tickets: list, topic: Topic):
-        return mailroom.get_client().ticket_change_topic(org, user, tickets, topic)
+        return cls._bulk_response(mailroom.get_client().ticket_change_topic(org, user, tickets, topic), tickets)
 
     @classmethod
-    def bulk_close(cls, org, user, tickets, *, force: bool = False):
-        return mailroom.get_client().ticket_close(org, user, tickets, force=force)
+    def bulk_close(cls, org, user, tickets):
+        return cls._bulk_response(mailroom.get_client().ticket_close(org, user, tickets), tickets)
 
     @classmethod
     def bulk_reopen(cls, org, user, tickets):
-        return mailroom.get_client().ticket_reopen(org, user, tickets)
+        return cls._bulk_response(mailroom.get_client().ticket_reopen(org, user, tickets), tickets)
+
+    @classmethod
+    def _bulk_response(self, resp: dict, tickets: list) -> list:
+        return [t for t in tickets if t.id in resp["changed_ids"]]
 
     @classmethod
     def get_allowed_assignees(cls, org):

temba/tickets/tests/test_ticket.py

@@ -46,9 +46,9 @@ def test_model(self, mr_mocks):
         self.assertEqual([call(self.org, self.admin, [ticket], topic)], mr_mocks.calls["ticket_change_topic"])
 
         # test bulk closing
-        Ticket.bulk_close(self.org, self.admin, [ticket], force=True)
+        Ticket.bulk_close(self.org, self.admin, [ticket])
 
-        self.assertEqual([call(self.org, self.admin, [ticket], force=True)], mr_mocks.calls["ticket_close"])
+        self.assertEqual([call(self.org, self.admin, [ticket])], mr_mocks.calls["ticket_close"])
 
         # test bulk re-opening
         Ticket.bulk_reopen(self.org, self.admin, [ticket])