Merge pull request #735 from readthedocs/davidfischer/forcing-ads-ign…

…ore-live-campaign-type

Forcing an ad/campaign ignores some targeting/filtering

adserver/api/views.py

@@ -73,8 +73,9 @@ class AdDecisionView(GeoIpMixin, APIView):
         :<json string url: The URL of the requesting page. This is where the ad will appear.
         :<json string format: Format can optionally be specified as ``jsonp`` to allow a callback.
         :<json string callback: The name of the callback for a JSONP request (default is ``callback``)
-        :<json string force_ad: Limit results to a specific ad
-        :<json string force_campaign: Limit results to ads from a specific campaign
+        :<json string force_ad: Limit results to a specific ad. Forcing a specific ad will ignore ad targeting,
+            but forced ads are never counted for billing purposes.
+        :<json string force_campaign: As with ``force_ad``, limit results to ads from a specific campaign.
 
         :>json string id: The advertisement slug of the chosen ad
         :>json string text: The HTML text of only the ad without any images (see ``html`` for full HTML)

adserver/decisionengine/backends.py

@@ -200,31 +200,34 @@ def get_candidate_flights(self):
         if not self.should_display_ads():
             return Flight.objects.none()
 
-        flights = (
-            Flight.objects.filter(
-                advertisements__ad_types__slug__in=self.ad_types,
-                campaign__campaign_type__in=self.campaign_types,
+        # If the flight is restricted by campaign or ad slug
+        # Don't restrict it by anything else (campaign type, live, flight date, publisher, etc.)
+        if self.ad_slug:
+            log.debug("Restricting ad decision by ad. ad_slug=%s", self.ad_slug)
+            flights = Flight.objects.filter(advertisements__slug=self.ad_slug)
+        elif self.campaign_slug:
+            log.debug(
+                "Restricting ad decision by campaign. campaign=%s", self.campaign_slug
             )
-            .filter(
-                campaign__publisher_groups__in=self.publisher.publisher_groups.all()
+            flights = Flight.objects.filter(campaign__slug=self.campaign_slug)
+        else:
+            flights = (
+                Flight.objects.filter(
+                    advertisements__ad_types__slug__in=self.ad_types,
+                    campaign__campaign_type__in=self.campaign_types,
+                )
+                .filter(
+                    campaign__publisher_groups__in=self.publisher.publisher_groups.all()
+                )
+                .exclude(campaign__exclude_publishers=self.publisher)
             )
-            .exclude(campaign__exclude_publishers=self.publisher)
-        )
 
-        if self.campaign_types != ALL_CAMPAIGN_TYPES:
-            log.debug(
-                "Ads restricted to the following campaign types: %s",
-                self.campaign_types,
-            )
+            if self.campaign_types != ALL_CAMPAIGN_TYPES:
+                log.debug(
+                    "Ads restricted to the following campaign types: %s",
+                    self.campaign_types,
+                )
 
-        # Specifying the ad or campaign slug skips filtering by live or date
-        if self.ad_slug:
-            log.debug("Restricting ad decision ad_slug=%s", self.ad_slug)
-            flights = flights.filter(advertisements__slug=self.ad_slug)
-        elif self.campaign_slug:
-            log.debug("Restricting ad decision campaign=%s", self.campaign_slug)
-            flights = flights.filter(campaign__slug=self.campaign_slug)
-        else:
             flights = flights.filter(live=True, start_date__lte=get_ad_day().date())
 
             # Ensure there's a live ad of the chosen types for each flight

adserver/tests/test_api.py

@@ -379,6 +379,7 @@ def test_force_ad(self):
         self.publisher.unauthed_ad_decisions = True
         self.publisher.save()
 
+        # Ad does not exist
         self.data["force_ad"] = "unknown-slug"
         resp = self.client.post(
             self.url, json.dumps(self.data), content_type="application/json"
@@ -409,6 +410,8 @@ def test_force_ad(self):
         self.assertEqual(resp_json["id"], "ad-slug", resp_json)
 
         # Force the same ad with the unauth/JSONP client
+        # Forcing an ad ignores campaign type checking
+        self.query_params["campaign_types"] = "house"
         self.query_params["force_ad"] = "ad-slug"
         resp = self.unauth_client.get(self.url, self.query_params)
         self.assertEqual(resp.status_code, 200, resp.content)
@@ -418,7 +421,8 @@ def test_force_ad(self):
 
     def test_force_ad_counted(self):
         # Paid ads don't count
-
+        # But ads can be forced even if the flight is not live
+        self.ad.flight.live = False
         self.ad.flight.campaign.campaign_type = "paid"
         self.ad.flight.campaign.save()
 
@@ -427,10 +431,12 @@ def test_force_ad_counted(self):
         resp = self.client.post(
             self.url, json.dumps(self.data), content_type="application/json"
         )
-        view_resp = self.proxy_client.get(resp.json()["view_url"])
+        self.assertTrue("id" in resp.json())
+        self.assertEqual(resp.json()["id"], "ad-slug")
+        self.proxy_client.get(resp.json()["view_url"])
         self.assertFalse(self.ad.offers.first().viewed)
 
-        # House ads are counted
+        # House ads are counted even when forced
         self.ad.flight.campaign.campaign_type = "house"
         self.ad.flight.campaign.save()