Skip to content

Commit

Permalink
[ads] Fixes earnings in rewards non-custodian state count towards ear…
Browse files Browse the repository at this point in the history
…nings in rewards custodian state
  • Loading branch information
tmancey committed Oct 25, 2024
1 parent f70f697 commit f9d8348
Show file tree
Hide file tree
Showing 19 changed files with 22 additions and 220 deletions.
15 changes: 8 additions & 7 deletions components/brave_ads/browser/ads_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ void AdsServiceImpl::NotifyAdsServiceInitialized() const {
}
}

void AdsServiceImpl::ShutdownAndClearData() {
void AdsServiceImpl::ShutdownClearDataAndMaybeRestart() {
ShutdownAdsService();

VLOG(6) << "Clearing ads data";
Expand All @@ -525,11 +525,12 @@ void AdsServiceImpl::ShutdownAndClearData() {

file_task_runner_->PostTaskAndReplyWithResult(
FROM_HERE, base::BindOnce(&DeletePathOnFileTaskRunner, ads_service_path_),
base::BindOnce(&AdsServiceImpl::ShutdownAndClearDataCallback,
base::BindOnce(&AdsServiceImpl::ShutdownClearDataAndMaybeRestartCallback,
weak_ptr_factory_.GetWeakPtr()));
}

void AdsServiceImpl::ShutdownAndClearDataCallback(const bool success) {
void AdsServiceImpl::ShutdownClearDataAndMaybeRestartCallback(
const bool success) {
if (!success) {
VLOG(0) << "Failed to clear ads data";
} else {
Expand Down Expand Up @@ -1157,7 +1158,7 @@ void AdsServiceImpl::OnNotificationAdClicked(const std::string& placement_id) {

void AdsServiceImpl::ClearData() {
UMA_HISTOGRAM_BOOLEAN(kClearDataHistogramName, true);
ShutdownAndClearData();
ShutdownClearDataAndMaybeRestart();
}

void AdsServiceImpl::GetDiagnostics(GetDiagnosticsCallback callback) {
Expand Down Expand Up @@ -1903,14 +1904,14 @@ void AdsServiceImpl::OnRewardsWalletCreated() {
}

void AdsServiceImpl::OnExternalWalletConnected() {
SetProfilePref(prefs::kShouldMigrateVerifiedRewardsUser, base::Value(true));

ShowReminder(mojom::ReminderType::kExternalWalletConnected);

ShutdownClearDataAndMaybeRestart();
}

void AdsServiceImpl::OnCompleteReset(const bool success) {
if (success) {
ShutdownAndClearData();
ShutdownClearDataAndMaybeRestart();
}
}

Expand Down
6 changes: 4 additions & 2 deletions components/brave_ads/browser/ads_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,10 @@ class AdsServiceImpl final : public AdsService,

void NotifyAdsServiceInitialized() const;

void ShutdownAndClearData();
void ShutdownAndClearDataCallback(bool success);
void ShutdownClearDataAndMaybeRestart();
void ShutdownClearDataAndMaybeRestartCallback(bool success);

void OnExternalWalletConnectedCallback(bool success);

void SetSysInfo();
void SetBuildChannel();
Expand Down
1 change: 0 additions & 1 deletion components/brave_ads/core/internal/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ static_library("internal") {
"account/user_data/user_data_info.h",
"account/user_rewards/user_rewards.cc",
"account/user_rewards/user_rewards.h",
"account/user_rewards/user_rewards_delegate.h",
"account/user_rewards/user_rewards_util.cc",
"account/user_rewards/user_rewards_util.h",
"account/utility/redeem_confirmation/non_reward/redeem_non_reward_confirmation.cc",
Expand Down
4 changes: 0 additions & 4 deletions components/brave_ads/core/internal/account/account.cc
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,4 @@ void Account::OnFailedToConfirm(const ConfirmationInfo& /*confirmation*/) {
MaybeRefillConfirmationTokens();
}

void Account::OnDidMigrateVerifiedRewardsUser() {
InitializeConfirmations();
}

} // namespace brave_ads
7 changes: 1 addition & 6 deletions components/brave_ads/core/internal/account/account.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "brave/components/brave_ads/core/internal/account/account_observer.h"
#include "brave/components/brave_ads/core/internal/account/confirmations/confirmations_delegate.h"
#include "brave/components/brave_ads/core/internal/account/user_rewards/user_rewards.h"
#include "brave/components/brave_ads/core/internal/account/user_rewards/user_rewards_delegate.h"
#include "brave/components/brave_ads/core/internal/account/wallet/wallet_info.h"
#include "brave/components/brave_ads/core/mojom/brave_ads.mojom-forward.h"
#include "brave/components/brave_ads/core/public/ads_callback.h"
Expand All @@ -28,8 +27,7 @@ class Confirmations;
struct TransactionInfo;

class Account final : public AdsClientNotifierObserver,
public ConfirmationDelegate,
public UserRewardsDelegate {
public ConfirmationDelegate {
public:
Account();

Expand Down Expand Up @@ -118,9 +116,6 @@ class Account final : public AdsClientNotifierObserver,
void OnDidConfirm(const ConfirmationInfo& confirmation) override;
void OnFailedToConfirm(const ConfirmationInfo& confirmation) override;

// UserRewardsDelegate:
void OnDidMigrateVerifiedRewardsUser() override;

base::ObserverList<AccountObserver> observers_;

std::unique_ptr<Confirmations> confirmations_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,39 +55,10 @@ void UserRewards::MaybeRedeemPaymentTokens() {

///////////////////////////////////////////////////////////////////////////////

void UserRewards::MaybeMigrateVerifiedRewardsUser() {
if (!ShouldMigrateVerifiedRewardsUser()) {
return;
}

BLOG(1, "Migrate verified rewards user");

ResetTokens();

ResetIssuers();
FetchIssuers();

SetProfileBooleanPref(prefs::kShouldMigrateVerifiedRewardsUser, false);

NotifyDidMigrateVerifiedRewardsUser();
}

void UserRewards::NotifyDidMigrateVerifiedRewardsUser() const {
if (delegate_) {
delegate_->OnDidMigrateVerifiedRewardsUser();
}
}

void UserRewards::OnNotifyDidSolveAdaptiveCaptcha() {
MaybeRefillConfirmationTokens();
}

void UserRewards::OnNotifyPrefDidChange(const std::string& path) {
if (path == prefs::kShouldMigrateVerifiedRewardsUser) {
MaybeMigrateVerifiedRewardsUser();
}
}

void UserRewards::OnDidFetchIssuers(const IssuersInfo& issuers) {
if (!IsIssuersValid(issuers)) {
return BLOG(0, "Invalid issuers");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@

#include <string>

#include "base/memory/raw_ptr.h"
#include "brave/components/brave_ads/core/internal/account/issuers/url_request/issuers_url_request.h"
#include "brave/components/brave_ads/core/internal/account/issuers/url_request/issuers_url_request_delegate.h"
#include "brave/components/brave_ads/core/internal/account/tokens/payment_tokens/payment_token_info.h"
#include "brave/components/brave_ads/core/internal/account/transactions/transactions_database_table.h"
#include "brave/components/brave_ads/core/internal/account/user_rewards/user_rewards_delegate.h"
#include "brave/components/brave_ads/core/internal/account/utility/redeem_payment_tokens/redeem_payment_tokens.h"
#include "brave/components/brave_ads/core/internal/account/utility/redeem_payment_tokens/redeem_payment_tokens_delegate.h"
#include "brave/components/brave_ads/core/internal/account/utility/refill_confirmation_tokens/refill_confirmation_tokens.h"
Expand All @@ -40,25 +38,15 @@ class UserRewards final : public AdsClientNotifierObserver,

~UserRewards() override;

void SetDelegate(UserRewardsDelegate* delegate) {
CHECK_EQ(delegate_, nullptr);
delegate_ = delegate;
}

void FetchIssuers();

void MaybeRefillConfirmationTokens();

void MaybeRedeemPaymentTokens();

private:
void MaybeMigrateVerifiedRewardsUser();

void NotifyDidMigrateVerifiedRewardsUser() const;

// AdsClientNotifierObserver:
void OnNotifyDidSolveAdaptiveCaptcha() override;
void OnNotifyPrefDidChange(const std::string& path) override;

// IssuersUrlRequestDelegate:
void OnDidFetchIssuers(const IssuersInfo& issuers) override;
Expand All @@ -76,8 +64,6 @@ class UserRewards final : public AdsClientNotifierObserver,
void OnCaptchaRequiredToRefillConfirmationTokens(
const std::string& captcha_id) override;

raw_ptr<UserRewardsDelegate> delegate_ = nullptr;

IssuersUrlRequest issuers_url_request_;
RefillConfirmationTokens refill_confirmation_tokens_;
RedeemPaymentTokens redeem_payment_tokens_;
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "brave/components/brave_ads/core/internal/account/tokens/payment_tokens/payment_token_util.h"
#include "brave/components/brave_ads/core/internal/account/tokens/payment_tokens/payment_tokens_test_util.h"
#include "brave/components/brave_ads/core/internal/account/tokens/token_generator_test_util.h"
#include "brave/components/brave_ads/core/internal/account/user_rewards/user_rewards_delegate_mock.h"
#include "brave/components/brave_ads/core/internal/account/utility/redeem_payment_tokens/redeem_payment_tokens_test_util.h"
#include "brave/components/brave_ads/core/internal/account/utility/redeem_payment_tokens/url_request_builders/redeem_payment_tokens_url_request_builder_util.h"
#include "brave/components/brave_ads/core/internal/account/utility/refill_confirmation_tokens/refill_confirmation_tokens_test_util.h"
Expand All @@ -29,8 +28,6 @@
#include "brave/components/brave_ads/core/internal/common/test/profile_pref_value_test_util.h"
#include "brave/components/brave_ads/core/internal/common/test/test_base.h"
#include "brave/components/brave_ads/core/internal/common/test/time_test_util.h"
#include "brave/components/brave_ads/core/internal/prefs/pref_util.h"
#include "brave/components/brave_ads/core/internal/settings/settings_test_util.h"
#include "brave/components/brave_ads/core/public/prefs/pref_names.h"
#include "net/http/http_status_code.h"

Expand All @@ -44,11 +41,9 @@ class BraveAdsUserRewardsTest : public AdsClientMock, public test::TestBase {
test::TestBase::SetUp();

user_rewards_ = std::make_unique<UserRewards>(test::Wallet());
user_rewards_->SetDelegate(&delegate_mock_);
}

std::unique_ptr<UserRewards> user_rewards_;
UserRewardsDelegateMock delegate_mock_;
};

TEST_F(BraveAdsUserRewardsTest, FetchIssuers) {
Expand Down Expand Up @@ -218,44 +213,6 @@ TEST_F(BraveAdsUserRewardsTest, RedeemPaymentTokens) {
EXPECT_TRUE(PaymentTokensIsEmpty());
}

TEST_F(BraveAdsUserRewardsTest, MigrateVerifiedRewardsUser) {
// Arrange
test::BuildAndSetIssuers();

test::MockTokenGenerator(/*count=*/50);

const test::URLResponseMap url_responses = {
{BuildIssuersUrlPath(),
{{net::HTTP_OK, test::BuildIssuersUrlResponseBody()}}}};
test::MockUrlResponses(ads_client_mock_, url_responses);

test::SetPaymentTokens(/*count=*/1);

EXPECT_CALL(delegate_mock_, OnDidMigrateVerifiedRewardsUser);

// Act
SetProfileBooleanPref(prefs::kShouldMigrateVerifiedRewardsUser, true);

// Assert
EXPECT_EQ(0U, ConfirmationTokenCount());
EXPECT_EQ(0U, PaymentTokenCount());
EXPECT_FALSE(GetProfileBooleanPref(prefs::kShouldMigrateVerifiedRewardsUser));
EXPECT_TRUE(HasIssuers());
}

TEST_F(BraveAdsUserRewardsTest, DoNotMigrateVerifiedRewardsUser) {
// Arrange
test::DisableBraveRewards();

EXPECT_CALL(delegate_mock_, OnDidMigrateVerifiedRewardsUser).Times(0);

// Act
SetProfileBooleanPref(prefs::kShouldMigrateVerifiedRewardsUser, false);

// Assert
EXPECT_FALSE(GetProfileBooleanPref(prefs::kShouldMigrateVerifiedRewardsUser));
}

TEST_F(BraveAdsUserRewardsTest,
RequireCaptchaToRefillConfirmationTokensIfCaptchaIdExists) {
// Arrange
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@

namespace brave_ads {

bool ShouldMigrateVerifiedRewardsUser() {
return UserHasJoinedBraveRewards() &&
GetProfileBooleanPref(prefs::kShouldMigrateVerifiedRewardsUser);
}

void UpdateIssuers(const IssuersInfo& issuers) {
if (!HasIssuersChanged(issuers)) {
return BLOG(1, "Issuers already up to date");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ namespace brave_ads {

struct IssuersInfo;

bool ShouldMigrateVerifiedRewardsUser();

void UpdateIssuers(const IssuersInfo& issuers);

} // namespace brave_ads
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,27 +37,6 @@ class BraveAdsUserRewardsUtilTest : public AdsClientNotifierObserverMock,
AdsClientNotifierObserverMock ads_client_notifier_observer_mock_;
};

TEST_F(BraveAdsUserRewardsUtilTest, ShouldMigrateVerifiedRewardsUser) {
// Arrange
test::SetProfileBooleanPrefValue(prefs::kShouldMigrateVerifiedRewardsUser,
true);

// Act & Assert
EXPECT_TRUE(ShouldMigrateVerifiedRewardsUser());
}

TEST_F(BraveAdsUserRewardsUtilTest,
ShouldNotMigrateVerifiedRewardsUserIfBraveRewardsIsDisabled) {
// Arrange
test::DisableBraveRewards();

test::SetProfileBooleanPrefValue(prefs::kShouldMigrateVerifiedRewardsUser,
false);

// Act & Assert
EXPECT_FALSE(ShouldMigrateVerifiedRewardsUser());
}

TEST_F(BraveAdsUserRewardsUtilTest, UpdateIssuers) {
// Arrange
EXPECT_CALL(ads_client_notifier_observer_mock_,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ void RegisterProfilePrefs() {

RegisterProfileBooleanPref(prefs::kHasMigratedClientState, true);
RegisterProfileBooleanPref(prefs::kHasMigratedConfirmationState, true);
RegisterProfileBooleanPref(prefs::kShouldMigrateVerifiedRewardsUser, false);

RegisterProfileStringPref(prefs::kBrowserVersionNumber, "");

Expand Down
Loading

0 comments on commit f9d8348

Please sign in to comment.