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

Cleanup Brave Ads #18076

Merged
merged 11 commits into from
Apr 15, 2023
Merged

Cleanup Brave Ads #18076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8e45517
Cleanup Brave Ads data structure naming
tmancey Apr 14, 2023
6215e61
Cleanup Brave Ads callback parameters
tmancey Apr 14, 2023
ba2820b
Refactor Brave Ads features
tmancey Apr 14, 2023
4e6bc9d
Fix Brave Ads logging for resources version mismatch
tmancey Apr 14, 2023
908198e
Cleanup Brave Ads database table variable names
tmancey Apr 14, 2023
662f0c2
Cleanup Brave Ads name spacing
tmancey Apr 14, 2023
8628e43
Cleanup Brave Ads parameter comments
tmancey Apr 14, 2023
7a03d58
Cleanup Brave Ads unit tests
tmancey Apr 14, 2023
25d47f8
Cleanup Brave Ads database
tmancey Apr 14, 2023
2031196
Cleanup Brave Ads parameter names
tmancey Apr 14, 2023
04cb0e9
Cleanup Brave Ads unit tests
tmancey Apr 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
27 changes: 15 additions & 12 deletions components/brave_ads/browser/ads_service_callback.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,29 +16,32 @@
namespace brave_ads {

using GetDiagnosticsCallback =
base::OnceCallback<void(absl::optional<base::Value::List>)>;
base::OnceCallback<void(absl::optional<base::Value::List> diagnostics)>;

using GetStatementOfAccountsCallback =
base::OnceCallback<void(mojom::StatementInfoPtr)>;
base::OnceCallback<void(mojom::StatementInfoPtr statement)>;

using MaybeServeInlineContentAdAsDictCallback =
base::OnceCallback<void(const std::string&,
absl::optional<base::Value::Dict>)>;
base::OnceCallback<void(const std::string& dimensions,
absl::optional<base::Value::Dict> ads)>;

using PurgeOrphanedAdEventsForTypeCallback =
base::OnceCallback<void(const bool)>;
base::OnceCallback<void(bool success)>;

using GetHistoryCallback = base::OnceCallback<void(base::Value::List)>;
using GetHistoryCallback = base::OnceCallback<void(base::Value::List history)>;

using ToggleLikeAdCallback = base::OnceCallback<void(base::Value::Dict)>;
using ToggleDislikeAdCallback = base::OnceCallback<void(base::Value::Dict)>;
using ToggleLikeAdCallback =
base::OnceCallback<void(base::Value::Dict ad_content)>;
using ToggleDislikeAdCallback =
base::OnceCallback<void(base::Value::Dict ad_content)>;
using ToggleMarkToReceiveAdsForCategoryCallback =
base::OnceCallback<void(const std::string&, int)>;
base::OnceCallback<void(const std::string& category, int action)>;
using ToggleMarkToNoLongerReceiveAdsForCategoryCallback =
base::OnceCallback<void(const std::string&, int)>;
using ToggleSaveAdCallback = base::OnceCallback<void(base::Value::Dict)>;
base::OnceCallback<void(const std::string& category, int action)>;
using ToggleSaveAdCallback =
base::OnceCallback<void(base::Value::Dict ad_content)>;
using ToggleMarkAdAsInappropriateCallback =
base::OnceCallback<void(base::Value::Dict)>;
base::OnceCallback<void(base::Value::Dict ad_content)>;

} // namespace brave_ads

Expand Down
3 changes: 2 additions & 1 deletion components/brave_ads/browser/ads_tooltips_delegate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
namespace brave_ads {

using ShowScheduledCaptchaCallback =
base::OnceCallback<void(const std::string&, const std::string&)>;
base::OnceCallback<void(const std::string& payment_id,
const std::string& captcha_id)>;
using SnoozeScheduledCaptchaCallback = base::OnceCallback<void()>;

class AdsTooltipsDelegate {
Expand Down
2 changes: 1 addition & 1 deletion components/brave_ads/browser/device_id.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

namespace brave_ads {

using DeviceIdCallback = base::OnceCallback<void(std::string)>;
using DeviceIdCallback = base::OnceCallback<void(std::string device_id)>;

class DeviceId {
public:
Expand Down
2 changes: 1 addition & 1 deletion components/brave_ads/common/features.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ constexpr char kFieldTrialParameterShouldSupportMultipleDisplays[] =
constexpr bool kDefaultShouldSupportMultipleDisplays = false;

constexpr char kFieldTrialParameterShouldAttachNotificationAdToBrowserWindow[] =
"should_attached_ad_notification_to_browser_window";
"should_attach_ad_notification_to_browser_window";
tmancey marked this conversation as resolved.
Show resolved Hide resolved
constexpr bool kDefaultShouldAttachNotificationAdToBrowserWindow = false;

// Ad notification normalized display coordinate for the x component should be
Expand Down
4 changes: 2 additions & 2 deletions components/brave_ads/content/browser/search_result_ad/search_result_ad_handler.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ SearchResultAdHandler::MaybeCreateSearchResultAdHandler(

void SearchResultAdHandler::MaybeRetrieveSearchResultAd(
content::RenderFrameHost* render_frame_host,
base::OnceCallback<void(std::vector<std::string>)> callback) {
base::OnceCallback<void(std::vector<std::string> placement_ids)> callback) {
DCHECK(render_frame_host);
DCHECK(ads_service_);

Expand Down Expand Up @@ -103,7 +103,7 @@ void SearchResultAdHandler::MaybeTriggerSearchResultAdClickedEvent(

void SearchResultAdHandler::OnRetrieveSearchResultAdEntities(
mojo::Remote<blink::mojom::DocumentMetadata> /*document_metadata*/,
base::OnceCallback<void(std::vector<std::string>)> callback,
base::OnceCallback<void(std::vector<std::string> placement_ids)> callback,
blink::mojom::WebPagePtr web_page) {
DCHECK(ads_service_);

Expand Down
2 changes: 1 addition & 1 deletion components/brave_ads/content/browser/search_result_ad/search_result_ad_handler_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ using testing::Mock;
using testing::Return;

using OnRetrieveSearchResultAdCallback =
base::OnceCallback<void(std::vector<std::string>)>;
base::OnceCallback<void(std::vector<std::string> placement_ids)>;

constexpr char kAllowedDomain[] = "https://search.brave.com";
constexpr char kNotAllowedDomain[] = "https://brave.com";
Expand Down
16 changes: 8 additions & 8 deletions components/brave_ads/core/ads_callback.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,26 @@

namespace brave_ads {

using InitializeCallback = base::OnceCallback<void(const bool)>;
using ShutdownCallback = base::OnceCallback<void(const bool)>;
using InitializeCallback = base::OnceCallback<void(bool success)>;
using ShutdownCallback = base::OnceCallback<void(bool success)>;

using RemoveAllHistoryCallback = base::OnceCallback<void(const bool)>;
using RemoveAllHistoryCallback = base::OnceCallback<void(bool success)>;

using MaybeServeNewTabPageAdCallback =
base::OnceCallback<void(const absl::optional<NewTabPageAdInfo>&)>;
base::OnceCallback<void(const absl::optional<NewTabPageAdInfo>& ad)>;

using MaybeServeInlineContentAdCallback =
base::OnceCallback<void(const std::string&,
const absl::optional<InlineContentAdInfo>&)>;
base::OnceCallback<void(const std::string& dimensions,
const absl::optional<InlineContentAdInfo>& ad)>;

using GetStatementOfAccountsCallback =
base::OnceCallback<void(mojom::StatementInfoPtr statement)>;

using GetDiagnosticsCallback =
base::OnceCallback<void(absl::optional<base::Value::List> value)>;
base::OnceCallback<void(absl::optional<base::Value::List> diagnostics)>;

using PurgeOrphanedAdEventsForTypeCallback =
base::OnceCallback<void(const bool)>;
base::OnceCallback<void(bool success)>;

} // namespace brave_ads

Expand Down
17 changes: 9 additions & 8 deletions components/brave_ads/core/ads_client_callback.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,26 @@

namespace brave_ads {

using ResultCallback = base::OnceCallback<void(const bool)>;
using ResultCallback = base::OnceCallback<void(bool success)>;

using SaveCallback = base::OnceCallback<void(const bool)>;
using SaveCallback = base::OnceCallback<void(bool success)>;

using LoadCallback = base::OnceCallback<void(const bool, const std::string&)>;
using LoadCallback =
base::OnceCallback<void(bool success, const std::string& value)>;

using LoadFileCallback = base::OnceCallback<void(base::File)>;
using LoadFileCallback = base::OnceCallback<void(base::File file)>;

using UrlRequestCallback =
base::OnceCallback<void(const mojom::UrlResponseInfo&)>;
base::OnceCallback<void(const mojom::UrlResponseInfo& url_response)>;

using RunDBTransactionCallback =
base::OnceCallback<void(mojom::DBCommandResponseInfoPtr)>;
base::OnceCallback<void(mojom::DBCommandResponseInfoPtr command_response)>;

using GetBrowsingHistoryCallback =
base::OnceCallback<void(const std::vector<GURL>&)>;
base::OnceCallback<void(const std::vector<GURL>& browsing_history)>;

using GetScheduledCaptchaCallback =
base::OnceCallback<void(const std::string&)>;
base::OnceCallback<void(const std::string& captcha_id)>;

} // namespace brave_ads

Expand Down
2 changes: 0 additions & 2 deletions components/brave_ads/core/internal/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,8 +561,6 @@ source_set("internal") {
"common/locale/subdivision_code_util.h",
"common/logging_util.cc",
"common/logging_util.h",
"common/metrics/field_trial_params_util.cc",
"common/metrics/field_trial_params_util.h",
"common/net/http/http_status_code.h",
"common/numbers/number_util.cc",
"common/numbers/number_util.h",
Expand Down
2 changes: 1 addition & 1 deletion components/brave_ads/core/internal/account/account.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ void Account::ProcessDeposit(const std::string& creative_instance_id,
const std::string& segment,
const ConfirmationType& confirmation_type,
const double value) const {
transactions::Add(
AddTransaction(
creative_instance_id, segment, value, ad_type, confirmation_type,
base::BindOnce(&Account::OnDepositProcessed, weak_factory_.GetWeakPtr(),
creative_instance_id, ad_type, confirmation_type));
Expand Down
23 changes: 4 additions & 19 deletions components/brave_ads/core/internal/account/account_features.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,12 @@

#include "brave/components/brave_ads/core/internal/account/account_features.h"

#include "base/metrics/field_trial_params.h"
namespace brave_ads {

namespace brave_ads::features {

namespace {

constexpr char kNextPaymentDayFieldTrialParamName[] = "next_payment_day";
constexpr int kNextPaymentDayDefaultValue = 7;

} // namespace

BASE_FEATURE(kAccount, "Account", base::FEATURE_ENABLED_BY_DEFAULT);
BASE_FEATURE(kAccountFeature, "Account", base::FEATURE_ENABLED_BY_DEFAULT);

bool IsAccountEnabled() {
return base::FeatureList::IsEnabled(kAccount);
}

int GetNextPaymentDay() {
return GetFieldTrialParamByFeatureAsInt(kAccount,
kNextPaymentDayFieldTrialParamName,
kNextPaymentDayDefaultValue);
return base::FeatureList::IsEnabled(kAccountFeature);
}

} // namespace brave_ads::features
} // namespace brave_ads
10 changes: 6 additions & 4 deletions components/brave_ads/core/internal/account/account_features.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,17 @@
#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_ACCOUNT_ACCOUNT_FEATURES_H_

#include "base/feature_list.h"
#include "base/metrics/field_trial_params.h"

namespace brave_ads::features {
namespace brave_ads {

BASE_DECLARE_FEATURE(kAccount);
BASE_DECLARE_FEATURE(kAccountFeature);

bool IsAccountEnabled();

int GetNextPaymentDay();
constexpr base::FeatureParam<int> kNextPaymentDay{&kAccountFeature,
"next_payment_day", 7};

} // namespace brave_ads::features
} // namespace brave_ads

#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_ACCOUNT_ACCOUNT_FEATURES_H_
20 changes: 10 additions & 10 deletions components/brave_ads/core/internal/account/account_features_unittest.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@

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

namespace brave_ads::features {
namespace brave_ads {

TEST(BatAdsAccountFeaturesTest, IsAccountEnabled) {
TEST(BatAdsAccountFeaturesTest, IsEnabled) {
// Arrange

// Act
Expand All @@ -23,12 +23,12 @@ TEST(BatAdsAccountFeaturesTest, IsAccountEnabled) {
EXPECT_TRUE(IsAccountEnabled());
}

TEST(BatAdsAccountFeaturesTest, IsAccountDisabled) {
TEST(BatAdsAccountFeaturesTest, IsDisabled) {
// Arrange
const std::vector<base::test::FeatureRefAndParams> enabled_features;

std::vector<base::test::FeatureRef> disabled_features;
disabled_features.emplace_back(kAccount);
disabled_features.emplace_back(kAccountFeature);

base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeaturesAndParameters(enabled_features,
Expand All @@ -45,7 +45,7 @@ TEST(BatAdsAccountFeaturesTest, GetNextPaymentDay) {
std::vector<base::test::FeatureRefAndParams> enabled_features;
base::FieldTrialParams params;
params["next_payment_day"] = "5";
enabled_features.emplace_back(kAccount, params);
enabled_features.emplace_back(kAccountFeature, params);

const std::vector<base::test::FeatureRef> disabled_features;

Expand All @@ -56,7 +56,7 @@ TEST(BatAdsAccountFeaturesTest, GetNextPaymentDay) {
// Act

// Assert
EXPECT_EQ(5, GetNextPaymentDay());
EXPECT_EQ(5, kNextPaymentDay.Get());
}

TEST(BatAdsAccountFeaturesTest, DefaultNextPaymentDay) {
Expand All @@ -65,15 +65,15 @@ TEST(BatAdsAccountFeaturesTest, DefaultNextPaymentDay) {
// Act

// Assert
EXPECT_EQ(7, GetNextPaymentDay());
EXPECT_EQ(7, kNextPaymentDay.Get());
}

TEST(BatAdsAccountFeaturesTest, DefaultNextPaymentDayWhenDisabled) {
// Arrange
const std::vector<base::test::FeatureRefAndParams> enabled_features;

std::vector<base::test::FeatureRef> disabled_features;
disabled_features.emplace_back(kAccount);
disabled_features.emplace_back(kAccountFeature);

base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitWithFeaturesAndParameters(enabled_features,
Expand All @@ -82,7 +82,7 @@ TEST(BatAdsAccountFeaturesTest, DefaultNextPaymentDayWhenDisabled) {
// Act

// Assert
EXPECT_EQ(7, GetNextPaymentDay());
EXPECT_EQ(7, kNextPaymentDay.Get());
}

} // namespace brave_ads::features
} // namespace brave_ads
Loading