Fixes state level ads being shown on version without state level targ…

…eting
brave · Jul 7, 2020 · a5aa4fb · a5aa4fb
1 parent eaf05bb
commit a5aa4fb
Show file tree

Hide file tree

Showing 10 changed files with 77 additions and 30 deletions.
diff --git a/vendor/bat-native-ads/data/resources/catalog-schema.json b/vendor/bat-native-ads/data/resources/catalog-schema.json
@@ -126,7 +126,8 @@
                 "totalMax",
                 "segments",
                 "creatives",
-                "oses"
+                "oses",
+                "channels"
               ],
               "properties": {
                 "creativeSetId": {
@@ -163,6 +164,7 @@
                 },
                 "segments": {
                   "type": "array",
+                  "minItems": 1,
                   "items": {
                     "type": "object",
                     "additionalProperties": false,
@@ -202,6 +204,12 @@
                     }
                   }
                 },
+                "channels": {
+                  "type": "array",
+                  "items": {
+                    "type": "string"
+                  }
+                },
                 "creatives": {
                   "type": "array",
                   "items": {
@@ -242,23 +250,36 @@
                       },
                       "payload": {
                         "type": "object",
-                        "additionalProperties": false,
-                        "required": [
-                          "targetUrl",
-                          "body",
-                          "title"
-                        ],
-                        "properties": {
-                          "targetUrl": {
-                            "type": "string"
-                          },
-                          "body": {
-                            "type": "string"
+                        "oneOf": [
+                          {
+                            "properties": {
+                              "targetUrl": {
+                                "type": "string"
+                              },
+                              "body": {
+                                "type": "string"
+                              },
+                              "title": {
+                                "type": "string"
+                              }
+                            },
+                            "additionalProperties": false
                           },
-                          "title": {
-                            "type": "string"
+                          {
+                            "properties": {
+                              "creativeUrl": {
+                                "type": "string"
+                              },
+                              "size": {
+                                "type": "string"
+                              },
+                              "targetUrl": {
+                                "type": "string"
+                              }
+                            },
+                            "additionalProperties": false
                           }
-                        }
+                        ]
                       }
                     }
                   }

diff --git a/vendor/bat-native-ads/data/test/catalog.json b/vendor/bat-native-ads/data/test/catalog.json
@@ -1,5 +1,5 @@
 {
-  "version": 1,
+  "version": 3,
   "issuers": [
     {
       "name": "confirmation",
@@ -60,12 +60,15 @@
               "type": "postview"
             }
           ],
+          "channels": [
+          ],
           "creativeSetId": "340c927f-696e-4060-9933-3eafc56c3f31",
           "perDay": 5,
           "totalMax": 100
         }
       ],
-      "dayParts": [],
+      "dayParts": [
+      ],
       "geoTargets": [
         {
           "code": "US",
@@ -113,12 +116,15 @@
               "type": "postclick"
             }
           ],
+          "channels": [
+          ],
           "creativeSetId": "3e2e503a-dd38-42d1-9e26-1e5dbb66aada",
           "perDay": 5,
           "totalMax": 100
         }
       ],
-      "dayParts": [],
+      "dayParts": [
+      ],
       "geoTargets": [
         {
           "code": "US",

diff --git a/vendor/bat-native-ads/src/bat/ads/internal/ads_serve.cc b/vendor/bat-native-ads/src/bat/ads/internal/ads_serve.cc
@@ -36,7 +36,7 @@ void AdsServe::DownloadCatalog() {
   }
 
   BLOG(1, "Download catalog");
-  BLOG(2, "GET /v2/catalog");
+  BLOG(2, "GET " << CATALOG_PATH);
 
   auto callback = std::bind(&AdsServe::OnCatalogDownloaded,
       this, url_, _1, _2, _3);

diff --git a/vendor/bat-native-ads/src/bat/ads/internal/catalog_state.cc b/vendor/bat-native-ads/src/bat/ads/internal/catalog_state.cc
@@ -44,7 +44,7 @@ Result CatalogState::FromJson(
   new_catalog_id = catalog["catalogId"].GetString();
 
   new_version = catalog["version"].GetUint64();
-  if (new_version != 1) {
+  if (new_version != 3) {
     return SUCCESS;
   }
 

diff --git a/...native-ads/src/bat/ads/internal/database/tables/ad_conversions_database_table_unittest.cc b/...native-ads/src/bat/ads/internal/database/tables/ad_conversions_database_table_unittest.cc
@@ -316,7 +316,7 @@ TEST_F(BatAdsAdConversionsDatabaseTableTest,
 
   const URLEndpoints endpoints = {
     {
-      "/v2/catalog", {
+      "/v3/catalog", {
         {
           net::HTTP_OK, "/catalog.json"
         }

diff --git a/...src/bat/ads/internal/database/tables/creative_ad_notifications_database_table_unittest.cc b/...src/bat/ads/internal/database/tables/creative_ad_notifications_database_table_unittest.cc
@@ -792,7 +792,7 @@ TEST_F(BatAdsCreativeAdNotificationsDatabaseTableTest,
 
   const URLEndpoints endpoints = {
     {
-      "/v2/catalog", {
+      "/v3/catalog", {
         {
           net::HTTP_OK, "/catalog.json"
         }

diff --git a/...bat/ads/internal/frequency_capping/exclusion_rules/subdivision_targeting_frequency_cap.cc b/...bat/ads/internal/frequency_capping/exclusion_rules/subdivision_targeting_frequency_cap.cc
@@ -49,11 +49,11 @@ bool SubdivisionTargetingFrequencyCap::DoesRespectCap(
 
   if (!ads_->get_subdivision_targeting()->ShouldAllowAdsSubdivisionTargeting(
       locale)) {
-    return true;
+    return !DoesAdTargetSubdivision(ad);
   }
 
   if (ads_->get_subdivision_targeting()->IsDisabled()) {
-    return true;
+    return !DoesAdTargetSubdivision(ad);
   }
 
   const std::string subdivision_targeting_code =
@@ -80,6 +80,23 @@ bool SubdivisionTargetingFrequencyCap::DoesAdSupportSubdivisionTargetingCode(
   return true;
 }
 
+bool SubdivisionTargetingFrequencyCap::DoesAdTargetSubdivision(
+    const CreativeAdInfo& ad) const {
+  const auto iter = std::find_if(ad.geo_targets.begin(), ad.geo_targets.end(),
+      [&](const std::string& geo_target) {
+    const std::vector<std::string> components = base::SplitString(
+        geo_target, "-", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
+
+    return components.size() == 2;
+  });
+
+  if (iter == ad.geo_targets.end()) {
+    return false;
+  }
+
+  return true;
+}
+
 std::string SubdivisionTargetingFrequencyCap::GetCountryCode(
     const std::string& subdivision_targeting_code) const {
   const std::vector<std::string> components = base::SplitString(

diff --git a/.../bat/ads/internal/frequency_capping/exclusion_rules/subdivision_targeting_frequency_cap.h b/.../bat/ads/internal/frequency_capping/exclusion_rules/subdivision_targeting_frequency_cap.h
@@ -44,6 +44,9 @@ class SubdivisionTargetingFrequencyCap : public ExclusionRule {
       const CreativeAdInfo& ad,
       const std::string& subdivision_targeting_code) const;
 
+  bool DoesAdTargetSubdivision(
+      const CreativeAdInfo& ad) const;
+
   std::string GetCountryCode(
       const std::string& subdivision_targeting_code) const;
 };

diff --git a/...nternal/frequency_capping/exclusion_rules/subdivision_targeting_frequency_cap_unittest.cc b/...nternal/frequency_capping/exclusion_rules/subdivision_targeting_frequency_cap_unittest.cc
@@ -194,7 +194,7 @@ TEST_F(BatAdsSubdivisionTargetingFrequencyCapTest,
 }
 
 TEST_F(BatAdsSubdivisionTargetingFrequencyCapTest,
-    AllowAdIfSubdivisionTargetingIsNotSupportedForSubdivisionGeoTarget) {
+    DoNotAllowAdIfSubdivisionTargetingIsNotSupportedForSubdivisionGeoTarget) {
   // Arrange
   ON_CALL(*locale_helper_mock_, GetLocale())
       .WillByDefault(Return("en-GB"));
@@ -207,7 +207,7 @@ TEST_F(BatAdsSubdivisionTargetingFrequencyCapTest,
   const bool should_exclude = frequency_cap_->ShouldExclude(ad);
 
   // Assert
-  EXPECT_FALSE(should_exclude);
+  EXPECT_TRUE(should_exclude);
 }
 
 TEST_F(BatAdsSubdivisionTargetingFrequencyCapTest,
@@ -228,7 +228,7 @@ TEST_F(BatAdsSubdivisionTargetingFrequencyCapTest,
 }
 
 TEST_F(BatAdsSubdivisionTargetingFrequencyCapTest,
-    AllowAdIfSubdivisionTargetingIsDisabledForSubdivisionGeoTarget) {
+    DoNotAllowAdIfSubdivisionTargetingIsDisabledForSubdivisionGeoTarget) {
   // Arrange
   ON_CALL(*ads_client_mock_, GetAdsSubdivisionTargetingCode())
       .WillByDefault(Return("DISABLED"));
@@ -241,7 +241,7 @@ TEST_F(BatAdsSubdivisionTargetingFrequencyCapTest,
   const bool should_exclude = frequency_cap_->ShouldExclude(ad);
 
   // Assert
-  EXPECT_FALSE(should_exclude);
+  EXPECT_TRUE(should_exclude);
 }
 
 TEST_F(BatAdsSubdivisionTargetingFrequencyCapTest,

diff --git a/vendor/bat-native-ads/src/bat/ads/internal/static_values.h b/vendor/bat-native-ads/src/bat/ads/internal/static_values.h
@@ -19,7 +19,7 @@ namespace ads {
 #define STAGING_SERVER "https://ads-serve.bravesoftware.com"
 #define DEVELOPMENT_SERVER "https://ads-serve.brave.software"
 
-#define CATALOG_PATH "/v2/catalog"
+#define CATALOG_PATH "/v3/catalog"
 #define GETSTATE_PATH "/v5/getstate"
 
 const int kIdleThresholdInSeconds = 15;