From 282d82f289bcdac363e3e892a07e0485017d8c7b Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Mon, 28 Oct 2024 11:32:18 +0000 Subject: [PATCH] fix(sales-dashboard): prevent 500 error when user doesn't exist on sales dashboard search (#4757) --- api/sales_dashboard/views.py | 28 ++++++++++--- .../test_unit_sales_dashboard_views.py | 39 +++++++++++++++++++ 2 files changed, 61 insertions(+), 6 deletions(-) diff --git a/api/sales_dashboard/views.py b/api/sales_dashboard/views.py index cba108f53041..5aabf7b871c8 100644 --- a/api/sales_dashboard/views.py +++ b/api/sales_dashboard/views.py @@ -33,6 +33,7 @@ from organisations.models import ( Organisation, OrganisationSubscriptionInformationCache, + UserOrganisation, ) from organisations.tasks import ( update_organisation_subscription_information_cache, @@ -53,6 +54,7 @@ DEFAULT_ORGANISATION_SORT_DIRECTION = "DESC" email_regex = re.compile(r"^[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,}$") +domain_regex = re.compile(r"^[a-z0-9.-]+\.[a-z]{2,}$") @method_decorator( @@ -126,14 +128,28 @@ 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() + search_term = search_term.lower() + if email_regex.match(search_term) and ( + user := FFAdminUser.objects.filter(email__iexact=search_term).first() + ): return Q(id__in=user.organisations.values_list("id", flat=True)) + else: + query = Q() - return Q(name__icontains=search_term) | Q( - subscription__subscription_id=search_term - ) + if domain_regex.match(search_term): + matching_users = FFAdminUser.objects.filter( + email__iendswith=search_term + ) + org_ids = UserOrganisation.objects.filter( + user__in=matching_users + ).values_list("organisation_id", flat=True) + query = query & Q(id__in=org_ids) + + return ( + query + | Q(name__icontains=search_term) + | Q(subscription__subscription_id__iexact=search_term) + ) @staff_member_required diff --git a/api/tests/unit/sales_dashboard/test_unit_sales_dashboard_views.py b/api/tests/unit/sales_dashboard/test_unit_sales_dashboard_views.py index c42e8ac8ca6e..163f101c638a 100644 --- a/api/tests/unit/sales_dashboard/test_unit_sales_dashboard_views.py +++ b/api/tests/unit/sales_dashboard/test_unit_sales_dashboard_views.py @@ -128,6 +128,45 @@ def test_list_organisations_search_by_user_email( assert list(response.context_data["organisation_list"]) == [organisation] +def test_list_organisations_search_by_user_email_for_non_existent_user( + organisation: Organisation, + superuser_client: Client, +) -> None: + # Given + domain = "bar.com" + user = FFAdminUser.objects.create(email=f"foo@{domain}") + user.add_organisation(organisation) + search_term = f"baz@{domain}" + + url = "%s?search=%s" % (reverse("sales_dashboard:index"), search_term) + + # When + response = superuser_client.get(url) + + # Then + assert response.status_code == 200 + assert list(response.context_data["organisation_list"]) == [] + + +def test_list_organisations_search_by_domain( + organisation: Organisation, + superuser_client: Client, +) -> None: + # Given + domain = "bar.com" + user = FFAdminUser.objects.create(email=f"foo@{domain}") + user.add_organisation(organisation) + + url = "%s?search=%s" % (reverse("sales_dashboard:index"), domain) + + # When + response = superuser_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,