Skip to content

Commit

Permalink
[VCN] Modify Enroll bubble cancel button text
Browse files Browse the repository at this point in the history
It will be "Skip" until last time

Will be "No thanks" for the last offer

Bug: 1311041, 1281695
Change-Id: Id01563d3fbe982941a439c323d0a40c55b3ec59f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3556416
Reviewed-by: Jared Saul <jsaul@google.com>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Commit-Queue: Siyu An <siyua@chromium.org>
Cr-Commit-Position: refs/heads/main@{#988237}
  • Loading branch information
Siyu An authored and Chromium LUCI CQ committed Apr 2, 2022
1 parent cd7ce15 commit 80f737b
Show file tree
Hide file tree
Showing 13 changed files with 149 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ std::u16string VirtualCardEnrollBubbleControllerImpl::GetDeclineButtonText()
virtual_card_enrollment_fields_.virtual_card_enrollment_source ==
VirtualCardEnrollmentSource::kSettingsPage
? IDS_CANCEL
: IDS_AUTOFILL_VIRTUAL_CARD_ENROLLMENT_DECLINE_BUTTON_LABEL);
: virtual_card_enrollment_fields_.last_show
? IDS_AUTOFILL_VIRTUAL_CARD_ENROLLMENT_DECLINE_BUTTON_LABEL_NO_THANKS
: IDS_AUTOFILL_VIRTUAL_CARD_ENROLLMENT_DECLINE_BUTTON_LABEL_SKIP);
}

std::u16string VirtualCardEnrollBubbleControllerImpl::GetLearnMoreLinkText()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "components/autofill/core/browser/payments/virtual_card_enrollment_manager.h"

#include "base/strings/string_number_conversions.h"
#include "components/autofill/core/browser/autofill_client.h"
#include "components/autofill/core/browser/data_model/credit_card.h"
#include "components/autofill/core/browser/metrics/payments/virtual_card_enrollment_metrics.h"
Expand Down Expand Up @@ -274,6 +275,17 @@ void VirtualCardEnrollmentManager::ShowVirtualCardEnrollBubble() {
AutofillClock::Now() - save_card_bubble_accepted_timestamp_.value());
save_card_bubble_accepted_timestamp_.reset();
}

// Check in StrikeDatabase whether enrollment has been offered for this card
// before.
state_.virtual_card_enrollment_fields.last_show = false;
if (GetVirtualCardEnrollmentStrikeDatabase() &&
GetVirtualCardEnrollmentStrikeDatabase()->IsLastOffer(
base::NumberToString(state_.virtual_card_enrollment_fields.credit_card
.instrument_id()))) {
state_.virtual_card_enrollment_fields.last_show = true;
}

autofill_client_->ShowVirtualCardEnrollDialog(
state_.virtual_card_enrollment_fields,
base::BindOnce(&VirtualCardEnrollmentManager::Enroll,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ struct VirtualCardEnrollmentFields {
// The source for which the VirtualCardEnrollmentBubble will be shown.
VirtualCardEnrollmentSource virtual_card_enrollment_source =
VirtualCardEnrollmentSource::kNone;
// A boolean value indicating if this will be the final time the user will see
// this offer, until strikes eventually expire. Determined by the number of
// existing strikes.
bool last_show = false;
};

// This struct is used to track the state of the virtual card enrollment
Expand Down Expand Up @@ -238,6 +242,8 @@ class VirtualCardEnrollmentManager {
StrikeDatabase_BubbleCanceled);
FRIEND_TEST_ALL_PREFIXES(VirtualCardEnrollmentManagerTest,
StrikeDatabase_SettingsPageNotBlocked);
FRIEND_TEST_ALL_PREFIXES(VirtualCardEnrollmentManagerTest,
VirtualCardEnrollmentFields_LastShow);

// Called once the risk data is loaded. The |risk_data| will be used with
// |state_|'s |virtual_card_enrollment_fields|'s |credit_card|'s
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <string>

#include "base/callback.h"
#include "base/strings/string_number_conversions.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/mock_callback.h"
#include "base/test/scoped_feature_list.h"
Expand Down Expand Up @@ -668,6 +669,44 @@ TEST_F(VirtualCardEnrollmentManagerTest,
.credit_card.instrument_id()),
VirtualCardEnrollmentSource::kSettingsPage));
}

// Test to ensure that the |last_show| inside a VirtualCardEnrollmentFields is
// set correctly.
TEST_F(VirtualCardEnrollmentManagerTest, VirtualCardEnrollmentFields_LastShow) {
base::HistogramTester histogram_tester;
raw_ptr<VirtualCardEnrollmentProcessState> state =
virtual_card_enrollment_manager_->GetVirtualCardEnrollmentProcessState();
state->vcn_context_token = kTestVcnContextToken;
SetUpCard();
state->virtual_card_enrollment_fields.credit_card = *card_;
personal_data_manager_->SetPaymentsCustomerData(
std::make_unique<PaymentsCustomerData>("123456"));

// Making sure there is no existing strike for the card.
ASSERT_EQ(
virtual_card_enrollment_manager_->GetVirtualCardEnrollmentStrikeDatabase()
->GetStrikes(
base::NumberToString(state->virtual_card_enrollment_fields
.credit_card.instrument_id())),
0);

for (int i = 0; i < virtual_card_enrollment_manager_
->GetVirtualCardEnrollmentStrikeDatabase()
->GetMaxStrikesLimit() -
1;
i++) {
// Show the bubble and ensures VirtualCardEnrollmentFields is set correctly.
virtual_card_enrollment_manager_->ShowVirtualCardEnrollBubble();
EXPECT_FALSE(state->virtual_card_enrollment_fields.last_show);
// Reject the bubble and log strike.
virtual_card_enrollment_manager_->OnVirtualCardEnrollmentBubbleCancelled();
}

// Show the bubble for the last time and ensures VirtualCardEnrollmentFields
// is set correctly.
virtual_card_enrollment_manager_->ShowVirtualCardEnrollBubble();
EXPECT_TRUE(state->virtual_card_enrollment_fields.last_show);
}
#endif // !BUILDFLAG(IS_IOS)

TEST_F(VirtualCardEnrollmentManagerTest, Metrics_LatencySinceUpstream) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ VirtualCardEnrollmentStrikeDatabase::VirtualCardEnrollmentStrikeDatabase(
VirtualCardEnrollmentStrikeDatabase::~VirtualCardEnrollmentStrikeDatabase() =
default;

bool VirtualCardEnrollmentStrikeDatabase::IsLastOffer(
const std::string& instrument_id) const {
// This check should not be invoked for blocked bubble.
DCHECK_LT(GetStrikes(instrument_id), GetMaxStrikesLimit());
return GetStrikes(instrument_id) == GetMaxStrikesLimit() - 1;
}

absl::optional<size_t> VirtualCardEnrollmentStrikeDatabase::GetMaximumEntries()
const {
return absl::optional<size_t>(kMaxStrikeEntities);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ class VirtualCardEnrollmentStrikeDatabase
StrikeDatabaseBase* strike_database);
~VirtualCardEnrollmentStrikeDatabase() override;

// Whether bubble to be shown is the last offer for the card with
// |instrument_id|.
bool IsLastOffer(const std::string& instrument_id) const;

absl::optional<size_t> GetMaximumEntries() const override;
absl::optional<size_t> GetMaximumEntriesAfterCleanup() const override;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,28 @@ TEST_F(VirtualCardEnrollmentStrikeDatabaseTest, AddAndRemoveStrikes) {
EXPECT_FALSE(strike_database_->IsMaxStrikesLimitReached(test_guid));
}

// Test to ensure that IsLastOffer works correctly.
TEST_F(VirtualCardEnrollmentStrikeDatabaseTest, IsLastOffer) {
int max_strikes = strike_database_->GetMaxStrikesLimit();
std::string instrument_id = "123";
ASSERT_EQ(strike_database_->GetStrikes(instrument_id), 0);

// Adds one strike and check IsLastOffer.
strike_database_->AddStrike(instrument_id);
EXPECT_EQ(strike_database_->GetStrikes(instrument_id), 1);
EXPECT_FALSE(strike_database_->IsLastOffer(instrument_id));

// Removes existing strikes.
strike_database_->RemoveStrike(instrument_id);
ASSERT_EQ(strike_database_->GetStrikes(instrument_id), 0);

// Adds |max_strikes - 1| strikes to the strike database and check
// IsLastOffer.
strike_database_->AddStrikes(max_strikes - 1, instrument_id);
EXPECT_EQ(strike_database_->GetStrikes(instrument_id), max_strikes - 1);
EXPECT_TRUE(strike_database_->IsLastOffer(instrument_id));
}

} // namespace

} // namespace autofill
5 changes: 5 additions & 0 deletions components/autofill/core/browser/test_autofill_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,11 @@ TestAutofillClient::GetVirtualCardEnrollmentManager() {
return form_data_importer_->GetVirtualCardEnrollmentManager();
}

void TestAutofillClient::ShowVirtualCardEnrollDialog(
const VirtualCardEnrollmentFields& virtual_card_enrollment_fields,
base::OnceClosure accept_virtual_card_callback,
base::OnceClosure decline_virtual_card_callback) {}

#if !BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_IOS)
std::vector<std::string>
TestAutofillClient::GetAllowedMerchantsForVirtualCards() {
Expand Down
4 changes: 4 additions & 0 deletions components/autofill/core/browser/test_autofill_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ class TestAutofillClient : public AutofillClient {
void OnUnmaskVerificationResult(PaymentsRpcResult result) override;
raw_ptr<VirtualCardEnrollmentManager> GetVirtualCardEnrollmentManager()
override;
void ShowVirtualCardEnrollDialog(
const VirtualCardEnrollmentFields& virtual_card_enrollment_fields,
base::OnceClosure accept_virtual_card_callback,
base::OnceClosure decline_virtual_card_callback) override;
#if !BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_IOS)
std::vector<std::string> GetAllowedMerchantsForVirtualCards() override;
std::vector<std::string> GetAllowedBinRangesForVirtualCards() override;
Expand Down

0 comments on commit 80f737b

Please sign in to comment.