Merge pull request #164 from readthedocs/davidfischer/explicit-flag-r…

…ecord-publisher-views

Have an explicit flag to record publisher views

adserver/migrations/0021_publisher_record_ad_views.py

@@ -0,0 +1,18 @@
+# Generated by Django 2.2.13 on 2020-07-06 18:19
+from django.db import migrations
+from django.db import models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('adserver', '0020_add_default_keywords'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='publisher',
+            name='record_views',
+            field=models.BooleanField(default=False, help_text='Record each ad view from this publisher to the database'),
+        ),
+    ]

adserver/models.py

@@ -122,6 +122,14 @@ class Publisher(TimeStampedModel, IndestructibleModel):
         default=True, help_text=_("Only show paid campaigns for this publisher")
     )
 
+    # This overrides settings.ADSERVER_RECORD_VIEWS for a specific publisher
+    # Details of each ad view are written to the database.
+    # Setting this can result in some performance degradation and a bloated database.
+    record_views = models.BooleanField(
+        default=False,
+        help_text=_("Record each ad view from this publisher to the database"),
+    )
+
     class Meta:
         ordering = ("name",)
 
@@ -1024,16 +1032,16 @@ def track_view(self, request, publisher, url):
         Store view data in the DB.
 
         Views are only stored if ``settings.ADSERVER_RECORD_VIEWS=True``
+        Or if a publisher has the ``Publisher.record_views`` flag set.
         For a large scale ad server, writing a database record per ad view
         is not feasible
         """
         self.incr(VIEWS, publisher)
 
-        # TODO: Find a better way to record explicit ad server views
-        if settings.ADSERVER_RECORD_VIEWS or "readthedocs" not in publisher.slug:
+        if settings.ADSERVER_RECORD_VIEWS or publisher.record_views:
             self._record_base(request, View, publisher, url)
         else:
-            log.debug("Not recording ad view (settings.ADSERVER_RECORD_VIEWS=False)")
+            log.debug("Not recording ad view.")
 
     def offer_ad(self, publisher):
         """

adserver/tests/test_api.py

@@ -851,9 +851,11 @@ def test_record_views_false(self):
 
     @override_settings(ADSERVER_RECORD_VIEWS=False)
     def test_record_views_ad_network(self):
-        self.publisher1.slug = "another-publisher"  # No `readthedocs`
-        self.publisher1.unauthed_ad_decisions = True  # In case we change logic
+        # Set the publisher flag to always record views
+        # It should override the one in settings
+        self.publisher1.record_views = True
         self.publisher1.save()
+
         data = {"placements": self.placements, "publisher": self.publisher1.slug}
         resp = self.client.post(
             self.url, json.dumps(data), content_type="application/json"

docs/install/configuration.rst

@@ -81,6 +81,7 @@ ADSERVER_RECORD_VIEWS
 Whether to store metadata (a database record) each time an ad is viewed.
 This is ``False`` by default and can result in a bloated database and poor performance.
 It's ``True`` by default in development.
+This can be overridden on a per publisher basis by setting the ``Publisher.record_views`` flag.
 
 
 ALLOWED_HOSTS