Skip to content

Commit 31be1fc

Browse files
author
Ivan Efremov
committed
Issue 6841: Fix the ads state P3A metric.
We were recording a wrong value ("No wallet") even if the user already had had a wallet in place. Now we try to properly record it and do it only once with the help of an additional flag.
1 parent 2ac5329 commit 31be1fc

File tree

6 files changed

+27
-7
lines changed

6 files changed

+27
-7
lines changed

components/brave_ads/browser/ads_service_factory.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ void AdsServiceFactory::RegisterProfilePrefs(
8484
registry->RegisterUint64Pref(prefs::kAdsPerDay, 20);
8585

8686
registry->RegisterIntegerPref(prefs::kIdleThreshold, 15);
87-
registry->RegisterBooleanPref(prefs::kBraveAdsWereDisabled, false);
87+
registry->RegisterBooleanPref(prefs::kAdsWereDisabled, false);
88+
registry->RegisterBooleanPref(prefs::kHasAdsP3AState, false);
8889

8990
registry->RegisterBooleanPref(prefs::kShouldShowMyFirstAdNotification, true);
9091

components/brave_ads/common/pref_names.cc

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ namespace prefs {
1313
const char kEnabled[] = "brave.brave_ads.enabled";
1414

1515
// Stores whether ads were disabled at least once
16-
const char kBraveAdsWereDisabled[] = "brave.brave_ads.were_disabled";
16+
const char kAdsWereDisabled[] = "brave.brave_ads.were_disabled";
17+
18+
// Indicates whether we have any initial state of the ads status metric, besides
19+
// "No Wallet".
20+
const char kHasAdsP3AState[] = "brave.brave_ads.has_p3a_state";
1721

1822
// Stores the maximum amount of ads per hour
1923
const char kAdsPerHour[] = "brave.brave_ads.ads_per_hour";

components/brave_ads/common/pref_names.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ extern const char kEnabled[];
1414

1515
extern const char kAdsPerHour[];
1616
extern const char kAdsPerDay[];
17-
extern const char kBraveAdsWereDisabled[];
17+
extern const char kAdsWereDisabled[];
18+
extern const char kHasAdsP3AState[];
1819

1920
extern const char kIdleThreshold[];
2021

components/brave_rewards/browser/rewards_p3a.cc

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,17 +88,17 @@ void UpdateAdsP3AOnPreferenceChange(PrefService *prefs,
8888
if (pref == brave_ads::prefs::kEnabled) {
8989
if (prefs->GetBoolean(brave_ads::prefs::kEnabled)) {
9090
brave_rewards::RecordAdsState(AdsP3AState::kAdsEnabled);
91-
prefs->SetBoolean(brave_ads::prefs::kBraveAdsWereDisabled, false);
91+
prefs->SetBoolean(brave_ads::prefs::kAdsWereDisabled, false);
9292
} else {
9393
// Apparently, the pref was disabled.
9494
brave_rewards::RecordAdsState(
9595
rewards_enabled ? AdsP3AState::kAdsEnabledThenDisabledRewardsOn :
9696
AdsP3AState::kAdsEnabledThenDisabledRewardsOff);
97-
prefs->SetBoolean(brave_ads::prefs::kBraveAdsWereDisabled, true);
97+
prefs->SetBoolean(brave_ads::prefs::kAdsWereDisabled, true);
9898
}
9999
} else if (pref == brave_rewards::prefs::kBraveRewardsEnabled) {
100100
// Rewards pref was changed
101-
if (prefs->GetBoolean(brave_ads::prefs::kBraveAdsWereDisabled)) {
101+
if (prefs->GetBoolean(brave_ads::prefs::kAdsWereDisabled)) {
102102
brave_rewards::RecordAdsState(
103103
rewards_enabled ? AdsP3AState::kAdsEnabledThenDisabledRewardsOn :
104104
AdsP3AState::kAdsEnabledThenDisabledRewardsOff);
@@ -107,6 +107,15 @@ void UpdateAdsP3AOnPreferenceChange(PrefService *prefs,
107107
}
108108
}
109109

110+
void MaybeRecordInitialAdsP3AState(PrefService* prefs) {
111+
if (not prefs->GetBoolean(brave_ads::prefs::kHasAdsP3AState)) {
112+
const bool ads_state = prefs->GetBoolean(brave_ads::prefs::kEnabled);
113+
RecordAdsState(ads_state ? AdsP3AState::kAdsEnabled
114+
: AdsP3AState::kAdsDisabled);
115+
prefs->SetBoolean(brave_ads::prefs::kHasAdsP3AState, true);
116+
}
117+
}
118+
110119
void RecordNoWalletCreatedForAllMetrics() {
111120
RecordWalletBalanceP3A(false, 0);
112121
RecordAutoContributionsState(AutoContributionsP3AState::kNoWallet, 0);

components/brave_rewards/browser/rewards_p3a.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ void RecordAdsState(AdsP3AState state);
5050
void UpdateAdsP3AOnPreferenceChange(PrefService* prefs,
5151
const std::string& pref);
5252

53+
// Records an initial metric state ("disabled" or "enabled") if it was not done
54+
// before. Intended to be called if the user has already created a wallet.
55+
void MaybeRecordInitialAdsP3AState(PrefService* local_state);
56+
5357
void RecordNoWalletCreatedForAllMetrics();
5458

5559
double CalcWalletBalanceForP3A(base::flat_map<std::string, double> wallets,

components/brave_rewards/browser/rewards_service_impl.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -927,7 +927,7 @@ void RewardsServiceImpl::OnWalletInitialized(ledger::Result result) {
927927
RecordWalletBalanceP3A(true, 0);
928928
#if BUILDFLAG(BRAVE_ADS_ENABLED)
929929
const bool ads_enabled =
930-
profile_->GetPrefs()->GetBoolean(brave_ads::prefs::kEnabled);
930+
profile_->GetPrefs()->GetBoolean(brave_ads::prefs::kEnabled);
931931
RecordAdsState(ads_enabled ? AdsP3AState::kAdsEnabled
932932
: AdsP3AState::kAdsDisabled);
933933
#endif
@@ -1070,6 +1070,7 @@ void RewardsServiceImpl::OnLedgerStateLoaded(
10701070
}
10711071
// Record stats.
10721072
RecordBackendP3AStats();
1073+
MaybeRecordInitialAdsP3AState(profile_->GetPrefs());
10731074
}
10741075
if (state.first.empty()) {
10751076
RecordNoWalletCreatedForAllMetrics();

0 commit comments

Comments
 (0)