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.

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 users should not receive ads for a category if the user opted-out or from a creative set if a user disliked #9617

Merged
merged 1 commit into from
Aug 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions components/brave_ads/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ source_set("brave_ads_unit_tests") {
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/exclusion_rules/conversion_frequency_cap_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/daypart_frequency_cap_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/exclusion_rules/dislike_frequency_cap_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/exclusion_rules/dismissed_frequency_cap_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/exclusion_rules/marked_as_inappropriate_frequency_cap_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/frequency_capping/exclusion_rules/marked_to_no_longer_receive_frequency_cap_unittest.cc",
Expand Down
2 changes: 2 additions & 0 deletions vendor/bat-native-ads/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,8 @@ source_set("ads") {
"src/bat/ads/internal/frequency_capping/exclusion_rules/daily_cap_frequency_cap.h",
"src/bat/ads/internal/frequency_capping/exclusion_rules/daypart_frequency_cap.cc",
"src/bat/ads/internal/frequency_capping/exclusion_rules/daypart_frequency_cap.h",
"src/bat/ads/internal/frequency_capping/exclusion_rules/dislike_frequency_cap.cc",
"src/bat/ads/internal/frequency_capping/exclusion_rules/dislike_frequency_cap.h",
"src/bat/ads/internal/frequency_capping/exclusion_rules/dismissed_frequency_cap.cc",
"src/bat/ads/internal/frequency_capping/exclusion_rules/dismissed_frequency_cap.h",
"src/bat/ads/internal/frequency_capping/exclusion_rules/exclusion_rule.h",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "bat/ads/internal/frequency_capping/exclusion_rules/conversion_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/exclusion_rules/daily_cap_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/exclusion_rules/daypart_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/exclusion_rules/dislike_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/exclusion_rules/dismissed_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/exclusion_rules/exclusion_rule_util.h"
#include "bat/ads/internal/frequency_capping/exclusion_rules/marked_as_inappropriate_frequency_cap.h"
Expand Down Expand Up @@ -104,6 +105,11 @@ bool ExclusionRules::ShouldExcludeAd(const CreativeAdInfo& ad) const {
should_exclude = true;
}

DislikeFrequencyCap dislike_frequency_cap;
if (ShouldExclude(ad, &dislike_frequency_cap)) {
should_exclude = true;
}

MarkedToNoLongerReceiveFrequencyCap marked_to_no_longer_receive_frequency_cap;
if (ShouldExclude(ad, &marked_to_no_longer_receive_frequency_cap)) {
should_exclude = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "bat/ads/internal/frequency_capping/exclusion_rules/conversion_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/exclusion_rules/daily_cap_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/exclusion_rules/daypart_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/exclusion_rules/dislike_frequency_cap.h"
#include "bat/ads/internal/frequency_capping/exclusion_rules/exclusion_rule.h"
#include "bat/ads/internal/frequency_capping/exclusion_rules/exclusion_rule_util.h"
#include "bat/ads/internal/frequency_capping/exclusion_rules/marked_as_inappropriate_frequency_cap.h"
Expand Down Expand Up @@ -99,6 +100,11 @@ bool ExclusionRules::ShouldExcludeAd(const CreativeAdInfo& ad) const {
should_exclude = true;
}

DislikeFrequencyCap dislike_frequency_cap;
if (ShouldExclude(ad, &dislike_frequency_cap)) {
should_exclude = true;
}

MarkedToNoLongerReceiveFrequencyCap marked_to_no_longer_receive_frequency_cap;
if (ShouldExclude(ad, &marked_to_no_longer_receive_frequency_cap)) {
should_exclude = true;
Expand Down
6 changes: 3 additions & 3 deletions vendor/bat-native-ads/src/bat/ads/internal/client/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ FilteredCategoryList::iterator FindFilteredCategory(
DCHECK(filtered_categories);

return std::find_if(filtered_categories->begin(), filtered_categories->end(),
[&name](const FilteredCategory& category) {
[&name](const FilteredCategoryInfo& category) {
moritzhaller marked this conversation as resolved.
Show resolved Hide resolved
return category.name == name;
});
}
Expand Down Expand Up @@ -244,7 +244,7 @@ CategoryContentInfo::OptAction Client::ToggleAdOptOutAction(
if (action == CategoryContentInfo::OptAction::kOptOut) {
opt_action = CategoryContentInfo::OptAction::kNone;
} else {
opt_action = CategoryContentInfo::CategoryContentInfo::OptAction::kOptOut;
opt_action = CategoryContentInfo::OptAction::kOptOut;
}

// Update this category in the filtered categories list
Expand All @@ -256,7 +256,7 @@ CategoryContentInfo::OptAction Client::ToggleAdOptOutAction(
}
} else {
if (it == client_->ad_preferences.filtered_categories.end()) {
FilteredCategory filtered_category;
FilteredCategoryInfo filtered_category;
filtered_category.name = category;
client_->ad_preferences.filtered_categories.push_back(filtered_category);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ Result AdPreferencesInfo::FromJson(const std::string& json) {
return FAILED;
}

FilteredCategory filtered_category;
FilteredCategoryInfo filtered_category;
filtered_category.name = ad["name"].GetString();
filtered_categories.push_back(filtered_category);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,20 @@

namespace ads {

FilteredCategory::FilteredCategory() = default;
FilteredCategoryInfo::FilteredCategoryInfo() = default;

FilteredCategory::FilteredCategory(const FilteredCategory& category) = default;
FilteredCategoryInfo::FilteredCategoryInfo(
const FilteredCategoryInfo& category) = default;

FilteredCategory::~FilteredCategory() = default;
FilteredCategoryInfo::~FilteredCategoryInfo() = default;

std::string FilteredCategory::ToJson() const {
std::string FilteredCategoryInfo::ToJson() const {
std::string json;
SaveToJson(*this, &json);
return json;
}

Result FilteredCategory::FromJson(const std::string& json) {
Result FilteredCategoryInfo::FromJson(const std::string& json) {
rapidjson::Document document;
document.Parse(json.c_str());

Expand All @@ -38,7 +39,7 @@ Result FilteredCategory::FromJson(const std::string& json) {
return SUCCESS;
}

void SaveToJson(JsonWriter* writer, const FilteredCategory& category) {
void SaveToJson(JsonWriter* writer, const FilteredCategoryInfo& category) {
writer->StartObject();

writer->String("name");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,18 @@

namespace ads {

struct FilteredCategory {
FilteredCategory();
FilteredCategory(const FilteredCategory& category);
~FilteredCategory();
struct FilteredCategoryInfo {
FilteredCategoryInfo();
FilteredCategoryInfo(const FilteredCategoryInfo& category);
~FilteredCategoryInfo();

std::string ToJson() const;
Result FromJson(const std::string& json);

std::string name;
};

using FilteredCategoryList = std::vector<FilteredCategory>;
using FilteredCategoryList = std::vector<FilteredCategoryInfo>;

} // namespace ads

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/* Copyright (c) 2021 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 "bat/ads/internal/frequency_capping/exclusion_rules/dislike_frequency_cap.h"

#include "base/strings/stringprintf.h"
#include "bat/ads/internal/bundle/creative_ad_info.h"
#include "bat/ads/internal/client/client.h"
#include "bat/ads/internal/client/preferences/filtered_ad_info.h"

namespace ads {

DislikeFrequencyCap::DislikeFrequencyCap() = default;

DislikeFrequencyCap::~DislikeFrequencyCap() = default;

bool DislikeFrequencyCap::ShouldExclude(const CreativeAdInfo& ad) {
if (!DoesRespectCap(ad)) {
last_message_ =
base::StringPrintf("creativeSetId %s excluded due to being disliked",
ad.creative_set_id.c_str());

return true;
}

return false;
}

std::string DislikeFrequencyCap::get_last_message() const {
return last_message_;
}

bool DislikeFrequencyCap::DoesRespectCap(const CreativeAdInfo& ad) {
const FilteredAdList filtered_ads = Client::Get()->get_filtered_ads();
if (filtered_ads.empty()) {
return true;
}

const auto iter =
std::find_if(filtered_ads.begin(), filtered_ads.end(),
[&ad](const FilteredAdInfo& filtered_ad) {
return filtered_ad.creative_set_id == ad.creative_set_id;
});

if (iter == filtered_ads.end()) {
return true;
}

return false;
}

} // namespace ads
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/* Copyright (c) 2021 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 BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_FREQUENCY_CAPPING_EXCLUSION_RULES_DISLIKE_FREQUENCY_CAP_H_
#define BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_FREQUENCY_CAPPING_EXCLUSION_RULES_DISLIKE_FREQUENCY_CAP_H_

#include <string>

#include "bat/ads/internal/frequency_capping/exclusion_rules/exclusion_rule.h"

namespace ads {

struct CreativeAdInfo;

class DislikeFrequencyCap : public ExclusionRule<CreativeAdInfo> {
public:
DislikeFrequencyCap();

~DislikeFrequencyCap() override;

DislikeFrequencyCap(const DislikeFrequencyCap&) = delete;
DislikeFrequencyCap& operator=(const DislikeFrequencyCap&) = delete;

bool ShouldExclude(const CreativeAdInfo& ad) override;

std::string get_last_message() const override;

private:
std::string last_message_;

bool DoesRespectCap(const CreativeAdInfo& ad);
};

} // namespace ads

#endif // BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_FREQUENCY_CAPPING_EXCLUSION_RULES_DISLIKE_FREQUENCY_CAP_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/* Copyright (c) 2021 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 "bat/ads/internal/frequency_capping/exclusion_rules/dislike_frequency_cap.h"

#include "bat/ads/internal/unittest_base.h"
#include "bat/ads/internal/unittest_util.h"

// npm run test -- brave_unit_tests --filter=BatAds*

namespace ads {

namespace {
const char kCreativeSetId[] = "654f10df-fbc4-4a92-8d43-2edf73734a60";
} // namespace

class BatAdsDislikeFrequencyCapTest : public UnitTestBase {
protected:
BatAdsDislikeFrequencyCapTest() = default;

~BatAdsDislikeFrequencyCapTest() override = default;
};

TEST_F(BatAdsDislikeFrequencyCapTest, AllowAd) {
// Arrange
CreativeAdInfo ad;
ad.creative_set_id = kCreativeSetId;

// Act
DislikeFrequencyCap frequency_cap;
const bool should_exclude = frequency_cap.ShouldExclude(ad);

// Assert
EXPECT_FALSE(should_exclude);
}

TEST_F(BatAdsDislikeFrequencyCapTest, DoNotAllowAd) {
// Arrange
CreativeAdInfo ad;
ad.creative_set_id = kCreativeSetId;

Client::Get()->ToggleAdThumbDown(ad.creative_instance_id, ad.creative_set_id,
AdContentInfo::LikeAction::kNeutral);

// Act
DislikeFrequencyCap frequency_cap;
const bool should_exclude = frequency_cap.ShouldExclude(ad);

// Assert
EXPECT_TRUE(should_exclude);
}

} // namespace ads
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ class BatAdsMarkedAsInappropriateFrequencyCapTest : public UnitTestBase {
~BatAdsMarkedAsInappropriateFrequencyCapTest() override = default;
};

TEST_F(BatAdsMarkedAsInappropriateFrequencyCapTest,
AllowAdIfNotMarkedAsInappropriate) {
TEST_F(BatAdsMarkedAsInappropriateFrequencyCapTest, AllowAd) {
// Arrange
CreativeAdInfo ad;
ad.creative_set_id = kCreativeSetId;
Expand All @@ -37,8 +36,7 @@ TEST_F(BatAdsMarkedAsInappropriateFrequencyCapTest,
EXPECT_FALSE(should_exclude);
}

TEST_F(BatAdsMarkedAsInappropriateFrequencyCapTest,
DoNotAllowAdIfMarkedAsInappropriate) {
TEST_F(BatAdsMarkedAsInappropriateFrequencyCapTest, DoNotAllowAd) {
// Arrange
CreativeAdInfo ad;
ad.creative_set_id = kCreativeSetId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "base/strings/stringprintf.h"
#include "bat/ads/internal/bundle/creative_ad_info.h"
#include "bat/ads/internal/client/client.h"
#include "bat/ads/internal/client/preferences/filtered_ad_info.h"
#include "bat/ads/internal/client/preferences/filtered_category_info.h"

namespace ads {

Expand Down Expand Up @@ -40,18 +40,19 @@ std::string MarkedToNoLongerReceiveFrequencyCap::get_last_message() const {

bool MarkedToNoLongerReceiveFrequencyCap::DoesRespectCap(
const CreativeAdInfo& ad) {
const FilteredAdList filtered_ads = Client::Get()->get_filtered_ads();
if (filtered_ads.empty()) {
const FilteredCategoryList filtered_categories =
Client::Get()->get_filtered_categories();
if (filtered_categories.empty()) {
return true;
}

const auto iter =
std::find_if(filtered_ads.begin(), filtered_ads.end(),
[&ad](const FilteredAdInfo& filtered_ad) {
return filtered_ad.creative_set_id == ad.creative_set_id;
std::find_if(filtered_categories.begin(), filtered_categories.end(),
[&ad](const FilteredCategoryInfo& filtered_category) {
return filtered_category.name == ad.segment;
});

if (iter == filtered_ads.end()) {
if (iter == filtered_categories.end()) {
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
namespace ads {

namespace {
const char kCreativeSetId[] = "654f10df-fbc4-4a92-8d43-2edf73734a60";
const char kSegment[] = "segment";
} // namespace

class BatAdsMarkedToNoLongerReceiveFrequencyCapTest : public UnitTestBase {
Expand All @@ -23,11 +23,10 @@ class BatAdsMarkedToNoLongerReceiveFrequencyCapTest : public UnitTestBase {
~BatAdsMarkedToNoLongerReceiveFrequencyCapTest() override = default;
};

TEST_F(BatAdsMarkedToNoLongerReceiveFrequencyCapTest,
AllowAdIfNotMarkedToNoLongerReceive) {
TEST_F(BatAdsMarkedToNoLongerReceiveFrequencyCapTest, AllowAd) {
// Arrange
CreativeAdInfo ad;
ad.creative_set_id = kCreativeSetId;
ad.segment = kSegment;

// Act
MarkedToNoLongerReceiveFrequencyCap frequency_cap;
Expand All @@ -37,14 +36,13 @@ TEST_F(BatAdsMarkedToNoLongerReceiveFrequencyCapTest,
EXPECT_FALSE(should_exclude);
}

TEST_F(BatAdsMarkedToNoLongerReceiveFrequencyCapTest,
DoNotAllowAdIfMarkedToNoLongerReceive) {
TEST_F(BatAdsMarkedToNoLongerReceiveFrequencyCapTest, DoNotAllowAd) {
// Arrange
CreativeAdInfo ad;
ad.creative_set_id = kCreativeSetId;
ad.segment = kSegment;

Client::Get()->ToggleAdThumbDown(ad.creative_instance_id, ad.creative_set_id,
tmancey marked this conversation as resolved.
Show resolved Hide resolved
AdContentInfo::LikeAction::kThumbsUp);
Client::Get()->ToggleAdOptOutAction(ad.segment,
CategoryContentInfo::OptAction::kNone);

// Act
MarkedToNoLongerReceiveFrequencyCap frequency_cap;
Expand Down