Commit cf4e2132 authored by Sahel Sharify's avatar Sahel Sharify Committed by Commit Bot

[payments] Do not allow payment app change during retry.

With this change only the app selected for the initial "pay" attempt
will be available during retry.

Bug: 1028098
Change-Id: I13815f19e86c2ca3413867ce17d04dc48862b98c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2015826
Commit-Queue: Sahel Sharify <sahel@chromium.org>
Reviewed-by: default avatarRouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735162}
parent 1e8e0ff5
...@@ -2245,6 +2245,16 @@ public class PaymentRequestImpl ...@@ -2245,6 +2245,16 @@ public class PaymentRequestImpl
mWasRetryCalled = true; mWasRetryCalled = true;
// Remove all payment instruments except the selected one.
assert mPaymentMethodsSection != null;
PaymentInstrument selectedInstrument =
(PaymentInstrument) mPaymentMethodsSection.getSelectedItem();
assert selectedInstrument != null;
mPaymentMethodsSection = new SectionInformation(PaymentRequestUI.DataType.PAYMENT_METHODS,
/* selection = */ 0, new ArrayList<>(Arrays.asList(selectedInstrument)));
mUI.updateSection(PaymentRequestUI.DataType.PAYMENT_METHODS, mPaymentMethodsSection);
mUI.disableAddingNewCardsDuringRetry();
// Go back to the payment sheet // Go back to the payment sheet
mUI.onPayButtonProcessingCancelled(); mUI.onPayButtonProcessingCancelled();
if (!TextUtils.isEmpty(errors.error)) { if (!TextUtils.isEmpty(errors.error)) {
......
...@@ -626,6 +626,15 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View ...@@ -626,6 +626,15 @@ public class PaymentRequestUI implements DialogInterface.OnDismissListener, View
} }
} }
/**
* Disables adding new cards during retry.
*/
public void disableAddingNewCardsDuringRetry() {
assert mPaymentMethodSection != null;
mPaymentMethodSection.setCanAddItems(false);
mPaymentMethodSection.update(mPaymentMethodSectionInformation);
}
/** /**
* Sets the icon in the top left of the UI. This can be, for example, the favicon of the * Sets the icon in the top left of the UI. This can be, for example, the favicon of the
* merchant website. This is not a part of the constructor because favicon retrieval is * merchant website. This is not a part of the constructor because favicon retrieval is
......
...@@ -63,10 +63,67 @@ public class PaymentRequestRetryTest implements MainActivityStartCallback { ...@@ -63,10 +63,67 @@ public class PaymentRequestRetryTest implements MainActivityStartCallback {
helper.setCreditCard(new CreditCard("", "https://example.com", true /* isLocal */, helper.setCreditCard(new CreditCard("", "https://example.com", true /* isLocal */,
true /* isCached */, "Jon Doe", "5555555555554444", "" /* obfuscatedNumber */, "12", true /* isCached */, "Jon Doe", "5555555555554444", "" /* obfuscatedNumber */, "12",
"2050", "mastercard", R.drawable.mc_card, billing_address_id, "" /* serverId */)); "2050", "mastercard", R.drawable.mc_card, billing_address_id, "" /* serverId */));
helper.setCreditCard(new CreditCard("", "https://example.com", true /* isLocal */,
true /* isCached */, "Jon Doe", "4111111111111111", "" /* obfuscatedNumber */, "12",
"2050", "visa", R.drawable.mc_card, billing_address_id, "" /* serverId */));
mPaymentRequestTestRule.installPaymentApp(HAVE_INSTRUMENTS, IMMEDIATE_RESPONSE); mPaymentRequestTestRule.installPaymentApp(HAVE_INSTRUMENTS, IMMEDIATE_RESPONSE);
} }
/**
* Tests that only the initially selected payment instrument is available during retry().
*/
@Test
@MediumTest
@Feature({"Payments"})
public void testDoNotAllowPaymentInstrumentChange() throws TimeoutException {
mPaymentRequestTestRule.triggerUIAndWait(mPaymentRequestTestRule.getReadyForInput());
mPaymentRequestTestRule.clickAndWait(
R.id.button_primary, mPaymentRequestTestRule.getReadyForUnmaskInput());
// Confirm that two payment instruments are available for payment.
Assert.assertEquals(2, mPaymentRequestTestRule.getNumberOfPaymentInstruments());
mPaymentRequestTestRule.setTextInCardUnmaskDialogAndWait(
R.id.card_unmask_input, "123", mPaymentRequestTestRule.getReadyToUnmask());
mPaymentRequestTestRule.clickCardUnmaskButtonAndWait(
ModalDialogProperties.ButtonType.POSITIVE,
mPaymentRequestTestRule.getPaymentResponseReady());
// Confirm that only one payment instrument is available for retry().
mPaymentRequestTestRule.retryPaymentRequest("{}", mPaymentRequestTestRule.getReadyToPay());
Assert.assertEquals(1, mPaymentRequestTestRule.getNumberOfPaymentInstruments());
}
/**
* Tests that adding new cards is disabled during retry().
*/
@Test
@MediumTest
@Feature({"Payments"})
public void testDoNotAllowAddingCards() throws TimeoutException {
mPaymentRequestTestRule.triggerUIAndWait(mPaymentRequestTestRule.getReadyForInput());
mPaymentRequestTestRule.clickAndWait(
R.id.button_primary, mPaymentRequestTestRule.getReadyForUnmaskInput());
// Confirm that "Add Card" option is available.
Assert.assertNotNull(mPaymentRequestTestRule.getPaymentRequestUI()
.getPaymentMethodSectionForTest()
.findViewById(R.id.payments_add_option_button));
mPaymentRequestTestRule.setTextInCardUnmaskDialogAndWait(
R.id.card_unmask_input, "123", mPaymentRequestTestRule.getReadyToUnmask());
mPaymentRequestTestRule.clickCardUnmaskButtonAndWait(
ModalDialogProperties.ButtonType.POSITIVE,
mPaymentRequestTestRule.getPaymentResponseReady());
// Confirm that "Add Card" option does not exist during retry.
mPaymentRequestTestRule.retryPaymentRequest("{}", mPaymentRequestTestRule.getReadyToPay());
Assert.assertNull(mPaymentRequestTestRule.getPaymentRequestUI()
.getPaymentMethodSectionForTest()
.findViewById(R.id.payments_add_option_button));
}
/** /**
* Test for retry() with default error message * Test for retry() with default error message
*/ */
......
...@@ -179,7 +179,8 @@ PaymentMethodViewController::PaymentMethodViewController( ...@@ -179,7 +179,8 @@ PaymentMethodViewController::PaymentMethodViewController(
PaymentRequestState* state, PaymentRequestState* state,
PaymentRequestDialogView* dialog) PaymentRequestDialogView* dialog)
: PaymentRequestSheetController(spec, state, dialog), : PaymentRequestSheetController(spec, state, dialog),
payment_method_list_(dialog) { payment_method_list_(dialog),
enable_add_card_(!state->is_retry_called()) {
const std::vector<std::unique_ptr<PaymentApp>>& available_apps = const std::vector<std::unique_ptr<PaymentApp>>& available_apps =
state->available_apps(); state->available_apps();
for (const auto& app : available_apps) { for (const auto& app : available_apps) {
...@@ -242,4 +243,8 @@ int PaymentMethodViewController::GetSecondaryButtonId() { ...@@ -242,4 +243,8 @@ int PaymentMethodViewController::GetSecondaryButtonId() {
return static_cast<int>(DialogViewID::PAYMENT_METHOD_ADD_CARD_BUTTON); return static_cast<int>(DialogViewID::PAYMENT_METHOD_ADD_CARD_BUTTON);
} }
bool PaymentMethodViewController::ShouldShowSecondaryButton() {
return enable_add_card_;
}
} // namespace payments } // namespace payments
...@@ -33,9 +33,13 @@ class PaymentMethodViewController : public PaymentRequestSheetController { ...@@ -33,9 +33,13 @@ class PaymentMethodViewController : public PaymentRequestSheetController {
base::string16 GetSecondaryButtonLabel() override; base::string16 GetSecondaryButtonLabel() override;
int GetSecondaryButtonTag() override; int GetSecondaryButtonTag() override;
int GetSecondaryButtonId() override; int GetSecondaryButtonId() override;
bool ShouldShowSecondaryButton() override;
PaymentRequestItemList payment_method_list_; PaymentRequestItemList payment_method_list_;
// Whether or not adding a new card is allowed.
bool enable_add_card_;
DISALLOW_COPY_AND_ASSIGN(PaymentMethodViewController); DISALLOW_COPY_AND_ASSIGN(PaymentMethodViewController);
}; };
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "base/strings/utf_string_conversions.h"
#include "chrome/browser/ui/views/payments/payment_request_browsertest_base.h"
#include "chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h"
#include "components/autofill/core/browser/autofill_test_utils.h"
namespace payments {
class PaymentRequestRetryTest : public PaymentRequestBrowserTestBase {
protected:
PaymentRequestRetryTest() = default;
};
IN_PROC_BROWSER_TEST_F(PaymentRequestRetryTest,
DoNotAllowPaymentInstrumentChange) {
NavigateTo("/payment_request_retry_with_payer_errors.html");
autofill::AutofillProfile contact = autofill::test::GetFullProfile();
AddAutofillProfile(contact);
// Add two credit cards and record usage for the Visa card to ensure that it
// gets preselected.
autofill::CreditCard visa_card = autofill::test::GetCreditCard();
visa_card.set_billing_address_id(contact.guid());
visa_card.RecordAndLogUse();
AddCreditCard(visa_card);
autofill::CreditCard amex_card = autofill::test::GetCreditCard2();
amex_card.set_billing_address_id(contact.guid());
AddCreditCard(amex_card);
// Confirm that there are two payment apps available.
InvokePaymentRequestUI();
PaymentRequest* request = GetPaymentRequests(GetActiveWebContents()).front();
EXPECT_EQ(2U, request->state()->available_apps().size());
// Enter a valid CVC format for the Visa card.
PayWithCreditCard(base::ASCIIToUTF16("123"));
// Confirm that only one payment app is available for retry().
RetryPaymentRequest(
"{"
" payer: {"
" email: 'EMAIL ERROR',"
" name: 'NAME ERROR',"
" phone: 'PHONE ERROR'"
" }"
"}",
DialogEvent::CONTACT_INFO_EDITOR_OPENED, dialog_view());
EXPECT_EQ(1U, request->state()->available_apps().size());
}
IN_PROC_BROWSER_TEST_F(PaymentRequestRetryTest, DisableAddCardDuringRetry) {
NavigateTo("/payment_request_retry_with_payer_errors.html");
autofill::AutofillProfile contact = autofill::test::GetFullProfile();
AddAutofillProfile(contact);
autofill::CreditCard visa_card = autofill::test::GetCreditCard();
visa_card.set_billing_address_id(contact.guid());
AddCreditCard(visa_card);
// Confirm that "Add card" button is enabled in payment list view.
InvokePaymentRequestUI();
OpenPaymentMethodScreen();
views::View* add_card_button = dialog_view()->GetViewByID(
static_cast<int>(DialogViewID::PAYMENT_METHOD_ADD_CARD_BUTTON));
EXPECT_TRUE(add_card_button);
EXPECT_TRUE(add_card_button->GetEnabled());
ClickOnBackArrow();
// Enter a valid CVC format for the Visa card.
PayWithCreditCard(base::ASCIIToUTF16("123"));
// Confirm that "Add card" button does not exist in payment list view.
RetryPaymentRequest(
"{"
" payer: {"
" email: 'EMAIL ERROR',"
" name: 'NAME ERROR',"
" phone: 'PHONE ERROR'"
" }"
"}",
DialogEvent::CONTACT_INFO_EDITOR_OPENED, dialog_view());
OpenPaymentMethodScreen();
add_card_button = dialog_view()->GetViewByID(
static_cast<int>(DialogViewID::PAYMENT_METHOD_ADD_CARD_BUTTON));
EXPECT_EQ(nullptr, add_card_button);
}
} // namespace payments
...@@ -1998,6 +1998,7 @@ if (!is_android) { ...@@ -1998,6 +1998,7 @@ if (!is_android) {
"../browser/ui/views/payments/payment_request_no_update_with_browsertest.cc", "../browser/ui/views/payments/payment_request_no_update_with_browsertest.cc",
"../browser/ui/views/payments/payment_request_payment_app_browsertest.cc", "../browser/ui/views/payments/payment_request_payment_app_browsertest.cc",
"../browser/ui/views/payments/payment_request_payment_response_browsertest.cc", "../browser/ui/views/payments/payment_request_payment_response_browsertest.cc",
"../browser/ui/views/payments/payment_request_retry_browsertest.cc",
"../browser/ui/views/payments/payment_request_shipping_address_instance_browsertest.cc", "../browser/ui/views/payments/payment_request_shipping_address_instance_browsertest.cc",
"../browser/ui/views/payments/payment_request_show_promise_browsertest.cc", "../browser/ui/views/payments/payment_request_show_promise_browsertest.cc",
"../browser/ui/views/payments/payment_request_update_with_browsertest.cc", "../browser/ui/views/payments/payment_request_update_with_browsertest.cc",
......
...@@ -310,6 +310,7 @@ void PaymentRequest::Retry(mojom::PaymentValidationErrorsPtr errors) { ...@@ -310,6 +310,7 @@ void PaymentRequest::Retry(mojom::PaymentValidationErrorsPtr errors) {
return; return;
} }
state()->SetAvailablePaymentAppForRetry();
spec()->Retry(std::move(errors)); spec()->Retry(std::move(errors));
display_handle_->Retry(); display_handle_->Retry();
} }
......
...@@ -331,6 +331,15 @@ void PaymentRequestState::RecordUseStats() { ...@@ -331,6 +331,15 @@ void PaymentRequestState::RecordUseStats() {
selected_app_->RecordUse(); selected_app_->RecordUse();
} }
void PaymentRequestState::SetAvailablePaymentAppForRetry() {
DCHECK(selected_app_);
base::EraseIf(available_apps_, [this](const auto& payment_app) {
// Remove the app if it is not selected.
return payment_app.get() != selected_app_;
});
is_retry_called_ = true;
}
void PaymentRequestState::AddAutofillPaymentApp( void PaymentRequestState::AddAutofillPaymentApp(
bool selected, bool selected,
const autofill::CreditCard& card) { const autofill::CreditCard& card) {
......
...@@ -176,6 +176,9 @@ class PaymentRequestState : public PaymentAppFactory::Delegate, ...@@ -176,6 +176,9 @@ class PaymentRequestState : public PaymentAppFactory::Delegate,
// Record the use of the data models that were used in the Payment Request. // Record the use of the data models that were used in the Payment Request.
void RecordUseStats(); void RecordUseStats();
// Sets selected app as the only available app for retry.
void SetAvailablePaymentAppForRetry();
// Gets the Autofill Profile representing the shipping address or contact // Gets the Autofill Profile representing the shipping address or contact
// information currently selected for this PaymentRequest flow. Can return // information currently selected for this PaymentRequest flow. Can return
// null. // null.
...@@ -250,6 +253,8 @@ class PaymentRequestState : public PaymentAppFactory::Delegate, ...@@ -250,6 +253,8 @@ class PaymentRequestState : public PaymentAppFactory::Delegate,
return are_requested_methods_supported_; return are_requested_methods_supported_;
} }
bool is_retry_called() const { return is_retry_called_; }
const std::string& GetApplicationLocale(); const std::string& GetApplicationLocale();
autofill::PersonalDataManager* GetPersonalDataManager(); autofill::PersonalDataManager* GetPersonalDataManager();
autofill::RegionDataLoader* GetRegionDataLoader(); autofill::RegionDataLoader* GetRegionDataLoader();
...@@ -353,6 +358,9 @@ class PaymentRequestState : public PaymentAppFactory::Delegate, ...@@ -353,6 +358,9 @@ class PaymentRequestState : public PaymentAppFactory::Delegate,
// Whether the data is currently being validated by the merchant. // Whether the data is currently being validated by the merchant.
bool is_waiting_for_merchant_validation_ = false; bool is_waiting_for_merchant_validation_ = false;
// Whether retry() has been called by the merchant.
bool is_retry_called_ = false;
const std::string app_locale_; const std::string app_locale_;
// Not owned. Never null. Will outlive this object. // Not owned. Never null. Will outlive this object.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment