Merge pull request #199 from readthedocs/davidfischer/fix-double-chec…

…k-geo-targeting

Fix a secondary check on geo-targeting

adserver/tests/test_api.py

@@ -1090,3 +1090,44 @@ def test_click_tracking_invalid_targeting(self):
 
         self.assertEqual(resp.status_code, 302)
         self.assertEqual(resp["X-Adserver-Reason"], "Invalid targeting impression")
+
+        # Set the ad to target a specific state and metro
+        self.ad.flight.targeting_parameters = {
+            "include_countries": ["US"],
+            "include_state_provinces": ["CA"],
+            "include_metro_codes": [825, 803],  # San Diego, LA
+        }
+        self.ad.flight.save()
+
+        with mock.patch("adserver.views.get_geolocation") as get_geo:
+            get_geo.return_value = {
+                "country_code": "US",
+                "region": "ID",
+                "dma_code": 757,  # Boise
+            }
+            resp = self.client.get(self.click_url)
+
+        self.assertEqual(resp.status_code, 302)
+        self.assertEqual(resp["X-Adserver-Reason"], "Invalid targeting impression")
+
+        with mock.patch("adserver.views.get_geolocation") as get_geo:
+            get_geo.return_value = {
+                "country_code": "US",
+                "region": "CA",
+                "dma_code": 807,  # Bay Area
+            }
+            resp = self.client.get(self.click_url)
+
+        self.assertEqual(resp.status_code, 302)
+        self.assertEqual(resp["X-Adserver-Reason"], "Invalid targeting impression")
+
+        with mock.patch("adserver.views.get_geolocation") as get_geo:
+            get_geo.return_value = {
+                "country_code": "US",
+                "region": "CA",
+                "dma_code": 825,  # San Diego
+            }
+            resp = self.client.get(self.click_url)
+
+        self.assertEqual(resp.status_code, 302)
+        self.assertEqual(resp["X-Adserver-Reason"], "Billed click")

adserver/views.py

@@ -328,9 +328,15 @@ def ignore_tracking_reason(self, request, advertisement, nonce, publisher):
         parsed_ua = parse_user_agent(user_agent)
 
         country_code = None
+        region_code = None
+        metro_code = None
         geo_data = get_geolocation(ip_address)
         if geo_data:
+            # One or more of these may be None which is OK
+            # Ads targeting countries/regions/metros won't be counted
             country_code = geo_data["country_code"]
+            region_code = geo_data["region"]
+            metro_code = geo_data["dma_code"]
 
         valid_nonce = advertisement.is_valid_nonce(self.impression_type, nonce)
 
@@ -361,15 +367,19 @@ def ignore_tracking_reason(self, request, advertisement, nonce, publisher):
         elif not publisher:
             log.log(self.log_level, "Ad impression for unknown publisher")
             reason = "Unknown publisher"
-        elif not advertisement.flight.show_to_geo(country_code):
+        elif not advertisement.flight.show_to_geo(
+            country_code, region_code, metro_code
+        ):
             # This is very rare but it is visible in ad reports
             # 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
             log.log(
                 self.log_security_level,
-                "Invalid geo targeting for ad [%s]. Country: [%s]",
+                "Invalid geo targeting for ad [%s]. Country: [%s], Regions: [%s], Metro: [%s]",
                 advertisement,
                 country_code,
+                region_code,
+                metro_code,
             )
             reason = "Invalid targeting impression"
         elif self.impression_type == CLICKS and is_click_ratelimited(request):