From 005a816fb6301920bfd99f8fef14d99ba8110d7a Mon Sep 17 00:00:00 2001 From: David Fischer Date: Tue, 18 Jul 2023 09:33:16 -0700 Subject: [PATCH 1/4] Flight hard stop Some advertisers running a time sensitive event want a hard stop date for their flight. --- adserver/forms.py | 1 + .../migrations/0085_flight_hard_stop_date.py | 47 +++++++++++++++++++ adserver/models.py | 13 ++++- adserver/tasks.py | 42 +++++++++++++++++ .../adserver/includes/flight-metadata.html | 4 ++ adserver/tests/test_tasks.py | 19 ++++++++ config/settings/production.py | 4 ++ 7 files changed, 128 insertions(+), 2 deletions(-) create mode 100644 adserver/migrations/0085_flight_hard_stop_date.py diff --git a/adserver/forms.py b/adserver/forms.py index 7941024f..d41b19c1 100644 --- a/adserver/forms.py +++ b/adserver/forms.py @@ -90,6 +90,7 @@ class Meta: "campaign", "start_date", "end_date", + "hard_stop_date", "live", "priority_multiplier", "pacing_interval", diff --git a/adserver/migrations/0085_flight_hard_stop_date.py b/adserver/migrations/0085_flight_hard_stop_date.py new file mode 100644 index 00000000..7a55fe64 --- /dev/null +++ b/adserver/migrations/0085_flight_hard_stop_date.py @@ -0,0 +1,47 @@ +# Generated by Django 3.2.20 on 2023-07-18 15:55 +import datetime + +from django.db import migrations +from django.db import models + +import adserver.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('adserver', '0084_publisher_traffic_shaping'), + ] + + operations = [ + migrations.AddField( + model_name='flight', + name='hard_stop_date', + field=models.DateField(blank=True, default=None, help_text='The flight will be stopped on this date even if not completely fulfilled', null=True, verbose_name='Hard Stop Date'), + ), + migrations.AddField( + model_name='historicalflight', + name='hard_stop_date', + field=models.DateField(blank=True, default=None, help_text='The flight will be stopped on this date even if not completely fulfilled', null=True, verbose_name='Hard Stop Date'), + ), + migrations.AlterField( + model_name='flight', + name='end_date', + field=models.DateField(default=adserver.models.default_flight_end_date, help_text='The target end date for the flight (it may go after this date)', verbose_name='End Date'), + ), + migrations.AlterField( + model_name='flight', + name='start_date', + field=models.DateField(db_index=True, default=datetime.date.today, help_text='This flight will not be shown before this date', verbose_name='Start Date'), + ), + migrations.AlterField( + model_name='historicalflight', + name='end_date', + field=models.DateField(default=adserver.models.default_flight_end_date, help_text='The target end date for the flight (it may go after this date)', verbose_name='End Date'), + ), + migrations.AlterField( + model_name='historicalflight', + name='start_date', + field=models.DateField(db_index=True, default=datetime.date.today, help_text='This flight will not be shown before this date', verbose_name='Start Date'), + ), + ] diff --git a/adserver/models.py b/adserver/models.py index 9a77d138..bfe9b4ad 100644 --- a/adserver/models.py +++ b/adserver/models.py @@ -720,12 +720,21 @@ class Flight(TimeStampedModel, IndestructibleModel): _("Start Date"), default=datetime.date.today, db_index=True, - help_text=_("This ad will not be shown before this date"), + help_text=_("This flight will not be shown before this date"), ) end_date = models.DateField( _("End Date"), default=default_flight_end_date, - help_text=_("The target end date for the ad (it may go after this date)"), + help_text=_("The target end date for the flight (it may go after this date)"), + ) + hard_stop_date = models.DateField( + _("Hard Stop Date"), + default=None, + blank=True, + null=True, + help_text=_( + "The flight will be stopped on this date even if not completely fulfilled" + ), ) live = models.BooleanField(_("Live"), default=False) priority_multiplier = models.IntegerField( diff --git a/adserver/tasks.py b/adserver/tasks.py index 9c9b58c3..dad04f96 100644 --- a/adserver/tasks.py +++ b/adserver/tasks.py @@ -14,6 +14,7 @@ from django.template.loader import render_to_string from django.utils.translation import gettext_lazy as _ from django_slack import slack_message +from simple_history.utils import update_change_reason from .constants import FLIGHT_STATE_CURRENT from .constants import FLIGHT_STATE_UPCOMING @@ -1024,6 +1025,47 @@ def update_flight_traffic_fill(): log.info("Completed updating flight traffic fill") +@app.task() +def daily_flight_hard_stop(): + """ + Set flight with a hard stop date to completed. + + This works by setting the sold amount equal to the current fulfilled amount + and sending a slack notification about the remaining credit. + + This should be called before `notify_of_completed_flights()` + so that the regular wrapup emails can be sent to advertisers. + """ + today = get_ad_day().date() + + for flight in Flight.objects.filter(live=True, hard_stop_date__lte=today): + if flight.clicks_remaining() <= 0 and flight.views_remaining() <= 0: + continue + + flight.sold_clicks = flight.total_clicks + flight.sold_impressions = flight.total_views + flight.save() + + value_remaining = round(flight.value_remaining(), 2) + + log.info("Hard stopped flight %s", flight) + + # Store the change reason in the history + update_change_reason( + flight, f"Hard stopped with ${value_remaining} value remaining." + ) + + flight_url = generate_absolute_url(flight.get_absolute_url()) + + # Send an internal notification about this flight being hard stopped. + slack_message( + "adserver/slack/generic-message.slack", + { + "text": f"Flight {flight.name} was hard stopped. There was ${value_remaining} value remaining. {flight_url}" + }, + ) + + @app.task() def run_publisher_importers(): """ diff --git a/adserver/templates/adserver/includes/flight-metadata.html b/adserver/templates/adserver/includes/flight-metadata.html index 0897d58c..0033f7c7 100644 --- a/adserver/templates/adserver/includes/flight-metadata.html +++ b/adserver/templates/adserver/includes/flight-metadata.html @@ -46,6 +46,10 @@
{% trans 'Estimated end date' %}
{{ flight.end_date }}
{% endif %} + {% if flight.hard_stop_date %} +
{% trans 'Hard stop date' %}
+
{{ flight.hard_stop_date }}
+ {% endif %}
{% trans 'Ad selection' %}
diff --git a/adserver/tests/test_tasks.py b/adserver/tests/test_tasks.py index 26baaac5..a6380fe3 100644 --- a/adserver/tests/test_tasks.py +++ b/adserver/tests/test_tasks.py @@ -21,6 +21,7 @@ from ..models import UpliftImpression from ..tasks import calculate_ad_ctrs from ..tasks import calculate_publisher_ctrs +from ..tasks import daily_flight_hard_stop from ..tasks import daily_update_advertisers from ..tasks import daily_update_geos from ..tasks import daily_update_impressions @@ -290,6 +291,24 @@ def test_disable_inactive_publishers(self): self.assertEqual(len(messages), 1) self.assertEqual(len(mail.outbox), 1) + def test_flight_hard_stop(self): + daily_flight_hard_stop() + self.flight.refresh_from_db() + + self.assertTrue(self.flight.clicks_remaining() > 0) + + # Set flight to hard stop today + self.flight.start_date = timezone.now() - datetime.timedelta(days=30) + self.flight.end_date = timezone.now() + self.flight.hard_stop_date = timezone.now() + self.flight.save() + + # This should hard stop the flight + daily_flight_hard_stop() + self.flight.refresh_from_db() + + self.assertEqual(self.flight.clicks_remaining(), 0) + class AggregationTaskTests(BaseAdModelsTestCase): def setUp(self): diff --git a/config/settings/production.py b/config/settings/production.py index b5232ac3..15efe348 100644 --- a/config/settings/production.py +++ b/config/settings/production.py @@ -176,6 +176,10 @@ "task": "adserver.tasks.run_publisher_importers", "schedule": crontab(hour="1", minute="0"), }, + "every-day-flight-hard-stop": { + "task": "adserver.tasks.daily_flight_hard_stop", + "schedule": crontab(hour="23", minute="55"), + }, } # Tasks which should only be run if the analyzer is installed From 23e9e18e5cb89ca1cbd297f89783ace5b7010d62 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Tue, 25 Jul 2023 16:39:53 -0700 Subject: [PATCH 2/4] Update adserver/models.py Co-authored-by: Eric Holscher <25510+ericholscher@users.noreply.github.com> --- adserver/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adserver/models.py b/adserver/models.py index bfe9b4ad..d29c04d7 100644 --- a/adserver/models.py +++ b/adserver/models.py @@ -725,7 +725,7 @@ class Flight(TimeStampedModel, IndestructibleModel): end_date = models.DateField( _("End Date"), default=default_flight_end_date, - help_text=_("The target end date for the flight (it may go after this date)"), + help_text=_("The estimated end date for the flight"), ) hard_stop_date = models.DateField( _("Hard Stop Date"), From a2b0adcf04a193f540dd53dd1167cc600767fd21 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Tue, 25 Jul 2023 17:45:18 -0700 Subject: [PATCH 3/4] Switch hard stop to a boolean --- adserver/forms.py | 2 +- .../migrations/0085_flight_hard_stop_date.py | 14 ++-- adserver/models.py | 14 ++-- adserver/tasks.py | 69 ++++++++----------- .../adserver/includes/flight-metadata.html | 9 ++- adserver/tests/test_tasks.py | 62 ++++++++++++----- config/settings/production.py | 4 -- 7 files changed, 89 insertions(+), 85 deletions(-) diff --git a/adserver/forms.py b/adserver/forms.py index d41b19c1..ad927dd8 100644 --- a/adserver/forms.py +++ b/adserver/forms.py @@ -90,7 +90,7 @@ class Meta: "campaign", "start_date", "end_date", - "hard_stop_date", + "hard_stop", "live", "priority_multiplier", "pacing_interval", diff --git a/adserver/migrations/0085_flight_hard_stop_date.py b/adserver/migrations/0085_flight_hard_stop_date.py index 7a55fe64..c91f87fd 100644 --- a/adserver/migrations/0085_flight_hard_stop_date.py +++ b/adserver/migrations/0085_flight_hard_stop_date.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.20 on 2023-07-18 15:55 +# Generated by Django 3.2.20 on 2023-07-25 23:46 import datetime from django.db import migrations @@ -16,18 +16,18 @@ class Migration(migrations.Migration): operations = [ migrations.AddField( model_name='flight', - name='hard_stop_date', - field=models.DateField(blank=True, default=None, help_text='The flight will be stopped on this date even if not completely fulfilled', null=True, verbose_name='Hard Stop Date'), + name='hard_stop', + field=models.BooleanField(default=False, help_text='The flight will be stopped on the end date even if not completely fulfilled', verbose_name='Hard stop'), ), migrations.AddField( model_name='historicalflight', - name='hard_stop_date', - field=models.DateField(blank=True, default=None, help_text='The flight will be stopped on this date even if not completely fulfilled', null=True, verbose_name='Hard Stop Date'), + name='hard_stop', + field=models.BooleanField(default=False, help_text='The flight will be stopped on the end date even if not completely fulfilled', verbose_name='Hard stop'), ), migrations.AlterField( model_name='flight', name='end_date', - field=models.DateField(default=adserver.models.default_flight_end_date, help_text='The target end date for the flight (it may go after this date)', verbose_name='End Date'), + field=models.DateField(default=adserver.models.default_flight_end_date, help_text='The estimated end date for the flight', verbose_name='End Date'), ), migrations.AlterField( model_name='flight', @@ -37,7 +37,7 @@ class Migration(migrations.Migration): migrations.AlterField( model_name='historicalflight', name='end_date', - field=models.DateField(default=adserver.models.default_flight_end_date, help_text='The target end date for the flight (it may go after this date)', verbose_name='End Date'), + field=models.DateField(default=adserver.models.default_flight_end_date, help_text='The estimated end date for the flight', verbose_name='End Date'), ), migrations.AlterField( model_name='historicalflight', diff --git a/adserver/models.py b/adserver/models.py index d29c04d7..f8b81e89 100644 --- a/adserver/models.py +++ b/adserver/models.py @@ -727,13 +727,11 @@ class Flight(TimeStampedModel, IndestructibleModel): default=default_flight_end_date, help_text=_("The estimated end date for the flight"), ) - hard_stop_date = models.DateField( - _("Hard Stop Date"), - default=None, - blank=True, - null=True, + hard_stop = models.BooleanField( + _("Hard stop"), + default=False, help_text=_( - "The flight will be stopped on this date even if not completely fulfilled" + "The flight will be stopped on the end date even if not completely fulfilled" ), ) live = models.BooleanField(_("Live"), default=False) @@ -1195,10 +1193,12 @@ def views_needed_this_interval(self): def clicks_needed_this_interval(self): """Calculates clicks needed based on the impressions this flight's ads have.""" + today = get_ad_day().date() if ( not self.live or self.clicks_remaining() <= 0 - or self.start_date > get_ad_day().date() + or self.start_date > today + or (self.hard_stop and self.end_date < today) ): return 0 diff --git a/adserver/tasks.py b/adserver/tasks.py index dad04f96..9e1c1329 100644 --- a/adserver/tasks.py +++ b/adserver/tasks.py @@ -738,7 +738,33 @@ def notify_of_completed_flights(): completed_flights_by_advertiser = defaultdict(list) for flight in Flight.objects.filter(live=True).select_related(): - if ( + # Check for hard stopped flights + if flight.hard_stop and flight.end_date <= cutoff.date(): + log.info("Flight %s is being hard stopped.", flight) + value_remaining = round(flight.value_remaining(), 2) + flight_url = generate_absolute_url(flight.get_absolute_url()) + + # Send an internal notification about this flight being hard stopped. + slack_message( + "adserver/slack/generic-message.slack", + { + "text": f"Flight {flight.name} was hard stopped. There was ${value_remaining:.2f} value remaining. {flight_url}" + }, + ) + + # Mark the flight as no longer live. It was hard stopped + flight.live = False + flight.save() + + # Store the change reason in the history + update_change_reason( + flight, f"Hard stopped with ${value_remaining} value remaining." + ) + + completed_flights_by_advertiser[flight.campaign.advertiser.slug].append( + flight + ) + elif ( flight.clicks_remaining() == 0 and flight.views_remaining() == 0 and AdImpression.objects.filter( @@ -1025,47 +1051,6 @@ def update_flight_traffic_fill(): log.info("Completed updating flight traffic fill") -@app.task() -def daily_flight_hard_stop(): - """ - Set flight with a hard stop date to completed. - - This works by setting the sold amount equal to the current fulfilled amount - and sending a slack notification about the remaining credit. - - This should be called before `notify_of_completed_flights()` - so that the regular wrapup emails can be sent to advertisers. - """ - today = get_ad_day().date() - - for flight in Flight.objects.filter(live=True, hard_stop_date__lte=today): - if flight.clicks_remaining() <= 0 and flight.views_remaining() <= 0: - continue - - flight.sold_clicks = flight.total_clicks - flight.sold_impressions = flight.total_views - flight.save() - - value_remaining = round(flight.value_remaining(), 2) - - log.info("Hard stopped flight %s", flight) - - # Store the change reason in the history - update_change_reason( - flight, f"Hard stopped with ${value_remaining} value remaining." - ) - - flight_url = generate_absolute_url(flight.get_absolute_url()) - - # Send an internal notification about this flight being hard stopped. - slack_message( - "adserver/slack/generic-message.slack", - { - "text": f"Flight {flight.name} was hard stopped. There was ${value_remaining} value remaining. {flight_url}" - }, - ) - - @app.task() def run_publisher_importers(): """ diff --git a/adserver/templates/adserver/includes/flight-metadata.html b/adserver/templates/adserver/includes/flight-metadata.html index 0033f7c7..ce52b41d 100644 --- a/adserver/templates/adserver/includes/flight-metadata.html +++ b/adserver/templates/adserver/includes/flight-metadata.html @@ -44,11 +44,10 @@ {% endif %} {% if flight.end_date %}
{% trans 'Estimated end date' %}
-
{{ flight.end_date }}
- {% endif %} - {% if flight.hard_stop_date %} -
{% trans 'Hard stop date' %}
-
{{ flight.hard_stop_date }}
+
+ {{ flight.end_date }} + {% if flight.hard_stop %} ({% trans 'Hard stop' %}){% endif %} +
{% endif %}
{% trans 'Ad selection' %}
diff --git a/adserver/tests/test_tasks.py b/adserver/tests/test_tasks.py index a6380fe3..5863849c 100644 --- a/adserver/tests/test_tasks.py +++ b/adserver/tests/test_tasks.py @@ -21,7 +21,6 @@ from ..models import UpliftImpression from ..tasks import calculate_ad_ctrs from ..tasks import calculate_publisher_ctrs -from ..tasks import daily_flight_hard_stop from ..tasks import daily_update_advertisers from ..tasks import daily_update_geos from ..tasks import daily_update_impressions @@ -179,6 +178,49 @@ def test_notify_completed_flights(self): self.flight.refresh_from_db() self.assertFalse(self.flight.live) + @override_settings( + # Use the memory email backend instead of front for testing + FRONT_BACKEND="django.core.mail.backends.locmem.EmailBackend", + FRONT_ENABLED=True, + ) + def test_notify_completed_flights_hard_stop(self): + # Ensure there's a recipient for a wrapup email + self.staff_user.advertisers.add(self.advertiser) + + backend = get_backend() + backend.reset_messages() + + notify_of_completed_flights() + messages = backend.retrieve_messages() + + # Shouldn't be any completed flight messages + self.assertEqual(len(messages), 0) + self.assertEqual(len(mail.outbox), 0) + + # Set this flight to hard stop + self.flight.sold_clicks = 100 + self.flight.total_views = 1_000 + self.flight.total_clicks = 50 + self.flight.hard_stop = True + self.flight.start_date = timezone.now() - datetime.timedelta(days=31) + self.flight.end_date = timezone.now() - datetime.timedelta(days=1) + self.flight.save() + + # This should hard stop the flight + notify_of_completed_flights() + self.flight.refresh_from_db() + + # Flight should no longer be live + self.assertFalse(self.flight.live) + + messages = backend.retrieve_messages() + self.assertEqual(len(messages), 1) + self.assertTrue( + "was hard stopped. There was $100.00 value remaining" in messages[0]["text"] + ) + self.assertEqual(len(mail.outbox), 1) + self.assertTrue(mail.outbox[0].subject.startswith("Advertising flight wrapup")) + def test_notify_of_publisher_changes(self): # Publisher changes only apply to paid campaigns self.publisher.allow_paid_campaigns = True @@ -291,24 +333,6 @@ def test_disable_inactive_publishers(self): self.assertEqual(len(messages), 1) self.assertEqual(len(mail.outbox), 1) - def test_flight_hard_stop(self): - daily_flight_hard_stop() - self.flight.refresh_from_db() - - self.assertTrue(self.flight.clicks_remaining() > 0) - - # Set flight to hard stop today - self.flight.start_date = timezone.now() - datetime.timedelta(days=30) - self.flight.end_date = timezone.now() - self.flight.hard_stop_date = timezone.now() - self.flight.save() - - # This should hard stop the flight - daily_flight_hard_stop() - self.flight.refresh_from_db() - - self.assertEqual(self.flight.clicks_remaining(), 0) - class AggregationTaskTests(BaseAdModelsTestCase): def setUp(self): diff --git a/config/settings/production.py b/config/settings/production.py index 15efe348..b5232ac3 100644 --- a/config/settings/production.py +++ b/config/settings/production.py @@ -176,10 +176,6 @@ "task": "adserver.tasks.run_publisher_importers", "schedule": crontab(hour="1", minute="0"), }, - "every-day-flight-hard-stop": { - "task": "adserver.tasks.daily_flight_hard_stop", - "schedule": crontab(hour="23", minute="55"), - }, } # Tasks which should only be run if the analyzer is installed From 903bf25452e59685e945bad1e922805d76fbec55 Mon Sep 17 00:00:00 2001 From: David Fischer Date: Thu, 27 Jul 2023 12:22:34 -0700 Subject: [PATCH 4/4] Similar check for view based flights --- adserver/models.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/adserver/models.py b/adserver/models.py index f8b81e89..0199bbec 100644 --- a/adserver/models.py +++ b/adserver/models.py @@ -1174,10 +1174,12 @@ def clicks_today(self): return aggregation or 0 def views_needed_this_interval(self): + today = get_ad_day().date() if ( not self.live or self.views_remaining() <= 0 - or self.start_date > get_ad_day().date() + or self.start_date > today + or (self.hard_stop and self.end_date < today) ): return 0