Revert "Omnibox fixes and cleanup"

This reverts commit 4438b65.

temba/contacts/omnibox.py → temba/contacts/search/omnibox.py

@@ -1,12 +1,14 @@
 import json
+import operator
+from functools import reduce
 
-from django.db.models.functions import Lower
+from django.db.models import Q
+from django.db.models.functions import Upper
 
 from temba import mailroom
+from temba.contacts.models import Contact, ContactGroup, ContactGroupCount
 from temba.utils.models.es import IDSliceQuerySet
 
-from .models import Contact, ContactGroup, ContactGroupCount
-
 SEARCH_ALL_GROUPS = "g"
 SEARCH_STATIC_GROUPS = "s"
 SEARCH_CONTACTS = "c"
@@ -16,71 +18,77 @@ def omnibox_query(org, **kwargs):
     """
     Performs a omnibox query based on the given arguments
     """
-
     # determine what type of group/contact/URN lookup is being requested
     contact_uuids = kwargs.get("c", None)  # contacts with ids
     group_uuids = kwargs.get("g", None)  # groups with ids
     search = kwargs.get("search", None)  # search of groups, contacts and URNs
     types = list(kwargs.get("types", ""))  # limit search to types (g | s | c)
 
     if contact_uuids:
-        return org.contacts.filter(
-            uuid__in=contact_uuids.split(","), status=Contact.STATUS_ACTIVE, is_active=True
-        ).order_by(Lower("name"))
+        return Contact.objects.filter(
+            org=org, status=Contact.STATUS_ACTIVE, is_active=True, uuid__in=contact_uuids.split(",")
+        ).order_by("name")
     elif group_uuids:
-        return ContactGroup.get_groups(org).filter(uuid__in=group_uuids.split(",")).order_by(Lower("name"))
+        return ContactGroup.get_groups(org).filter(uuid__in=group_uuids.split(",")).order_by("name")
 
     # searching returns something which acts enough like a queryset to be paged
-    return _omnibox_mixed_search(org, search, types)
+    return omnibox_mixed_search(org, search, types)
+
+
+def term_search(queryset, fields, terms):
+    term_queries = []
+    for term in terms:
+        field_queries = []
+        for field in fields:
+            field_queries.append(Q(**{field: term}))
+        term_queries.append(reduce(operator.or_, field_queries))
 
+    return queryset.filter(reduce(operator.and_, term_queries))
 
-def _omnibox_mixed_search(org, query: str, types: str):
+
+def omnibox_mixed_search(org, query, types):
     """
     Performs a mixed group and contact search, returning the first N matches of each type.
     """
-
+    query_terms = query.split(" ") if query else None
     search_types = types or (SEARCH_ALL_GROUPS, SEARCH_CONTACTS)
     per_type_limit = 25
     results = []
 
     if SEARCH_ALL_GROUPS in search_types or SEARCH_STATIC_GROUPS in search_types:
         groups = ContactGroup.get_groups(org, ready_only=True)
-        query_terms = query.split(" ") if query else ()
 
         # exclude dynamic groups if not searching all groups
         if SEARCH_ALL_GROUPS not in search_types:
             groups = groups.filter(query=None)
 
-        for query_term in query_terms:
-            groups = groups.filter(name__icontains=query_term)
+        if query:
+            groups = term_search(groups, ("name__icontains",), query_terms)
 
-        results += list(groups.order_by(Lower("name"))[:per_type_limit])
+        results += list(groups.order_by(Upper("name"))[:per_type_limit])
 
     if SEARCH_CONTACTS in search_types:
-        if query:
-            search_query = f"name ~ {json.dumps(query)}"
-            if not org.is_anon:
-                search_query += f" OR urn ~ {json.dumps(query)}"
-        else:
-            search_query = ""
-
         try:
+            if org.is_anon:
+                search_query = f"name ~ {json.dumps(query)}"
+            else:
+                search_query = f"name ~ {json.dumps(query)} OR urn ~ {json.dumps(query)}"
             search_results = mailroom.get_client().contact_search(
-                org, org.active_contacts_group, search_query, sort="name", limit=per_type_limit
+                org, org.active_contacts_group, search_query, sort="name"
             )
+            contacts = IDSliceQuerySet(
+                Contact,
+                search_results.contact_ids,
+                offset=0,
+                total=len(search_results.contact_ids),
+                only=("id", "uuid", "name", "org_id"),
+            ).prefetch_related("org")
+
+            results += list(contacts[:per_type_limit])
+            Contact.bulk_urn_cache_initialize(contacts=results)
+
         except mailroom.QueryValidationException:
-            return results
-
-        contacts = IDSliceQuerySet(
-            Contact,
-            search_results.contact_ids,
-            offset=0,
-            total=len(search_results.contact_ids),
-            only=("id", "uuid", "name", "org_id"),
-        ).prefetch_related("org")
-
-        Contact.bulk_urn_cache_initialize(contacts=results)
-        results += list(contacts)
+            pass
 
     return results
 

temba/contacts/tests.py

@@ -2137,7 +2137,9 @@ def omnibox_request(query):
         self.assertEqual([], omnibox_request("search=-123`213"))
 
         with self.assertNumQueries(14):
-            mr_mocks.contact_search(query="", total=4, contacts=[self.billy, self.frank, self.joe, self.voldemort])
+            mr_mocks.contact_search(
+                query="name ~ null OR urn ~ null", total=4, contacts=[self.billy, self.frank, self.joe, self.voldemort]
+            )
 
             self.assertEqual(
                 [
@@ -2168,7 +2170,7 @@ def omnibox_request(query):
             )
 
         with self.assertNumQueries(14):
-            mr_mocks.contact_search(query="", total=2, contacts=[self.billy, self.frank])
+            mr_mocks.contact_search(query="name ~ null OR urn ~ null", total=2, contacts=[self.billy, self.frank])
 
             self.assertEqual(
                 [
@@ -2213,7 +2215,7 @@ def omnibox_request(query):
         )
 
         with self.anonymous(self.org):
-            mr_mocks.contact_search(query="", total=1, contacts=[self.billy])
+            mr_mocks.contact_search(query="name ~ null", total=1, contacts=[self.billy])
 
             self.assertEqual(
                 [

temba/contacts/views.py

@@ -53,7 +53,7 @@
 
 from .forms import ContactGroupForm, UpdateContactForm
 from .models import URN, Contact, ContactExport, ContactField, ContactGroup, ContactGroupCount, ContactImport
-from .omnibox import omnibox_query, omnibox_results_to_dict
+from .search.omnibox import omnibox_query, omnibox_results_to_dict
 
 logger = logging.getLogger(__name__)
 
@@ -331,7 +331,9 @@ def render_to_response(self, context, **response_kwargs):
 
             results = omnibox_results_to_dict(org, object_list)
 
-            return JsonResponse({"results": results, "more": page.has_next(), "total": len(results), "err": "nil"})
+            json_result = {"results": results, "more": page.has_next(), "total": len(results), "err": "nil"}
+
+            return HttpResponse(json.dumps(json_result), content_type="application/json")
 
     class Read(SpaMixin, OrgObjPermsMixin, ContentMenuMixin, SmartReadView):
         slug_url_kwarg = "uuid"

temba/mailroom/client/client.py

@@ -108,7 +108,7 @@ def contact_parse_query(self, org, query: str, parse_only: bool = False) -> Pars
 
         return ParsedQuery(query=resp["query"], metadata=QueryMetadata(**resp.get("metadata", {})))
 
-    def contact_search(self, org, group, query: str, sort: str, offset=0, limit=50, exclude_ids=()) -> SearchResults:
+    def contact_search(self, org, group, query: str, sort: str, offset=0, exclude_ids=()) -> SearchResults:
         resp = self._request(
             "contact/search",
             {
@@ -118,7 +118,6 @@ def contact_search(self, org, group, query: str, sort: str, offset=0, limit=50,
                 "query": query,
                 "sort": sort,
                 "offset": offset,
-                "limit": limit,
             },
         )
 

temba/tests/mailroom.py

@@ -265,7 +265,7 @@ def contact_parse_query(self, org, query: str, parse_only: bool = False):
         return mailroom.ParsedQuery(query=query, metadata=mock_inspect_query(org, query))
 
     @_client_method
-    def contact_search(self, org, group, query: str, sort: str, offset=0, limit=50, exclude_ids=()):
+    def contact_search(self, org, group, query, sort, offset=0, exclude_ids=()):
         mock = self.mocks._contact_search.get(query or "")
 
         assert mock, f"missing contact_search mock for query '{query}'"

temba/triggers/tests.py

@@ -8,7 +8,7 @@
 
 from temba.channels.models import Channel
 from temba.contacts.models import ContactGroup
-from temba.contacts.omnibox import omnibox_serialize
+from temba.contacts.search.omnibox import omnibox_serialize
 from temba.flows.models import Flow
 from temba.schedules.models import Schedule
 from temba.tests import CRUDLTestMixin, TembaTest

temba/triggers/types.py

@@ -6,7 +6,7 @@
 
 from temba.channels.models import Channel
 from temba.contacts.models import ContactURN
-from temba.contacts.omnibox import omnibox_deserialize
+from temba.contacts.search.omnibox import omnibox_deserialize
 from temba.flows.models import Flow
 from temba.schedules.views import ScheduleFormMixin
 from temba.utils.fields import JSONField, OmniboxChoice, SelectWidget, TembaChoiceField

temba/triggers/views.py

@@ -11,7 +11,7 @@
 from temba.channels.models import Channel
 from temba.channels.types.android import AndroidType
 from temba.contacts.models import ContactGroup, ContactURN
-from temba.contacts.omnibox import omnibox_serialize
+from temba.contacts.search.omnibox import omnibox_serialize
 from temba.flows.models import Flow
 from temba.formax import FormaxMixin
 from temba.msgs.views import ModalMixin