Merge pull request #5607 from nyaruka/ticket_counts_v2_pt5

Stop writing old ticket counts

temba/sql/current_functions.sql

@@ -1,4 +1,4 @@
--- Generated by collect_sql on 2024-10-30 20:32 UTC
+-- Generated by collect_sql on 2024-10-31 21:17 UTC
 
 ----------------------------------------------------------------------
 -- Convenience method to call contact_toggle_system_group with a row
@@ -394,26 +394,6 @@ BEGIN
 END;
 $$ LANGUAGE plpgsql;
 
-----------------------------------------------------------------------
--- Inserts a new assignee ticketcount row with the given values
-----------------------------------------------------------------------
-CREATE OR REPLACE FUNCTION temba_insert_ticketcount_for_assignee(_org_id INTEGER, _assignee_id INTEGER, status CHAR(1), _count INT) RETURNS VOID AS $$
-BEGIN
-    INSERT INTO tickets_ticketcount("org_id", "scope", "status", "count", "is_squashed")
-    VALUES(_org_id, format('assignee:%s', coalesce(_assignee_id, 0)), status, _count, FALSE);
-END;
-$$ LANGUAGE plpgsql;
-
-----------------------------------------------------------------------
--- Inserts a new topic ticketcount row with the given values
-----------------------------------------------------------------------
-CREATE OR REPLACE FUNCTION temba_insert_ticketcount_for_topic(_org_id INTEGER, _topic_id INTEGER, status CHAR(1), _count INT) RETURNS VOID AS $$
-BEGIN
-    INSERT INTO tickets_ticketcount("org_id", "scope", "status", "count", "is_squashed")
-    VALUES(_org_id, format('topic:%s', _topic_id), status, _count, FALSE);
-END;
-$$ LANGUAGE plpgsql;
-
 ----------------------------------------------------------------------
 -- Handles DELETE statements on ivr_call table
 ----------------------------------------------------------------------
@@ -654,37 +634,21 @@ END;
 $$ LANGUAGE plpgsql;
 
 ----------------------------------------------------------------------
--- Trigger procedure to update user and system labels on column changes
+-- Trigger procedure to update contact ticket counts
 ----------------------------------------------------------------------
 CREATE OR REPLACE FUNCTION temba_ticket_on_change() RETURNS TRIGGER AS $$
 BEGIN
   IF TG_OP = 'INSERT' THEN -- new ticket inserted
-    PERFORM temba_insert_ticketcount_for_assignee(NEW.org_id, NEW.assignee_id, NEW.status, 1);
-    PERFORM temba_insert_ticketcount_for_topic(NEW.org_id, NEW.topic_id, NEW.status, 1);
-
     IF NEW.status = 'O' THEN
       UPDATE contacts_contact SET ticket_count = ticket_count + 1, modified_on = NOW() WHERE id = NEW.contact_id;
     END IF;
   ELSIF TG_OP = 'UPDATE' THEN -- existing ticket updated
-    IF OLD.assignee_id IS DISTINCT FROM NEW.assignee_id OR OLD.status != NEW.status THEN
-      PERFORM temba_insert_ticketcount_for_assignee(OLD.org_id, OLD.assignee_id, OLD.status, -1);
-      PERFORM temba_insert_ticketcount_for_assignee(NEW.org_id, NEW.assignee_id, NEW.status, 1);
-    END IF;
-
-    IF OLD.topic_id != NEW.topic_id OR OLD.status != NEW.status THEN
-      PERFORM temba_insert_ticketcount_for_topic(OLD.org_id, OLD.topic_id, OLD.status, -1);
-      PERFORM temba_insert_ticketcount_for_topic(NEW.org_id, NEW.topic_id, NEW.status, 1);
-    END IF;
-
     IF OLD.status = 'O' AND NEW.status = 'C' THEN -- ticket closed
       UPDATE contacts_contact SET ticket_count = ticket_count - 1, modified_on = NOW() WHERE id = OLD.contact_id;
     ELSIF OLD.status = 'C' AND NEW.status = 'O' THEN -- ticket reopened
       UPDATE contacts_contact SET ticket_count = ticket_count + 1, modified_on = NOW() WHERE id = OLD.contact_id;
     END IF;
   ELSIF TG_OP = 'DELETE' THEN -- existing ticket deleted
-    PERFORM temba_insert_ticketcount_for_assignee(OLD.org_id, OLD.assignee_id, OLD.status, -1);
-    PERFORM temba_insert_ticketcount_for_topic(OLD.org_id, OLD.topic_id, OLD.status, -1);
-
     IF OLD.status = 'O' THEN -- open ticket deleted
       UPDATE contacts_contact SET ticket_count = ticket_count - 1, modified_on = NOW() WHERE id = OLD.contact_id;
     END IF;

temba/sql/current_triggers.sql

@@ -1,4 +1,4 @@
--- Generated by collect_sql on 2024-10-30 20:32 UTC
+-- Generated by collect_sql on 2024-10-31 21:17 UTC
 
 CREATE TRIGGER temba_broadcast_on_delete
 AFTER DELETE ON msgs_broadcast REFERENCING OLD TABLE AS oldtab

temba/tickets/migrations/0071_backfill_item_counts.py

@@ -4,7 +4,7 @@
 from django.db.models import Count
 
 
-def backfill_item_counts(apps, schema_editor):
+def backfill_item_counts(apps, schema_editor):  # pragma: no cover
     Org = apps.get_model("orgs", "Org")
 
     for org in Org.objects.all():

temba/tickets/migrations/0072_drop_old_triggers.py

@@ -0,0 +1,39 @@
+# Generated by Django 5.1.2 on 2024-10-31 21:11
+
+from django.db import migrations
+
+SQL = """
+----------------------------------------------------------------------
+-- Trigger procedure to update contact ticket counts
+----------------------------------------------------------------------
+CREATE OR REPLACE FUNCTION temba_ticket_on_change() RETURNS TRIGGER AS $$
+BEGIN
+  IF TG_OP = 'INSERT' THEN -- new ticket inserted
+    IF NEW.status = 'O' THEN
+      UPDATE contacts_contact SET ticket_count = ticket_count + 1, modified_on = NOW() WHERE id = NEW.contact_id;
+    END IF;
+  ELSIF TG_OP = 'UPDATE' THEN -- existing ticket updated
+    IF OLD.status = 'O' AND NEW.status = 'C' THEN -- ticket closed
+      UPDATE contacts_contact SET ticket_count = ticket_count - 1, modified_on = NOW() WHERE id = OLD.contact_id;
+    ELSIF OLD.status = 'C' AND NEW.status = 'O' THEN -- ticket reopened
+      UPDATE contacts_contact SET ticket_count = ticket_count + 1, modified_on = NOW() WHERE id = OLD.contact_id;
+    END IF;
+  ELSIF TG_OP = 'DELETE' THEN -- existing ticket deleted
+    IF OLD.status = 'O' THEN -- open ticket deleted
+      UPDATE contacts_contact SET ticket_count = ticket_count - 1, modified_on = NOW() WHERE id = OLD.contact_id;
+    END IF;
+  END IF;
+  RETURN NULL;
+END;
+$$ LANGUAGE plpgsql;
+
+DROP FUNCTION temba_insert_ticketcount_for_assignee(INTEGER, INTEGER, CHAR(1), INT);
+DROP FUNCTION temba_insert_ticketcount_for_topic(INTEGER, INTEGER, CHAR(1), INT);
+"""
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [("tickets", "0071_backfill_item_counts")]
+
+    operations = [migrations.RunSQL(SQL)]

temba/tickets/models.py

@@ -429,84 +429,18 @@ def get_queryset(self, org, user, *, ordered: bool):
 
 class TicketCount(SquashableModel):
     """
-    Counts of tickets by assignment/topic and status
+    TODO drop
     """
 
-    squash_over = ("org_id", "scope", "status")
-
     org = models.ForeignKey(Org, on_delete=models.PROTECT, related_name="ticket_counts")
     scope = models.CharField(max_length=32)
     status = models.CharField(max_length=1, choices=Ticket.STATUS_CHOICES)
     count = models.IntegerField(default=0)
 
-    @classmethod
-    def get_squash_query(cls, distinct_set) -> tuple:
-        sql = """
-        WITH removed as (
-            DELETE FROM %(table)s WHERE "org_id" = %%s AND "scope" = %%s AND "status" = %%s RETURNING "count"
-        )
-        INSERT INTO %(table)s("org_id", "scope", "status", "count", "is_squashed")
-        VALUES (%%s, %%s, %%s, GREATEST(0, (SELECT SUM("count") FROM removed)), TRUE);
-        """ % {
-            "table": cls._meta.db_table
-        }
-
-        params = (distinct_set.org_id, distinct_set.scope, distinct_set.status) * 2
-
-        return sql, params
-
-    @classmethod
-    def get_by_assignees(cls, org, assignees: list, status: str) -> dict:
-        """
-        Gets counts for a set of assignees (None means no assignee)
-        """
-
-        scopes = [cls._assignee_scope(a) for a in assignees]
-        counts = (
-            cls.objects.filter(org=org, scope__in=scopes, status=status)
-            .values_list("scope")
-            .annotate(count_sum=Sum("count"))
-        )
-        counts_by_scope = {c[0]: c[1] for c in counts}
-
-        return {a: counts_by_scope.get(cls._assignee_scope(a), 0) for a in assignees}
-
-    @classmethod
-    def get_by_topics(cls, org, topics: list, status: str) -> dict:
-        """
-        Gets counts for a set of topics
-        """
-
-        scopes = [cls._topic_scope(t) for t in topics]
-        counts = (
-            cls.objects.filter(org=org, scope__in=scopes, status=status)
-            .values_list("scope")
-            .annotate(count_sum=Sum("count"))
-        )
-        counts_by_scope = {c[0]: c[1] for c in counts}
-
-        return {t: counts_by_scope.get(cls._topic_scope(t), 0) for t in topics}
-
-    @classmethod
-    def get_all(cls, org, status: str) -> int:
-        """
-        Gets count for org and status regardless of assignee
-        """
-        return cls.sum(cls.objects.filter(org=org, scope__startswith="assignee:", status=status))
-
-    @staticmethod
-    def _assignee_scope(user) -> str:
-        return f"assignee:{user.id if user else 0}"
-
-    @staticmethod
-    def _topic_scope(topic) -> str:
-        return f"topic:{topic.id}"
-
     class Meta:
         indexes = [
             models.Index(fields=("org", "status")),
             models.Index(fields=("org", "scope", "status")),
-            # for squashing task
             models.Index(
                 name="ticket_count_unsquashed", fields=("org", "scope", "status"), condition=Q(is_squashed=False)
             ),

temba/tickets/tasks.py

@@ -1,10 +1,9 @@
 from temba.utils.crons import cron_task
 
-from .models import TicketCount, TicketDailyCount, TicketDailyTiming
+from .models import TicketDailyCount, TicketDailyTiming
 
 
 @cron_task(lock_timeout=7200)
 def squash_ticket_counts():
-    TicketCount.squash()
     TicketDailyCount.squash()
     TicketDailyTiming.squash()

temba/tickets/tests.py

@@ -11,15 +11,14 @@
 from temba.contacts.models import Contact, ContactField, ContactURN
 from temba.orgs.models import Export, Org, OrgMembership, OrgRole
 from temba.orgs.tasks import squash_item_counts
-from temba.tests import CRUDLTestMixin, MigrationTest, TembaTest, matchers, mock_mailroom
+from temba.tests import CRUDLTestMixin, TembaTest, matchers, mock_mailroom
 from temba.utils.dates import datetime_to_timestamp
 from temba.utils.uuid import uuid4
 
 from .models import (
     Shortcut,
     Team,
     Ticket,
-    TicketCount,
     TicketDailyCount,
     TicketDailyTiming,
     TicketEvent,
@@ -107,9 +106,6 @@ def assert_counts(
             all_topics = org.topics.filter(is_active=True)
             assignees = [None] + list(Ticket.get_allowed_assignees(org))
 
-            self.assertEqual(assignee_open, TicketCount.get_by_assignees(org, assignees, Ticket.STATUS_OPEN))
-            self.assertEqual(assignee_closed, TicketCount.get_by_assignees(org, assignees, Ticket.STATUS_CLOSED))
-
             self.assertEqual(
                 assignee_open, {u: Ticket.get_assignee_count(org, u, all_topics, Ticket.STATUS_OPEN) for u in assignees}
             )
@@ -118,17 +114,11 @@ def assert_counts(
                 {u: Ticket.get_assignee_count(org, u, all_topics, Ticket.STATUS_CLOSED) for u in assignees},
             )
 
-            self.assertEqual(sum(assignee_open.values()), TicketCount.get_all(org, Ticket.STATUS_OPEN))
-            self.assertEqual(sum(assignee_closed.values()), TicketCount.get_all(org, Ticket.STATUS_CLOSED))
-
             self.assertEqual(sum(assignee_open.values()), Ticket.get_status_count(org, all_topics, Ticket.STATUS_OPEN))
             self.assertEqual(
                 sum(assignee_closed.values()), Ticket.get_status_count(org, all_topics, Ticket.STATUS_CLOSED)
             )
 
-            self.assertEqual(topic_open, TicketCount.get_by_topics(org, list(org.topics.all()), Ticket.STATUS_OPEN))
-            self.assertEqual(topic_closed, TicketCount.get_by_topics(org, list(org.topics.all()), Ticket.STATUS_CLOSED))
-
             self.assertEqual(topic_open, Ticket.get_topic_counts(org, list(org.topics.all()), Ticket.STATUS_OPEN))
             self.assertEqual(topic_closed, Ticket.get_topic_counts(org, list(org.topics.all()), Ticket.STATUS_CLOSED))
 
@@ -220,7 +210,7 @@ def assert_counts(
             contacts={contact1: 2, contact2: 2},
         )
 
-        squash_ticket_counts()  # shouldn't change counts
+        squash_item_counts()  # shouldn't change counts
 
         assert_counts(
             self.org,
@@ -1821,7 +1811,7 @@ def assert_timings():
 
         assert_timings()
 
-        TicketDailyTiming.squash()
+        squash_ticket_counts()
 
         assert_timings()
 
@@ -1846,37 +1836,3 @@ def _record_last_close(self, org, d: date, seconds: int, undo: bool = False):
         TicketDailyTiming.objects.create(
             count_type=TicketDailyTiming.TYPE_LAST_CLOSE, scope=f"o:{org.id}", day=d, count=count, seconds=seconds
         )
-
-
-class BackfillItemCountsTest(MigrationTest):
-    app = "tickets"
-    migrate_from = "0070_update_triggers"
-    migrate_to = "0071_backfill_item_counts"
-
-    def setUpBeforeMigration(self, apps):
-        self.general = self.org.default_ticket_topic
-        self.cats = Topic.create(self.org, self.admin, "Cats")
-
-        contact1 = self.create_contact("Bob", urns=["twitter:bobby"])
-        contact2 = self.create_contact("Jim", urns=["twitter:jimmy"])
-
-        org2_general = self.org2.default_ticket_topic
-        org2_contact = self.create_contact("Bob", urns=["twitter:bobby"], org=self.org2)
-
-        self.create_ticket(contact1, topic=self.general, assignee=self.admin)
-        self.create_ticket(contact2, topic=self.general, assignee=self.admin)
-        self.create_ticket(contact1, topic=self.general, assignee=self.admin, closed_on=timezone.now())
-        self.create_ticket(contact2, topic=self.cats, assignee=self.agent)
-        self.create_ticket(contact1, topic=self.cats)
-        self.create_ticket(org2_contact, topic=org2_general)
-
-    def test_migration(self):
-        self.assertEqual(
-            {
-                f"tickets:C:{self.general.id}:{self.admin.id}": 1,
-                f"tickets:O:{self.general.id}:{self.admin.id}": 2,
-                f"tickets:O:{self.cats.id}:0": 1,
-                f"tickets:O:{self.cats.id}:{self.agent.id}": 1,
-            },
-            {c["scope"]: c["count"] for c in self.org.counts.order_by("scope").values("scope", "count")},
-        )