From 05aa8f98bd568e243c21d3a610d215fe946da872 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Tue, 14 Jan 2025 08:30:19 -0800 Subject: [PATCH] Move the domain report to metabase This report is timing out. It won't timeout on metabase and it will handle slow loading reports a bit better with caching and other features. --- adserver/reports.py | 13 ------ .../adserver/reports/advertiser-domain.html | 28 +++++------- adserver/tests/test_reports.py | 44 ++----------------- adserver/views.py | 29 ++---------- config/settings/base.py | 1 + 5 files changed, 18 insertions(+), 97 deletions(-) diff --git a/adserver/reports.py b/adserver/reports.py index da31a945..9823a7c2 100644 --- a/adserver/reports.py +++ b/adserver/reports.py @@ -8,7 +8,6 @@ from .constants import PAID_CAMPAIGN from .models import AdImpression from .models import AdvertiserImpression -from .models import DomainImpression from .models import GeoImpression from .models import KeywordImpression from .models import PlacementImpression @@ -192,18 +191,6 @@ class AdvertiserPublisherReport(AdvertiserReport): select_related_fields = ("advertisement", "advertisement__flight", "publisher") -class AdvertiserDomainReport(AdvertiserReport): - """Report to breakdown advertiser performance by domain where the ad appears.""" - - model = DomainImpression - index = "domain" - order = "-views" - select_related_fields = ("advertisement", "advertisement__flight") - - def get_index_header(self): - return self.index.title() - - class PublisherReport(BaseReport): """Report for showing daily ad performance for a publisher.""" diff --git a/adserver/templates/adserver/reports/advertiser-domain.html b/adserver/templates/adserver/reports/advertiser-domain.html index 03c1aa2c..25814461 100644 --- a/adserver/templates/adserver/reports/advertiser-domain.html +++ b/adserver/templates/adserver/reports/advertiser-domain.html @@ -1,6 +1,7 @@ {% extends "adserver/reports/advertiser.html" %} {% load humanize %} {% load i18n %} +{% load metabase %} {% block title %}{% trans 'Advertiser Domain Report' %} - {{ advertiser }}{% endblock %} @@ -16,31 +17,24 @@ {% endblock breadcrumbs %} -{% block additional_filters %} -{{ block.super }} - -
- - -
- -{% endblock additional_filters %} - - {% block explainer %}

{% trans 'About this report' %}

{% trans 'This report shows the top domains where your ads are shown.' %}

- {% blocktrans %}This report shows the top {{ limit }} domains and updates daily. All previous days data is complete.{% endblocktrans %} + {% blocktrans %}This report shows the top 20 domains and updates daily. All previous days data is complete. This report can take up to a minute to load.{% endblocktrans %}
{% endblock explainer %} {% block report %}{% endblock report %} + + +{% block summary %} +
+
+ {% metabase_question_embed metabase_advertiser_domains advertiser_slug=advertiser.slug start_date=start_date end_date=end_date %} +
+
+{% endblock summary %} diff --git a/adserver/tests/test_reports.py b/adserver/tests/test_reports.py index dc4f5f77..570d3c36 100644 --- a/adserver/tests/test_reports.py +++ b/adserver/tests/test_reports.py @@ -20,19 +20,16 @@ from ..models import Advertiser from ..models import AdvertiserImpression from ..models import Campaign -from ..models import DomainImpression from ..models import Flight from ..models import Offer from ..models import Publisher from ..models import PublisherPaidImpression -from ..reports import AdvertiserDomainReport from ..reports import AdvertiserReport from ..reports import OptimizedAdvertiserReport from ..reports import OptimizedPublisherPaidReport from ..reports import PublisherGeoReport from ..reports import PublisherReport from ..tasks import daily_update_advertisers -from ..tasks import daily_update_domains from ..tasks import daily_update_geos from ..tasks import daily_update_impressions from ..tasks import daily_update_keywords @@ -468,33 +465,6 @@ def test_advertiser_publisher_report_contents(self): self.assertContains(response, "Total,3") def test_advertiser_domain_report_contents(self): - get( - Offer, - advertisement=self.ad1, - publisher=self.publisher1, - viewed=True, - domain="example.com", - ) - get( - Offer, - advertisement=self.ad1, - publisher=self.publisher2, - viewed=True, - clicked=True, - domain="example.com", - ) - get( - Offer, - advertisement=self.ad1, - publisher=self.publisher2, - viewed=True, - clicked=False, - domain="example2.com", - ) - - # Update reporting - daily_update_domains() - url = reverse("advertiser_domain_report", args=[self.advertiser1.slug]) # Anonymous @@ -504,17 +474,9 @@ def test_advertiser_domain_report_contents(self): self.client.force_login(self.staff_user) - response = self.client.get(url) - self.assertContains(response, "example.com") - self.assertContains(response, "example2.com") - - report = AdvertiserDomainReport(DomainImpression.objects.filter(advertisement=self.ad1)) - report.generate() - - # Check the actual data - self.assertEqual(len(report.results), 2) - self.assertAlmostEqual(report.total["views"], 3) - self.assertAlmostEqual(report.total["clicks"], 1) + # Handled by metabase + resp = self.client.get(url) + self.assertContains(resp, "Advertiser Domain Report") def test_advertiser_keyword_report(self): url = reverse("advertiser_keyword_report", args=[self.advertiser1.slug]) diff --git a/adserver/views.py b/adserver/views.py index 099f9333..3d06e643 100644 --- a/adserver/views.py +++ b/adserver/views.py @@ -93,7 +93,6 @@ from .models import Advertiser from .models import AdvertiserImpression from .models import Campaign -from .models import DomainImpression from .models import Flight from .models import GeoImpression from .models import KeywordImpression @@ -108,7 +107,6 @@ from .models import RegionTopicImpression from .models import Topic from .models import UpliftImpression -from .reports import AdvertiserDomainReport from .reports import AdvertiserPublisherReport from .reports import AdvertiserReport from .reports import OptimizedAdvertiserReport @@ -1642,12 +1640,10 @@ def get_context_data(self, **kwargs): class AdvertiserDomainReportView(AdvertiserAccessMixin, BaseReportView): - LIMIT = 50 DATA_COLLECTION_START_DATE = datetime( year=2024, month=12, day=1, tzinfo=timezone.get_current_timezone() ) - impression_model = DomainImpression template_name = "adserver/reports/advertiser-domain.html" def get_context_data(self, **kwargs): @@ -1656,11 +1652,6 @@ def get_context_data(self, **kwargs): advertiser_slug = kwargs.get("advertiser_slug", "") advertiser = get_object_or_404(Advertiser, slug=advertiser_slug) - flight_slug = self.request.GET.get("flight", "") - flight = Flight.objects.filter( - campaign__advertiser=advertiser, slug=flight_slug - ).first() - if context["start_date"] < self.DATA_COLLECTION_START_DATE: messages.info( self.request, @@ -1670,26 +1661,12 @@ def get_context_data(self, **kwargs): % (self.DATA_COLLECTION_START_DATE.strftime("%B %Y")), ) - queryset = self.get_queryset( - advertiser=advertiser, - flight=flight, - start_date=context["start_date"], - end_date=context["end_date"], - ) - - report = AdvertiserDomainReport( - queryset, - max_results=self.LIMIT, - ) - report.generate() - context.update( { "advertiser": advertiser, - "report": report, - "flights": Flight.objects.filter( - campaign__advertiser=advertiser - ).order_by("-start_date"), + "metabase_advertiser_domains": settings.METABASE_QUESTIONS.get( + "ADVERTISER_DOMAIN_REPORT" + ), } ) diff --git a/config/settings/base.py b/config/settings/base.py index 885521af..86c66b8e 100644 --- a/config/settings/base.py +++ b/config/settings/base.py @@ -493,6 +493,7 @@ "PUBLISHER_GEO_REPORT": 319, "ADVERTISER_TOPIC_PERFORMANCE": 366, "ADVERTISER_PER_AD_TABLE": 966, + "ADVERTISER_DOMAIN_REPORT": 1098, } METABASE_DASHBOARDS = { "ADVERTISER_FIGURES": 80,