Merge pull request #10735 from /issues/19050

Fixes ads will not be served for campaign with more than one state geotarget

vendor/bat-native-ads/src/bat/ads/internal/bundle/creative_ad_notification_info_aliases.h

@@ -6,13 +6,17 @@
 #ifndef BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_BUNDLE_CREATIVE_AD_NOTIFICATION_INFO_ALIASES_H_
 #define BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_BUNDLE_CREATIVE_AD_NOTIFICATION_INFO_ALIASES_H_
 
+#include <map>
+#include <string>
 #include <vector>
 
 #include "bat/ads/internal/bundle/creative_ad_notification_info.h"
 
 namespace ads {
 
 using CreativeAdNotificationList = std::vector<CreativeAdNotificationInfo>;
+using CreativeAdNotificationMap =
+    std::map<std::string, CreativeAdNotificationInfo>;
 
 }  // namespace ads
 

vendor/bat-native-ads/src/bat/ads/internal/bundle/creative_inline_content_ad_info_aliases.h

@@ -6,13 +6,17 @@
 #ifndef BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_BUNDLE_CREATIVE_INLINE_CONTENT_AD_INFO_ALIASES_H_
 #define BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_BUNDLE_CREATIVE_INLINE_CONTENT_AD_INFO_ALIASES_H_
 
+#include <map>
+#include <string>
 #include <vector>
 
 #include "bat/ads/internal/bundle/creative_inline_content_ad_info.h"
 
 namespace ads {
 
 using CreativeInlineContentAdList = std::vector<CreativeInlineContentAdInfo>;
+using CreativeInlineContentAdMap =
+    std::map<std::string, CreativeInlineContentAdInfo>;
 
 }  // namespace ads
 

vendor/bat-native-ads/src/bat/ads/internal/bundle/creative_new_tab_page_ad_info_aliases.h

@@ -6,13 +6,16 @@
 #ifndef BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_BUNDLE_CREATIVE_NEW_TAB_PAGE_AD_INFO_ALIASES_H_
 #define BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_BUNDLE_CREATIVE_NEW_TAB_PAGE_AD_INFO_ALIASES_H_
 
+#include <map>
+#include <string>
 #include <vector>
 
 #include "bat/ads/internal/bundle/creative_new_tab_page_ad_info.h"
 
 namespace ads {
 
 using CreativeNewTabPageAdList = std::vector<CreativeNewTabPageAdInfo>;
+using CreativeNewTabPageAdMap = std::map<std::string, CreativeNewTabPageAdInfo>;
 
 }  // namespace ads
 

vendor/bat-native-ads/src/bat/ads/internal/bundle/creative_promoted_content_ad_info_aliases.h

@@ -6,6 +6,8 @@
 #ifndef BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_BUNDLE_CREATIVE_PROMOTED_CONTENT_AD_INFO_ALIASES_H_
 #define BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_BUNDLE_CREATIVE_PROMOTED_CONTENT_AD_INFO_ALIASES_H_
 
+#include <map>
+#include <string>
 #include <vector>
 
 #include "bat/ads/internal/bundle/creative_promoted_content_ad_info.h"
@@ -14,6 +16,8 @@ namespace ads {
 
 using CreativePromotedContentAdList =
     std::vector<CreativePromotedContentAdInfo>;
+using CreativePromotedContentAdMap =
+    std::map<std::string, CreativePromotedContentAdInfo>;
 
 }  // namespace ads
 

vendor/bat-native-ads/src/bat/ads/internal/database/tables/creative_ad_notifications_database_table.cc

@@ -5,7 +5,6 @@
 
 #include "bat/ads/internal/database/tables/creative_ad_notifications_database_table.h"
 
-#include <algorithm>
 #include <utility>
 #include <vector>
 
@@ -15,7 +14,8 @@
 #include "base/time/time.h"
 #include "bat/ads/ads_client.h"
 #include "bat/ads/internal/ads_client_helper.h"
-#include "bat/ads/internal/bundle/creative_ad_info.h"
+#include "bat/ads/internal/bundle/creative_ad_info_aliases.h"
+#include "bat/ads/internal/bundle/creative_ad_notification_info.h"
 #include "bat/ads/internal/container_util.h"
 #include "bat/ads/internal/database/database_statement_util.h"
 #include "bat/ads/internal/database/database_table_util.h"
@@ -26,6 +26,7 @@
 #include "bat/ads/internal/database/tables/geo_targets_database_table.h"
 #include "bat/ads/internal/database/tables/segments_database_table.h"
 #include "bat/ads/internal/logging.h"
+#include "bat/ads/internal/segments/segments_util.h"
 #include "bat/ads/internal/time_formatting_util.h"
 
 namespace ads {
@@ -356,16 +357,10 @@ void CreativeAdNotifications::OnGetForSegments(
     return;
   }
 
-  CreativeAdNotificationList creative_ad_notifications;
+  CreativeAdNotificationList creative_ads =
+      GetCreativeAdsFromResponse(std::move(response));
 
-  for (const auto& record : response->result->get_records()) {
-    const CreativeAdNotificationInfo creative_ad_notification =
-        GetFromRecord(record.get());
-
-    creative_ad_notifications.push_back(creative_ad_notification);
-  }
-
-  callback(/* success */ true, segments, creative_ad_notifications);
+  callback(/* success */ true, segments, creative_ads);
 }
 
 void CreativeAdNotifications::OnGetAll(
@@ -378,24 +373,12 @@ void CreativeAdNotifications::OnGetAll(
     return;
   }
 
-  CreativeAdNotificationList creative_ad_notifications;
-
-  SegmentList segments;
-
-  for (const auto& record : response->result->get_records()) {
-    const CreativeAdNotificationInfo creative_ad_notification =
-        GetFromRecord(record.get());
-
-    creative_ad_notifications.push_back(creative_ad_notification);
-
-    segments.push_back(creative_ad_notification.segment);
-  }
+  const CreativeAdNotificationList creative_ads =
+      GetCreativeAdsFromResponse(std::move(response));
 
-  std::sort(segments.begin(), segments.end());
-  const auto iter = std::unique(segments.begin(), segments.end());
-  segments.erase(iter, segments.end());
+  const SegmentList segments = GetSegments(creative_ads);
 
-  callback(/* success */ true, segments, creative_ad_notifications);
+  callback(/* success */ true, segments, creative_ads);
 }
 
 CreativeAdNotificationInfo CreativeAdNotifications::GetFromRecord(
@@ -435,6 +418,50 @@ CreativeAdNotificationInfo CreativeAdNotifications::GetFromRecord(
   return creative_ad_notification;
 }
 
+CreativeAdNotificationMap CreativeAdNotifications::GroupCreativeAdsFromResponse(
+    mojom::DBCommandResponsePtr response) {
+  DCHECK(response);
+
+  CreativeAdNotificationMap creative_ads;
+
+  for (const auto& record : response->result->get_records()) {
+    const CreativeAdNotificationInfo creative_ad = GetFromRecord(record.get());
+
+    const auto iter = creative_ads.find(creative_ad.creative_instance_id);
+    if (iter == creative_ads.end()) {
+      creative_ads.insert({creative_ad.creative_instance_id, creative_ad});
+      continue;
+    }
+
+    // Creative instance already exists, so append the geo targets and dayparts
+    // to the existing creative ad
+    iter->second.geo_targets.insert(creative_ad.geo_targets.begin(),
+                                    creative_ad.geo_targets.end());
+
+    iter->second.dayparts.insert(iter->second.dayparts.end(),
+                                 creative_ad.dayparts.begin(),
+                                 creative_ad.dayparts.end());
+  }
+
+  return creative_ads;
+}
+
+CreativeAdNotificationList CreativeAdNotifications::GetCreativeAdsFromResponse(
+    mojom::DBCommandResponsePtr response) {
+  DCHECK(response);
+
+  const CreativeAdNotificationMap grouped_creative_ads =
+      GroupCreativeAdsFromResponse(std::move(response));
+
+  CreativeAdNotificationList creative_ads;
+  for (const auto& grouped_creative_ad : grouped_creative_ads) {
+    const CreativeAdNotificationInfo creative_ad = grouped_creative_ad.second;
+    creative_ads.push_back(creative_ad);
+  }
+
+  return creative_ads;
+}
+
 void CreativeAdNotifications::CreateTableV16(
     mojom::DBTransaction* transaction) {
   DCHECK(transaction);

vendor/bat-native-ads/src/bat/ads/internal/database/tables/creative_ad_notifications_database_table.h

@@ -74,6 +74,10 @@ class CreativeAdNotifications final : public Table {
                 GetCreativeAdNotificationsCallback callback);
 
   CreativeAdNotificationInfo GetFromRecord(mojom::DBRecord* record) const;
+  CreativeAdNotificationMap GroupCreativeAdsFromResponse(
+      mojom::DBCommandResponsePtr response);
+  CreativeAdNotificationList GetCreativeAdsFromResponse(
+      mojom::DBCommandResponsePtr response);
 
   void CreateTableV16(mojom::DBTransaction* transaction);
   void MigrateToV16(mojom::DBTransaction* transaction);

vendor/bat-native-ads/src/bat/ads/internal/database/tables/creative_ad_notifications_database_table_unittest.cc

@@ -260,6 +260,7 @@ TEST_F(BatAdsCreativeAdNotificationsDatabaseTableTest,
   CreativeAdNotificationList creative_ads;
 
   CreativeDaypartInfo daypart_info;
+
   CreativeAdNotificationInfo info_1;
   info_1.creative_instance_id = "3519f52c-46a4-4c48-9c2b-c264c0067f04";
   info_1.creative_set_id = "c2ba3e7d-f688-4bc4-a053-cbe7ac1e6123";
@@ -276,6 +277,7 @@ TEST_F(BatAdsCreativeAdNotificationsDatabaseTableTest,
   info_1.value = 1.0;
   info_1.segment = "technology & computing-software";
   info_1.dayparts.push_back(daypart_info);
+  info_1.dayparts.push_back(daypart_info);
   info_1.geo_targets = {"US"};
   info_1.target_url = "https://brave.com";
   info_1.title = "Test Ad 1 Title";
@@ -299,7 +301,8 @@ TEST_F(BatAdsCreativeAdNotificationsDatabaseTableTest,
   info_2.value = 1.0;
   info_2.segment = "technology & computing-software";
   info_2.dayparts.push_back(daypart_info);
-  info_2.geo_targets = {"US"};
+  info_2.dayparts.push_back(daypart_info);
+  info_2.geo_targets = {"US-FL", "US-CA"};
   info_2.target_url = "https://brave.com";
   info_2.title = "Test Ad 2 Title";
   info_2.body = "Test Ad 2 Body";
@@ -372,7 +375,7 @@ TEST_F(BatAdsCreativeAdNotificationsDatabaseTableTest,
 }
 
 TEST_F(BatAdsCreativeAdNotificationsDatabaseTableTest,
-       GetCreativeAdNotificationsForNonExistentCategory) {
+       GetCreativeAdNotificationsForNonExistentSegment) {
   // Arrange
   CreativeAdNotificationList creative_ads;