Skip to content

Commit

Permalink
Merge pull request #815 from readthedocs/davidfischer/publisher-group…
Browse files Browse the repository at this point in the history
…-nondefault-display

Display publisher groups if not default
  • Loading branch information
davidfischer authored Jan 12, 2024
2 parents cd58d0f + 0cd8cf7 commit 3ffbff5
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 8 deletions.
7 changes: 5 additions & 2 deletions adserver/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1056,8 +1056,11 @@ class PublisherPayoutAdmin(SimpleHistoryAdmin):

@admin.register(PublisherGroup)
class PublisherGroupAdmin(SimpleHistoryAdmin):
list_display = ("name", "slug", "modified", "created")
list_filter = ("publishers",)
list_display = ("name", "slug", "default_enabled", "modified", "created")
list_filter = (
"default_enabled",
"publishers",
)
model = PublisherGroup
prepopulated_fields = {"slug": ("name",)}
readonly_fields = ("modified", "created")
Expand Down
29 changes: 29 additions & 0 deletions adserver/migrations/0091_publisher_group_default.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Generated by Django 4.2.4 on 2024-01-10 22:57
from django.db import migrations
from django.db import models


class Migration(migrations.Migration):

dependencies = [
("adserver", "0090_ads_rotated"),
]

operations = [
migrations.AddField(
model_name="historicalpublishergroup",
name="default_enabled",
field=models.BooleanField(
default=False,
help_text="Whether this publisher group is enabled on new campaigns by default",
),
),
migrations.AddField(
model_name="publishergroup",
name="default_enabled",
field=models.BooleanField(
default=False,
help_text="Whether this publisher group is enabled on new campaigns by default",
),
),
]
22 changes: 22 additions & 0 deletions adserver/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,13 @@ class PublisherGroup(TimeStampedModel):
help_text=_("A group of publishers that can be targeted by advertisers"),
)

default_enabled = models.BooleanField(
default=False,
help_text=_(
"Whether this publisher group is enabled on new campaigns by default"
),
)

history = HistoricalRecords()

class Meta:
Expand Down Expand Up @@ -715,6 +722,21 @@ def total_value(self):

return aggregation or 0.0

def publisher_group_display(self):
"""Helper function to display publisher groups if the selected set isn't the default."""
default_pub_groups = [
pg.name
for pg in PublisherGroup.objects.filter(default_enabled=True).order_by(
"name"
)
]
campaign_pub_groups = [pg.name for pg in self.publisher_groups.order_by("name")]

if default_pub_groups != campaign_pub_groups:
return campaign_pub_groups

return None


class Flight(TimeStampedModel, IndestructibleModel):

Expand Down
9 changes: 4 additions & 5 deletions adserver/staff/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ class CreateAdvertiserForm(forms.Form):
DEFAULT_CPM = 5
DEFAULT_NUM_IMPRESSIONS = 200000
DEFAULT_REGION_TARGETING = ["us-ca", "eu-au-nz"]
DEFAULT_TARGETED_GROUPS = ("ethicalads-network", "readthedocs")

# Advertiser information
advertiser_name = forms.CharField(label=_("Advertiser name"), max_length=200)
Expand Down Expand Up @@ -127,11 +126,11 @@ def create_advertiser(self):
name=advertiser_name,
slug=slugify(advertiser_name),
)

# Add the default publisher groups to this campaign
# TODO: Allow configuring of targeted publisher groups in form
for group_slug in self.DEFAULT_TARGETED_GROUPS:
pub_group = PublisherGroup.objects.filter(slug=group_slug).first()
if pub_group:
campaign.publisher_groups.add(pub_group)
for pub_group in PublisherGroup.objects.filter(default_enabled=True):
campaign.publisher_groups.add(pub_group)

flight_name = f"{advertiser_name} Initial Flight"
flight = Flight.objects.create(
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 @@ -125,6 +125,9 @@
{% if flight.targeting_parameters.mobile_traffic %}
<li>{% blocktrans with value=flight.targeting_parameters.mobile_traffic %}Mobile traffic: {{ value }}{% endblocktrans %}</li>
{% endif %}
{% if flight.campaign.publisher_group_display %}
<li>{% blocktrans with value=flight.campaign.publisher_group_display|join:", " %}Networks: {{ value }}{% endblocktrans %}</li>
{% endif %}
</ul>
</dd>
{% endif %}
Expand Down
4 changes: 3 additions & 1 deletion adserver/tests/test_staff_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,12 @@ def setUp(self):
self.pub_group_rtd = get(
PublisherGroup,
slug="readthedocs",
default_enabled=True,
)
self.pub_group_ea = get(
PublisherGroup,
slug="ethicalads-network",
default_enabled=True,
)
self.pub_group_other = get(
PublisherGroup,
Expand Down Expand Up @@ -134,7 +136,7 @@ def test_view(self):
self.assertIsNotNone(campaign)

# Check that the campaign targets the two pub groups (readthedocs and ethicalads-network)
for slug in CreateAdvertiserForm.DEFAULT_TARGETED_GROUPS:
for slug in ("readthedocs", "ethicalads-network"):
self.assertTrue(campaign.publisher_groups.filter(slug=slug).exists())
self.assertFalse(
campaign.publisher_groups.filter(pk=self.pub_group_other.pk).exists()
Expand Down

0 comments on commit 3ffbff5

Please sign in to comment.