Skip to content

Commit

Permalink
Merge pull request #201 from readthedocs/davidfischer/variables-in-ad…
Browse files Browse the repository at this point in the history
…-links

Allow simple variables in Advertisement.link
  • Loading branch information
davidfischer committed Jul 27, 2020
2 parents 9d65901 + 4c66ecd commit ab45c5d
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 5 deletions.
2 changes: 2 additions & 0 deletions adserver/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,8 @@ class Advertisement(TimeStampedModel, IndestructibleModel):
blank=True,
help_text=_("Different ad types have different text requirements"),
)
# Supports simple variables like ${publisher} and ${advertisement}
# using string.Template syntax
link = models.URLField(_("Link URL"), max_length=255)
image = models.ImageField(
_("Image"),
Expand Down
21 changes: 21 additions & 0 deletions adserver/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -1046,6 +1046,27 @@ def test_view_tracking_invalid_ad(self):
resp = self.client.get(url)
self.assertEqual(resp.status_code, 404)

def test_click_tracking_variable_expansion(self):
self.ad.link = "http://example.com?utm_source=${publisher}"
self.ad.save()

resp = self.client.get(self.click_url)

self.assertEqual(resp.status_code, 302)
self.assertEqual(
resp["Location"], "http://example.com?utm_source=test-publisher"
)

# invalid string replacement template
self.ad.link = "http://example.com?utm_source=$%7Btest%7Bpublisher&t=1"
self.ad.save()

resp = self.client.get(self.click_url)

# Even with an invalid template, this should "work" without failing
self.assertEqual(resp.status_code, 302)
self.assertEqual(resp["Location"], self.ad.link)

def test_click_tracking_valid(self):
resp = self.client.get(self.click_url)

Expand Down
16 changes: 11 additions & 5 deletions adserver/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import collections
import csv
import logging
import string
from datetime import datetime
from datetime import timedelta

Expand Down Expand Up @@ -416,7 +417,7 @@ def get(self, request, advertisement_id, nonce):
)

message = ignore_reason or self.success_message
response = self.get_response(request, advertisement)
response = self.get_response(request, advertisement, publisher)

self.send_to_analytics(request, advertisement, message)

Expand All @@ -427,7 +428,7 @@ def get(self, request, advertisement_id, nonce):

return response

def get_response(self, request, advertisement):
def get_response(self, request, advertisement, publisher):
"""Subclasses *must* override this method."""
raise NotImplementedError

Expand All @@ -439,7 +440,7 @@ class AdViewProxyView(BaseProxyView):
impression_type = VIEWS
success_message = "Billed view"

def get_response(self, request, advertisement):
def get_response(self, request, advertisement, publisher):
return HttpResponse(
"<svg><!-- View Proxy --></svg>", content_type="image/svg+xml"
)
Expand Down Expand Up @@ -473,8 +474,13 @@ def send_to_analytics(self, request, advertisement, message):
uip=ip_address, # will be anonymized
)

def get_response(self, request, advertisement):
return HttpResponseRedirect(advertisement.link)
def get_response(self, request, advertisement, publisher):
# Allows using variables in links such as `?utm_source=${publisher}`
template = string.Template(advertisement.link)
url = template.safe_substitute(
publisher=publisher.slug, advertisement=advertisement.slug
)
return HttpResponseRedirect(url)


class BaseReportView(UserPassesTestMixin, TemplateView):
Expand Down

0 comments on commit ab45c5d

Please sign in to comment.