Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial work for day of the week targeting #820

Merged
merged 7 commits into from
Feb 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions adserver/decisionengine/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from django.conf import settings
from django.db import models
from django.utils import timezone
from user_agents import parse

from ..analyzer.models import AnalyzedUrl
Expand Down Expand Up @@ -292,6 +293,10 @@ def filter_flight(self, flight):
if flight.weighted_clicks_needed_this_interval() <= 0:
return False

# Skip if the flight is not meant to show on these days
if not flight.show_to_day(timezone.now().strftime("%A").lower()):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess locale will always be EN_US so this should just work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good point. You can also get it to return an int, and use that to map to the day, if we're worried about it.

return False

return True

def select_flight(self):
Expand Down
20 changes: 20 additions & 0 deletions adserver/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -978,6 +978,12 @@ def excluded_domains(self):
return []
return self.targeting_parameters.get("exclude_domains", [])

@property
def days(self):
if not self.targeting_parameters:
return []
return self.targeting_parameters.get("days", [])

@property
def state(self):
today = get_ad_day().date()
Expand Down Expand Up @@ -1032,6 +1038,9 @@ def get_exclude_countries_display(self):
excluded_country_codes = self.excluded_countries
return [COUNTRY_DICT.get(cc, "Unknown") for cc in excluded_country_codes]

def get_days_display(self):
return [day.capitalize() for day in self.days]

def show_to_geo(self, geo_data):
"""
Check if a flight is valid for a given country code.
Expand Down Expand Up @@ -1151,6 +1160,17 @@ def show_to_mobile(self, is_mobile):

return True

def show_to_day(self, day):
"""Check if a flight is valid for this traffic based on the day."""
if not self.targeting_parameters:
return True

days_targeting = self.targeting_parameters.get("days")
if days_targeting and day not in days_targeting:
return False

return True

def show_on_publisher(self, publisher):
if self.included_publishers:
return publisher.slug in self.included_publishers
Expand Down
3 changes: 3 additions & 0 deletions adserver/templates/adserver/includes/flight-metadata.html
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@
{% if flight.campaign.publisher_group_display %}
<li>{% blocktrans with value=flight.campaign.publisher_group_display|join:", " %}Networks: {{ value }}{% endblocktrans %}</li>
{% endif %}
{% if flight.targeting_parameters.days %}
<li>{% blocktrans with value=flight.get_days_display|join:', ' %}Days: {{ value }}{% endblocktrans %}</li>
{% endif %}
</ul>
</dd>
{% endif %}
Expand Down
2 changes: 2 additions & 0 deletions adserver/tests/test_advertiser_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ def test_flight_detail_metadata(self):
"exclude_publishers": ["readthedocs"],
"exclude_domains": ["example.com"],
"mobile_traffic": "exclude",
"days": ["saturday", "sunday"],
}
self.flight.save()

Expand All @@ -281,6 +282,7 @@ def test_flight_detail_metadata(self):
self.assertContains(resp, "Exclude publishers")
self.assertContains(resp, "Exclude domains")
self.assertContains(resp, "Mobile traffic: exclude")
self.assertContains(resp, "Days: Saturday, Sunday")

def test_flight_create_view(self):
url = reverse(
Expand Down
41 changes: 41 additions & 0 deletions adserver/tests/test_decision_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,47 @@ def test_flight_mobile_targeting(self):
ad, _ = self.backend.get_ad_and_placement()
self.assertIsNone(ad)

def test_flight_days_targeting(self):
# Remove existing flights
for flight in Flight.objects.all():
flight.live = False
flight.save()

# Setup a new flight and ad
flight = get(
Flight,
campaign=self.campaign,
live=True,
sold_clicks=100,
targeting_parameters={"days": "monday"},
)

self.advertisement1.flight = flight
self.advertisement1.save()

# Fun times with dates
with mock.patch("adserver.decisionengine.backends.timezone") as tz:

tz.now.return_value = datetime.datetime(2020, 6, 1, 0, 0, 0) # Monday

# Monday targeting works
ad, _ = self.backend.get_ad_and_placement()
self.assertEqual(ad, self.advertisement1)

tz.now.return_value = datetime.datetime(2020, 6, 2, 0, 0, 0) # Tuesday

# Ad is excluded
ad, _ = self.backend.get_ad_and_placement()
self.assertIsNone(ad)

# Set to target Tuesday
flight.targeting_parameters = {"days": ["tuesday"]}
flight.save()

# Ad is back
ad, _ = self.backend.get_ad_and_placement()
self.assertEqual(ad, self.advertisement1)

def test_clicks_needed(self):
self.assertEqual(self.include_flight.clicks_needed_this_interval(), 33)

Expand Down
16 changes: 16 additions & 0 deletions adserver/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,15 @@ class TargetingParametersValidator(BaseValidator):
limit_value = None # required for classes extending BaseValidator

mobile_traffic_values = ("exclude", "only")
days_values = (
"monday",
"tuesday",
"wednesday",
"thursday",
"friday",
"saturday",
"sunday",
)

messages = {
"invalid_type": _("Value must be a dict, not %(type)s"),
Expand All @@ -70,6 +79,7 @@ class TargetingParametersValidator(BaseValidator):
"mobile": _(f"%(value)s must be one of {mobile_traffic_values}"),
"str": _("%(word)s must be a string"),
"percent": _("%(value)s must be a float between [0.0, 1.0]"),
"days": _(f"%(value)s must be one of {days_values}"),
}

validators = {
Expand All @@ -87,6 +97,7 @@ class TargetingParametersValidator(BaseValidator):
"include_domains": "_validate_strs",
"exclude_domains": "_validate_strs",
"mobile_traffic": "_validate_mobile",
"days": "_validate_days",
}

def __init__(self, message=None):
Expand Down Expand Up @@ -137,6 +148,11 @@ def _validate_mobile(self, value):
if value not in self.mobile_traffic_values:
raise ValidationError(self.messages["mobile"], params={"value": value})

def _validate_days(self, values):
for value in values:
if value not in self.days_values:
raise ValidationError(self.messages["days"], params={"value": value})

def _validate_regions(self, region_values):
# Avoid a circular import since the models use these validators
from .models import Region # noqa
Expand Down
3 changes: 3 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ services:
- postgres
volumes:
- .:/app
# Make it so we can edit the start script dynamically,
# for example to install dependencies
- ./docker-compose/django/start:/start
env_file:
- ./.envs/local/django
- ./.envs/local/postgres
Expand Down
2 changes: 2 additions & 0 deletions docker-compose/django/start
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ set -o errexit
set -o pipefail
set -o nounset

# Reinstall dependencies without rebuilding docker image
# pip install -r /app/requirements/production.txt

# Don't auto-migrate locally because this can cause weird issues when testing migrations
# python manage.py migrate
Expand Down
3 changes: 2 additions & 1 deletion docs/user-guide/administration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ The valid options here are:
``include_metro_codes``
An allowlist of 3 digit DMA codes that can be combined with ``include_countries``.
Not all of our geo targeting middlewares support targeting by metro.

``days``
A lowercase list of days of the week where the flight should be shown. Generally used for weekdays or weekends.

For a campaign that targets data science pages visited by users in the US & Canada not on mobile,
an example targeting JSON would be::
Expand Down
2 changes: 1 addition & 1 deletion requirements/production.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
gunicorn==20.0.4

# Database driver
psycopg2-binary==2.9.3
psycopg2-binary==2.9.6 # Required for macOS

# Email sending
django-anymail==8.5
Expand Down
Loading