Skip to content

Commit

Permalink
Merge pull request #3960 from /issues/6205
Browse files Browse the repository at this point in the history
Fixes Ads History popup shows 2 entries for the same ad (even if only served once)
  • Loading branch information
masparrow authored Nov 13, 2019
2 parents 81e1839 + 8d7c067 commit 7380696
Show file tree
Hide file tree
Showing 11 changed files with 633 additions and 11 deletions.
13 changes: 7 additions & 6 deletions components/services/bat_ads/bat_ads_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,13 +159,14 @@ void BatAdsImpl::GetAdsHistory(
GetAdsHistoryCallback callback) {
std::map<uint64_t, std::vector<std::string>> result;

auto ads_history_map = ads_->GetAdsHistory();
for (const auto& entry : ads_history_map) {
std::vector<std::string> ads_history;
for (const auto& ads_history_entry : entry.second) {
ads_history.push_back(ads_history_entry.ToJson());
auto ads_histories = ads_->GetAdsHistory(
ads::AdsHistoryFilterType::kConfirmationType);
for (const auto& ads_history : ads_histories) {
std::vector<std::string> ads_history_json;
for (const auto& ads_history_entry : ads_history.second) {
ads_history_json.push_back(ads_history_entry.ToJson());
}
result[entry.first] = ads_history;
result[ads_history.first] = ads_history_json;
}

std::move(callback).Run(mojo::MapToFlatMap(result));
Expand Down
1 change: 1 addition & 0 deletions test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ test("brave_unit_tests") {
"//brave/components/brave_ads/browser/ads_service_impl_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/client_mock.h",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/client_mock.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/filters/ad_history_confirmation_filter_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/exclusion_rules/daily_cap_frequency_cap_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/exclusion_rules/per_day_frequency_cap_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/exclusion_rules/per_hour_frequency_cap_unittest.cc",
Expand Down
3 changes: 3 additions & 0 deletions vendor/bat-native-ads/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@ source_set("ads") {
"src/bat/ads/internal/filtered_category.h",
"src/bat/ads/internal/flagged_ad.cc",
"src/bat/ads/internal/flagged_ad.h",
"src/bat/ads/internal/filters/ad_history_filter.h",
"src/bat/ads/internal/filters/ad_history_confirmation_filter.h",
"src/bat/ads/internal/filters/ad_history_confirmation_filter.cc",
"src/bat/ads/internal/frequency_capping/exclusion_rule.h",
"src/bat/ads/internal/frequency_capping/exclusion_rules/daily_cap_frequency_cap.cc",
"src/bat/ads/internal/frequency_capping/exclusion_rules/daily_cap_frequency_cap.h",
Expand Down
6 changes: 3 additions & 3 deletions vendor/bat-native-ads/include/bat/ads/ads.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@
#include "bat/ads/notification_event_type.h"
#include "bat/ads/notification_info.h"
#include "bat/ads/public/interfaces/ads.mojom.h"
#include "bat/ads/ads_history.h"

namespace ads {

struct AdsHistory;

using Environment = mojom::Environment;

using InitializeCallback = std::function<void(const Result)>;
Expand Down Expand Up @@ -206,7 +205,8 @@ class ADS_EXPORT Ads {

// Should be called to get ads history. Returns |std::map<uint64_t,
// std::vector<AdsHistory>>| in the format |{timestamp, array<AdsHistory>}|
virtual std::map<uint64_t, std::vector<AdsHistory>> GetAdsHistory() = 0;
virtual std::map<uint64_t, std::vector<AdsHistory>> GetAdsHistory(
const AdsHistoryFilterType ad_history_filterype) = 0;

// Should be called to indicate interest in the specified ad. This is a
// toggle, so calling it again returns the setting to the neutral state
Expand Down
5 changes: 5 additions & 0 deletions vendor/bat-native-ads/include/bat/ads/ads_history.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ namespace ads {

struct AdHistoryDetail;

enum class AdsHistoryFilterType {
kNone = 0,
kConfirmationType
};

struct ADS_EXPORT AdsHistory {
AdsHistory();
explicit AdsHistory(const AdsHistory& history);
Expand Down
20 changes: 19 additions & 1 deletion vendor/bat-native-ads/src/bat/ads/internal/ads_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "bat/ads/internal/frequency_capping/permission_rules/minimum_wait_time_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/permission_rules/ads_per_day_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/permission_rules/ads_per_hour_frequency_cap.h"
#include "bat/ads/internal/filters/ad_history_confirmation_filter.h"

#include "rapidjson/document.h"
#include "rapidjson/error/en.h"
Expand Down Expand Up @@ -547,11 +548,28 @@ void AdsImpl::SetConfirmationsIsReady(
is_confirmations_ready_ = is_ready;
}

std::map<uint64_t, std::vector<AdsHistory>> AdsImpl::GetAdsHistory() {
std::map<uint64_t, std::vector<AdsHistory>> AdsImpl::GetAdsHistory(
const AdsHistoryFilterType ads_history_filter_type) {
std::map<uint64_t, std::vector<AdsHistory>> ads_history;
base::Time now = base::Time::Now().LocalMidnight();

auto ad_history_details = client_->GetAdsShownHistory();

std::unique_ptr<AdHistoryFilter> ad_history_filter;
switch (ads_history_filter_type) {
case AdsHistoryFilterType::kNone: {
break;
}
case AdsHistoryFilterType::kConfirmationType: {
ad_history_filter = std::make_unique<AdHistoryConfirmationFilter>();
break;
}
}

if (ad_history_filter) {
ad_history_details = ad_history_filter->ApplyFilter(ad_history_details);
}

for (auto& detail_item : ad_history_details) {
auto history_item = std::make_unique<AdsHistory>();
history_item->details.push_back(detail_item);
Expand Down
3 changes: 2 additions & 1 deletion vendor/bat-native-ads/src/bat/ads/internal/ads_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ class AdsImpl : public Ads {
void SetConfirmationsIsReady(
const bool is_ready) override;

std::map<uint64_t, std::vector<AdsHistory>> GetAdsHistory() override;
std::map<uint64_t, std::vector<AdsHistory>> GetAdsHistory(
const AdsHistoryFilterType ads_history_filter_type) override;
AdContent::LikeAction ToggleAdThumbUp(
const std::string& id,
const std::string& creative_set_id,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/* Copyright (c) 2019 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include <algorithm>
#include <memory>
#include <string>

#include "bat/ads/internal/filters/ad_history_confirmation_filter.h"
#include "bat/ads/confirmation_type.h"
#include "bat/ads/ad_history_detail.h"

#include "bat/ads/ads_history.h"

namespace ads {

bool IsConfirmationTypeOfInterest(
const ConfirmationType& confirmation_type) {
bool is_of_interest = false;

switch (confirmation_type.value()) {
case ConfirmationType::Value::CLICK:
case ConfirmationType::Value::VIEW:
case ConfirmationType::Value::DISMISS: {
is_of_interest = true;
break;
}
case ConfirmationType::Value::UNKNOWN:
case ConfirmationType::Value::LANDED:
case ConfirmationType::Value::FLAG:
case ConfirmationType::Value::UPVOTE:
case ConfirmationType::Value::DOWNVOTE: {
is_of_interest = false;
break;
}
}
return is_of_interest;
}

bool DoesConfirmationTypeATrumpB(
const ConfirmationType& confirmation_type_a,
const ConfirmationType& confirmation_type_b) {
bool does_type_a_trump_type_b = false;

switch (confirmation_type_a.value()) {
case ConfirmationType::Value::CLICK: {
switch (confirmation_type_b.value()) {
case ConfirmationType::Value::CLICK:
case ConfirmationType::Value::VIEW:
case ConfirmationType::Value::DISMISS: {
does_type_a_trump_type_b = true;
break;
}
default: {
break;
}
}
break;
}
case ConfirmationType::Value::VIEW: {
switch (confirmation_type_b.value()) {
case ConfirmationType::Value::VIEW:
case ConfirmationType::Value::DISMISS: {
does_type_a_trump_type_b = true;
break;
}
default: {
break;
}
}
break;
}
case ConfirmationType::Value::DISMISS: {
switch (confirmation_type_b.value()) {
case ConfirmationType::Value::DISMISS: {
does_type_a_trump_type_b = true;
break;
}
default: {
break;
}
}
break;
}
}

return does_type_a_trump_type_b;
}

AdHistoryConfirmationFilter::~AdHistoryConfirmationFilter() = default;

std::deque<AdHistoryDetail> AdHistoryConfirmationFilter::ApplyFilter(
const std::deque<AdHistoryDetail>& ad_history_details) const {

std::map<std::string, AdHistoryDetail> filtered_ad_history;

for (const AdHistoryDetail& ad_history_detail : ad_history_details) {
if (!IsConfirmationTypeOfInterest(ad_history_detail.ad_content.ad_action)) {
continue;
}
if (filtered_ad_history.count(ad_history_detail.ad_content.uuid) != 0) {
const AdHistoryDetail& check_ad_history_detail =
filtered_ad_history[ad_history_detail.ad_content.uuid];

if (ad_history_detail.timestamp_in_seconds >=
check_ad_history_detail.timestamp_in_seconds) {
if (DoesConfirmationTypeATrumpB(ad_history_detail.ad_content.ad_action,
check_ad_history_detail.ad_content.ad_action)) {
filtered_ad_history[ad_history_detail.ad_content.uuid] =
ad_history_detail;
}
}
} else {
filtered_ad_history[ad_history_detail.ad_content.uuid] =
ad_history_detail;
}
}

std::deque<AdHistoryDetail> filtered_ad_history_details;

for (const auto& ad_history : filtered_ad_history) {
filtered_ad_history_details.push_back(ad_history.second);
}

return filtered_ad_history_details;
}

} // namespace ads
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/* Copyright (c) 2019 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BAT_ADS_INTERNAL_AD_HISTORY_CONFIRMATION_FILTER_H_
#define BAT_ADS_INTERNAL_AD_HISTORY_CONFIRMATION_FILTER_H_

#include <deque>
#include <map>
#include <vector>

#include "bat/ads/internal/filters/ad_history_filter.h"

namespace ads {

struct AdsHistory;

class AdHistoryConfirmationFilter : public AdHistoryFilter {
public :
AdHistoryConfirmationFilter() = default;
~AdHistoryConfirmationFilter() override;

std::deque<AdHistoryDetail> ApplyFilter(
const std::deque<AdHistoryDetail>& ad_history_details) const override;
};

} // namespace ads

#endif // BAT_ADS_INTERNAL_AD_HISTORY_CONFIRMATION_FILTER_H_
Loading

0 comments on commit 7380696

Please sign in to comment.