Merge pull request #5587 from nyaruka/contact_list_cleanup

Cleanup contact list pages and reduce db hits

temba/contacts/models.py

@@ -495,13 +495,16 @@ def get_or_create(cls, org, user, key: str, name: str = None, value_type=None):
         )
 
     @classmethod
-    def get_fields(cls, org: Org, viewable_by=None):
+    def get_fields(cls, org: Org, featured=None, viewable_by=None):
         """
         Gets the fields for the given org
         """
 
         fields = org.fields.filter(is_active=True, is_proxy=False)
 
+        if featured is not None:
+            fields = fields.filter(show_in_table=featured)
+
         if viewable_by and org.get_user_role(viewable_by) == OrgRole.AGENT:
             fields = fields.exclude(agent_access=cls.ACCESS_NONE)
 
@@ -836,7 +839,7 @@ def get_field_serialized(self, field) -> str:
 
         return value_dict.get(engine_type)
 
-    def get_field_value(self, field):
+    def get_field_value(self, field: ContactField):
         """
         Given the passed in contact field object, returns the value (as a string, decimal, datetime, AdminBoundary)
         for this contact or None.
@@ -858,7 +861,7 @@ def get_field_value(self, field):
             elif field.value_type in [ContactField.TYPE_STATE, ContactField.TYPE_DISTRICT, ContactField.TYPE_WARD]:
                 return AdminBoundary.get_by_path(self.org, string_value)
 
-    def get_field_display(self, field):
+    def get_field_display(self, field: ContactField) -> str:
         """
         Returns the display value for the passed in field, or empty string if None
         """

temba/contacts/templatetags/contacts.py

@@ -24,16 +24,14 @@
 
 
 @register.simple_tag()
-def contact_field(contact, key):
-    field = contact.org.fields.filter(is_active=True, key=key).first()
-    if field is None:
-        return MISSING_VALUE
-
+def contact_field(contact, field):
     value = contact.get_field_display(field)
+
     if value and field.value_type == ContactField.TYPE_DATETIME:
         value = contact.get_field_value(field)
         if value:
-            return mark_safe(f"<temba-date value='{value.isoformat()}' display='date'></temba-date>")
+            display = "timedate" if field.is_proxy else "date"
+            return mark_safe(f"<temba-date value='{value.isoformat()}' display='{display}'></temba-date>")
 
     return value or MISSING_VALUE
 

temba/contacts/templatetags/tests.py

@@ -1,49 +1,65 @@
+from temba.contacts.models import ContactField
 from temba.tests import TembaTest
 
-from .contacts import format_urn, name_or_urn, urn_icon, urn_or_anon
+from . import contacts as tags
 
 
 class ContactsTest(TembaTest):
+    def test_contact_field(self):
+        gender = self.create_field("gender", "Gender", ContactField.TYPE_TEXT)
+        age = self.create_field("age", "Age", ContactField.TYPE_NUMBER)
+        joined = self.create_field("joined", "Joined", ContactField.TYPE_DATETIME)
+        last_seen_on = self.org.fields.get(key="last_seen_on")
+        contact = self.create_contact("Bob", fields={"age": 30, "gender": "M", "joined": "2024-01-01T00:00:00Z"})
+
+        self.assertEqual("M", tags.contact_field(contact, gender))
+        self.assertEqual("30", tags.contact_field(contact, age))
+        self.assertEqual(
+            "<temba-date value='2024-01-01T02:00:00+02:00' display='date'></temba-date>",
+            tags.contact_field(contact, joined),
+        )
+        self.assertEqual("--", tags.contact_field(contact, last_seen_on))
+
     def test_name_or_urn(self):
         contact1 = self.create_contact("", urns=[])
         contact2 = self.create_contact("Ann", urns=[])
         contact3 = self.create_contact("Bob", urns=["tel:+12024561111", "telegram:098761111"])
         contact4 = self.create_contact("", urns=["tel:+12024562222", "telegram:098762222"])
 
-        self.assertEqual("", name_or_urn(contact1, self.org))
-        self.assertEqual("Ann", name_or_urn(contact2, self.org))
-        self.assertEqual("Bob", name_or_urn(contact3, self.org))
-        self.assertEqual("(202) 456-2222", name_or_urn(contact4, self.org))
+        self.assertEqual("", tags.name_or_urn(contact1, self.org))
+        self.assertEqual("Ann", tags.name_or_urn(contact2, self.org))
+        self.assertEqual("Bob", tags.name_or_urn(contact3, self.org))
+        self.assertEqual("(202) 456-2222", tags.name_or_urn(contact4, self.org))
 
         with self.anonymous(self.org):
-            self.assertEqual(f"{contact1.id:010}", name_or_urn(contact1, self.org))
-            self.assertEqual("Ann", name_or_urn(contact2, self.org))
-            self.assertEqual("Bob", name_or_urn(contact3, self.org))
-            self.assertEqual(f"{contact4.id:010}", name_or_urn(contact4, self.org))
+            self.assertEqual(f"{contact1.id:010}", tags.name_or_urn(contact1, self.org))
+            self.assertEqual("Ann", tags.name_or_urn(contact2, self.org))
+            self.assertEqual("Bob", tags.name_or_urn(contact3, self.org))
+            self.assertEqual(f"{contact4.id:010}", tags.name_or_urn(contact4, self.org))
 
     def test_urn_or_anon(self):
         contact1 = self.create_contact("Bob", urns=[])
         contact2 = self.create_contact("Uri", urns=["tel:+12024561414", "telegram:098765432"])
 
-        self.assertEqual("--", urn_or_anon(contact1, self.org))
-        self.assertEqual("+1 202-456-1414", urn_or_anon(contact2, self.org))
+        self.assertEqual("--", tags.urn_or_anon(contact1, self.org))
+        self.assertEqual("+1 202-456-1414", tags.urn_or_anon(contact2, self.org))
 
         with self.anonymous(self.org):
-            self.assertEqual(f"{contact1.id:010}", urn_or_anon(contact1, self.org))
-            self.assertEqual(f"{contact2.id:010}", urn_or_anon(contact2, self.org))
+            self.assertEqual(f"{contact1.id:010}", tags.urn_or_anon(contact1, self.org))
+            self.assertEqual(f"{contact2.id:010}", tags.urn_or_anon(contact2, self.org))
 
     def test_urn_icon(self):
         contact = self.create_contact("Uri", urns=["tel:+1234567890", "telegram:098765432", "viber:346376373"])
         tel_urn, tg_urn, viber_urn = contact.urns.order_by("-priority")
 
-        self.assertEqual("icon-phone", urn_icon(tel_urn))
-        self.assertEqual("icon-telegram", urn_icon(tg_urn))
-        self.assertEqual("", urn_icon(viber_urn))
+        self.assertEqual("icon-phone", tags.urn_icon(tel_urn))
+        self.assertEqual("icon-telegram", tags.urn_icon(tg_urn))
+        self.assertEqual("", tags.urn_icon(viber_urn))
 
     def test_format_urn(self):
         contact = self.create_contact("Uri", urns=["tel:+12024561414"])
 
-        self.assertEqual("+1 202-456-1414", format_urn(contact.get_urn(), self.org))
+        self.assertEqual("+1 202-456-1414", tags.format_urn(contact.get_urn(), self.org))
 
         with self.anonymous(self.org):
-            self.assertEqual("••••••••", format_urn(contact.get_urn(), self.org))
+            self.assertEqual("••••••••", tags.format_urn(contact.get_urn(), self.org))

temba/contacts/tests.py

@@ -49,7 +49,7 @@
     ContactURN,
 )
 from .tasks import squash_group_counts
-from .templatetags.contacts import contact_field, msg_status_badge
+from .templatetags.contacts import msg_status_badge
 
 
 class ContactCRUDLTest(CRUDLTestMixin, TembaTest):
@@ -59,8 +59,8 @@ def setUp(self):
         self.country = AdminBoundary.create(osm_id="171496", name="Rwanda", level=0)
         AdminBoundary.create(osm_id="1708283", name="Kigali", level=1, parent=self.country)
 
-        self.create_field("age", "Age", value_type="N")
-        self.create_field("home", "Home", value_type="S", priority=10)
+        self.create_field("age", "Age", value_type="N", show_in_table=True)
+        self.create_field("home", "Home", value_type="S", show_in_table=True, priority=10)
 
         # sample flows don't actually get created by org initialization during tests because there are no users at that
         # point so create them explicitly here, so that we also get the sample groups
@@ -141,7 +141,7 @@ def test_list(self, mr_mocks):
         mr_mocks.contact_search('name != ""', contacts=[])
         self.create_group("No Name", query='name = ""')
 
-        with self.assertNumQueries(15):
+        with self.assertNumQueries(16):
             response = self.client.get(list_url)
 
         self.assertEqual([frank, joe], list(response.context["object_list"]))
@@ -162,7 +162,9 @@ def test_list(self, mr_mocks):
         self.assertEqual(response.context["search"], "age = 18")
         self.assertEqual(response.context["save_dynamic_search"], True)
         self.assertIsNone(response.context["search_error"])
-        self.assertEqual(list(response.context["contact_fields"].values_list("name", flat=True)), ["Home", "Age"])
+        self.assertEqual(
+            [f.name for f in response.context["contact_fields"]], ["Home", "Age", "Last Seen On", "Created On"]
+        )
 
         mr_mocks.contact_search("age = 18", contacts=[frank], total=10020)
 
@@ -3131,21 +3133,6 @@ def test_get_or_create(self):
         self.assertEqual("new_key", field7.key)
         self.assertEqual("New Key", field7.name)  # generated
 
-    def test_contact_templatetag(self):
-        ContactField.get_or_create(
-            self.org, self.admin, "date_joined", name="join date", value_type=ContactField.TYPE_DATETIME
-        )
-
-        self.set_contact_field(self.joe, "first", "Starter")
-        self.set_contact_field(self.joe, "date_joined", "01-01-2022 8:30")
-
-        self.assertEqual(contact_field(self.joe, "first"), "Starter")
-        self.assertEqual(
-            contact_field(self.joe, "date_joined"),
-            "<temba-date value='2022-01-01T08:30:00+02:00' display='date'></temba-date>",
-        )
-        self.assertEqual(contact_field(self.joe, "not_there"), "--")
-
     def test_make_key(self):
         self.assertEqual("first_name", ContactField.make_key("First Name"))
         self.assertEqual("second_name", ContactField.make_key("Second   Name  "))

temba/contacts/views.py

@@ -74,6 +74,7 @@ class ContactListView(SpaMixin, OrgPermsMixin, BulkActionMixin, SmartListView):
     sort_field = None
     sort_direction = None
 
+    search_fields = ("name",)  # so that search box is displayed
     search_error = None
 
     def pre_process(self, request, *args, **kwargs):
@@ -96,13 +97,6 @@ def derive_export_url(self):
         search = quote_plus(self.request.GET.get("search", ""))
         return f"{reverse('contacts.contact_export')}?g={self.group.uuid}&s={search}"
 
-    def derive_refresh(self):
-        # smart groups that are reevaluating should refresh every 2 seconds
-        if self.group.is_smart and self.group.status != ContactGroup.STATUS_READY:
-            return 200000
-
-        return None
-
     def get_queryset(self, **kwargs):
         org = self.request.org
         self.search_error = None
@@ -148,10 +142,16 @@ def get_queryset(self, **kwargs):
 
     def get_context_data(self, **kwargs):
         context = super().get_context_data(**kwargs)
+        org = self.request.org
 
         # prefetch contact URNs
         Contact.bulk_urn_cache_initialize(context["object_list"])
 
+        # get the first 6 featured fields as well as the last seen and created fields
+        featured_fields = ContactField.get_fields(org, featured=True).order_by("-priority", "id")[0:6]
+        proxy_fields = org.fields.filter(key__in=("last_seen_on", "created_on"), is_proxy=True).order_by("-key")
+        context["contact_fields"] = list(featured_fields) + list(proxy_fields)
+
         context["search_error"] = self.search_error
         context["sort_direction"] = self.sort_direction
         context["sort_field"] = self.sort_field
@@ -541,13 +541,6 @@ def build_context_menu(self, menu):
             if self.has_org_perm("contacts.contact_export"):
                 menu.add_modax(_("Export"), "export-contacts", self.derive_export_url(), title=_("Export Contacts"))
 
-        def get_context_data(self, *args, **kwargs):
-            context = super().get_context_data(*args, **kwargs)
-            org = self.request.org
-
-            context["contact_fields"] = ContactField.get_fields(org).order_by("-show_in_table", "-priority", "id")[0:6]
-            return context
-
     class Blocked(ContextMenuMixin, ContactListView):
         title = _("Blocked")
         system_group = ContactGroup.TYPE_DB_BLOCKED

templates/contacts/contact_archived.html

@@ -1,6 +1,6 @@
 {% extends "contacts/contact_list.html" %}
 {% load i18n %}
 
-{% block subtitle %}
-  {% trans "These contacts have been removed from all groups and can be deleted permanently." %}
-{% endblock subtitle %}
+{% block pre-table %}
+  <div class="mb-4">{% trans "These contacts have been removed from all groups and can be deleted permanently." %}</div>
+{% endblock pre-table %}

templates/contacts/contact_blocked.html

@@ -1,6 +1,6 @@
 {% extends "contacts/contact_list.html" %}
 {% load i18n %}
 
-{% block subtitle %}
-  {% trans "All inbound messages from these contacts are ignored. They have also been removed from all groups." %}
-{% endblock subtitle %}
+{% block pre-table %}
+  <div class="mb-4">{% trans "All inbound messages from these contacts are ignored. They have also been removed from all groups." %}</div>
+{% endblock pre-table %}

templates/contacts/contact_filter.html

@@ -1,6 +1,6 @@
 {% extends "contacts/contact_list.html" %}
 {% load smartmin i18n %}
 
-{% block subtitle %}
-  {% if current_group.is_smart %}<div class="font-mono text-sm mt-2 bg-gray-200 text-gray-700 p-3 rounded-lg">{{ current_group.query }}</div>{% endif %}
-{% endblock subtitle %}
+{% block pre-table %}
+  {% if current_group.is_smart %}<div class="mb-4 font-mono text-sm mt-2 bg-gray-200 text-gray-700 p-3 rounded-lg">{{ current_group.query }}</div>{% endif %}
+{% endblock pre-table %}