Merge pull request #1089 from readthedocs/davidfischer/rotations-warnings

Minor changes to logging and validation

Author: davidfischer

adserver/api/serializers.py

@@ -124,8 +124,7 @@ def validate_url(self, url):
             validator(url)  # Throws ValidationError on invalid
             return url
         except ValidationError:
-            log.warning("Invalid ad decision referring URL: %s", url)
-            return None
+            raise serializers.ValidationError("Invalid ad referring URL")
 
     def validate_user_ip(self, ip):
         if ip and "," in ip:

adserver/api/views.py

@@ -306,8 +306,7 @@ def decision(self, request, data):
                 keywords = []
 
             if rotations > 1:
-                # This is a temporary log record to see how frequently ads are rotated
-                log.warning(
+                log.info(
                     "Ad rotation. rotations=%s, publisher=%s, url=%s,",
                     rotations,
                     publisher.slug,

adserver/tests/test_api.py

@@ -1590,19 +1590,14 @@ def test_offer_url(self):
         self.assertIsNotNone(offer)
         self.assertEqual(offer.url, post_url)
 
-        # Invalid URL
+        # Invalid URL - rejected
         self.data["url"] = "invalid-url"
         resp = self.client.post(
             self.url,
             json.dumps(self.data),
             content_type="application/json",
         )
-        self.assertEqual(resp.status_code, 200, resp.content)
-        offer = Offer.objects.filter(id=resp.json()["nonce"]).first()
-
-        # Offer is accepted - URL is set to None
-        self.assertIsNotNone(offer)
-        self.assertIsNone(offer.url)
+        self.assertEqual(resp.status_code, 400, resp.content)
 
         # Missing URL
         del self.data["url"]

adserver/views.py

@@ -1122,11 +1122,12 @@ def ignore_tracking_reason(self, request, advertisement, offer):
             log.log(self.log_level, "Ad impression for unknown publisher")
             reason = "Unknown publisher"
         elif not advertisement.flight.show_to_geo(geo_data):
-            # This is very rare but it is visible in ad reports
+            # Check again that the geo-targeting matches
             # I believe the most common cause for this is somebody uses a VPN and is served an ad
             # Then they turn off their VPN and click on the ad
+            # These should not be billed to advertisers and can be safely ignored.
             log.log(
-                self.log_security_level,
+                self.log_level,
                 "Invalid geo targeting for ad [%s]. Country: [%s], Region: [%s], Metro: [%s]",
                 advertisement,
                 geo_data.country,