chore(sales-dashboard): optimise sales dashboard search query (#4377)

api/sales_dashboard/views.py

@@ -1,5 +1,6 @@
 import json
 
+import re2 as re
 from app_analytics.influxdb_wrapper import (
     get_event_list_for_organisation,
     get_events_for_organisation,
@@ -34,6 +35,7 @@
     update_organisation_subscription_information_cache,
     update_organisation_subscription_information_influx_cache,
 )
+from users.models import FFAdminUser
 
 from .forms import (
     EmailUsageForm,
@@ -47,6 +49,8 @@
 DEFAULT_ORGANISATION_SORT = "subscription_information_cache__api_calls_30d"
 DEFAULT_ORGANISATION_SORT_DIRECTION = "DESC"
 
+email_regex = re.compile(r"^[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,}$")
+
 
 class OrganisationList(ListView):
     model = Organisation
@@ -65,13 +69,8 @@ def get_queryset(self):
             ),
         ).select_related("subscription", "subscription_information_cache")
 
-        if self.request.GET.get("search"):
-            search_term = self.request.GET["search"]
-            queryset = queryset.filter(
-                Q(name__icontains=search_term)
-                | Q(users__email__icontains=search_term)
-                | Q(subscription__subscription_id=search_term)
-            )
+        if search_term := self.request.GET.get("search"):
+            queryset = queryset.filter(self._build_search_query(search_term))
 
         if self.request.GET.get("filter_plan"):
             filter_plan = self.request.GET["filter_plan"]
@@ -119,6 +118,16 @@ def get_context_data(self, **kwargs):
 
         return data
 
+    def _build_search_query(self, search_term: str) -> Q:
+        if email_regex.match(search_term.lower()):
+            # Assume that the search is for the email of a given user
+            user = FFAdminUser.objects.filter(email__iexact=search_term).first()
+            return Q(id__in=user.organisations.values_list("id", flat=True))
+
+        return Q(name__icontains=search_term) | Q(
+            subscription__subscription_id=search_term
+        )
+
 
 @staff_member_required
 def organisation_info(request: HttpRequest, organisation_id: int) -> HttpResponse:

api/tests/unit/sales_dashboard/test_unit_sales_dashboard_views.py

@@ -1,5 +1,5 @@
 import pytest
-from django.test import RequestFactory
+from django.test import Client, RequestFactory
 from django.urls import reverse
 from pytest_django.fixtures import SettingsWrapper
 from pytest_mock import MockerFixture
@@ -8,8 +8,10 @@
 from organisations.models import (
     Organisation,
     OrganisationSubscriptionInformationCache,
+    Subscription,
 )
 from sales_dashboard.views import OrganisationList
+from users.models import FFAdminUser
 
 
 @pytest.mark.parametrize(
@@ -21,14 +23,15 @@ def test_organisation_subscription_get_api_call_overage(
     allowed_calls_30d: int,
     actual_calls_30d: int,
     expected_overage: int,
+    rf: RequestFactory,
 ) -> None:
     OrganisationSubscriptionInformationCache.objects.create(
         organisation=organisation,
         allowed_30d_api_calls=allowed_calls_30d,
         api_calls_30d=actual_calls_30d,
     )
 
-    request = RequestFactory().get("/sales-dashboard")
+    request = rf.get("/sales-dashboard")
     view = OrganisationList()
     view.request = request
     result = view.get_queryset().get(pk=organisation.id)
@@ -63,3 +66,84 @@ def test_get_organisation_info__get_event_list_for_organisation(
     assert "label1" in str(response.content)
     assert "label2" in str(response.content)
     event_list_mock.assert_called_once_with(organisation.id, "-180d", "now()")
+
+
+def test_list_organisations_search_by_name(
+    organisation: Organisation,
+    client: Client,
+    admin_user: FFAdminUser,
+) -> None:
+    # Given
+    # use the truncated organisation name to ensure fuzzy search works
+    search_term = organisation.name[1:-1]
+
+    url = "%s?search=%s" % (reverse("sales_dashboard:index"), search_term)
+    client.force_login(admin_user)
+
+    # When
+    response = client.get(url)
+
+    # Then
+    assert response.status_code == 200
+
+    assert list(response.context_data["organisation_list"]) == [organisation]
+
+
+def test_list_organisations_search_by_subscription_id(
+    organisation: Organisation,
+    chargebee_subscription: Subscription,
+    client: Client,
+    admin_user: FFAdminUser,
+) -> None:
+    # Given
+    search_term = chargebee_subscription.subscription_id
+
+    url = "%s?search=%s" % (reverse("sales_dashboard:index"), search_term)
+    client.force_login(admin_user)
+
+    # When
+    response = client.get(url)
+
+    # Then
+    assert response.status_code == 200
+    assert list(response.context_data["organisation_list"]) == [organisation]
+
+
+def test_list_organisations_search_by_user_email(
+    organisation: Organisation,
+    client: Client,
+    admin_user: FFAdminUser,
+) -> None:
+    # Given
+    search_term = admin_user.email
+
+    url = "%s?search=%s" % (reverse("sales_dashboard:index"), search_term)
+    client.force_login(admin_user)
+
+    # When
+    response = client.get(url)
+
+    # Then
+    assert response.status_code == 200
+    assert list(response.context_data["organisation_list"]) == [organisation]
+
+
+def test_list_organisations_filter_plan(
+    organisation: Organisation,
+    chargebee_subscription: Subscription,
+    client: Client,
+    admin_user: FFAdminUser,
+) -> None:
+    # Given
+    url = "%s?filter_plan=%s" % (
+        reverse("sales_dashboard:index"),
+        chargebee_subscription.plan,
+    )
+    client.force_login(admin_user)
+
+    # When
+    response = client.get(url)
+
+    # Then
+    assert response.status_code == 200
+    assert list(response.context_data["organisation_list"]) == [organisation]