Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Oct 28, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions vendor/bat-native-ads/src/bat/ads/internal/bundle/creative_ad_notification_info_aliases.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions vendor/bat-native-ads/src/bat/ads/internal/bundle/creative_inline_content_ad_info_aliases.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 3 additions & 0 deletions vendor/bat-native-ads/src/bat/ads/internal/bundle/creative_new_tab_page_ad_info_aliases.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 4 additions & 0 deletions ...or/bat-native-ads/src/bat/ads/internal/bundle/creative_promoted_content_ad_info_aliases.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -14,6 +16,8 @@ namespace ads {

using CreativePromotedContentAdList =
std::vector<CreativePromotedContentAdInfo>;
using CreativePromotedContentAdMap =
std::map<std::string, CreativePromotedContentAdInfo>;

} // namespace ads

Expand Down
81 changes: 54 additions & 27 deletions ...tive-ads/src/bat/ads/internal/database/tables/creative_ad_notifications_database_table.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

#include "bat/ads/internal/database/tables/creative_ad_notifications_database_table.h"

#include <algorithm>
#include <utility>
#include <vector>

Expand All @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions ...ative-ads/src/bat/ads/internal/database/tables/creative_ad_notifications_database_table.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 5 additions & 2 deletions ...src/bat/ads/internal/database/tables/creative_ad_notifications_database_table_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";
Expand All @@ -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";
Expand Down Expand Up @@ -372,7 +375,7 @@ TEST_F(BatAdsCreativeAdNotificationsDatabaseTableTest,
}

TEST_F(BatAdsCreativeAdNotificationsDatabaseTableTest,
GetCreativeAdNotificationsForNonExistentCategory) {
GetCreativeAdNotificationsForNonExistentSegment) {
// Arrange
CreativeAdNotificationList creative_ads;

Expand Down
Loading