1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-25 16:22:43 +01:00

Improve UI/UX when users try to add an invalid card with Stripe

Summary: Ref T13244. See PHI1052. Our error handling for Stripe errors isn't great right now. We can give users a bit more information, and a less jarring UI.

Test Plan:
Before (this is in developer mode, production doesn't get a stack trace):

{F6197394}

After:

{F6197397}

- Tried all the invalid test codes listed here: https://stripe.com/docs/testing#cards

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13244

Differential Revision: https://secure.phabricator.com/D20132
This commit is contained in:
epriestley 2019-02-08 07:12:39 -08:00
parent ae54af32c1
commit 2b718d78bb
8 changed files with 149 additions and 30 deletions

View file

@ -9,7 +9,7 @@ return array(
'names' => array( 'names' => array(
'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.css' => '3c8a0668',
'conpherence.pkg.js' => '020aebcf', 'conpherence.pkg.js' => '020aebcf',
'core.pkg.css' => 'eab5ccaf', 'core.pkg.css' => '08baca0c',
'core.pkg.js' => '5c737607', 'core.pkg.js' => '5c737607',
'differential.pkg.css' => 'b8df73d4', 'differential.pkg.css' => 'b8df73d4',
'differential.pkg.js' => '67c9ea4c', 'differential.pkg.js' => '67c9ea4c',
@ -30,7 +30,7 @@ return array(
'rsrc/css/aphront/notification.css' => '30240bd2', 'rsrc/css/aphront/notification.css' => '30240bd2',
'rsrc/css/aphront/panel-view.css' => '46923d46', 'rsrc/css/aphront/panel-view.css' => '46923d46',
'rsrc/css/aphront/phabricator-nav-view.css' => 'f8a0c1bf', 'rsrc/css/aphront/phabricator-nav-view.css' => 'f8a0c1bf',
'rsrc/css/aphront/table-view.css' => '76eda3f8', 'rsrc/css/aphront/table-view.css' => 'daa1f9df',
'rsrc/css/aphront/tokenizer.css' => 'b52d0668', 'rsrc/css/aphront/tokenizer.css' => 'b52d0668',
'rsrc/css/aphront/tooltip.css' => 'e3f2412f', 'rsrc/css/aphront/tooltip.css' => 'e3f2412f',
'rsrc/css/aphront/typeahead-browse.css' => 'b7ed02d2', 'rsrc/css/aphront/typeahead-browse.css' => 'b7ed02d2',
@ -519,7 +519,7 @@ return array(
'aphront-list-filter-view-css' => 'feb64255', 'aphront-list-filter-view-css' => 'feb64255',
'aphront-multi-column-view-css' => 'fbc00ba3', 'aphront-multi-column-view-css' => 'fbc00ba3',
'aphront-panel-view-css' => '46923d46', 'aphront-panel-view-css' => '46923d46',
'aphront-table-view-css' => '76eda3f8', 'aphront-table-view-css' => 'daa1f9df',
'aphront-tokenizer-control-css' => 'b52d0668', 'aphront-tokenizer-control-css' => 'b52d0668',
'aphront-tooltip-css' => 'e3f2412f', 'aphront-tooltip-css' => 'e3f2412f',
'aphront-typeahead-control-css' => '8779483d', 'aphront-typeahead-control-css' => '8779483d',

View file

@ -5015,6 +5015,7 @@ phutil_register_library_map(array(
'PhortuneCurrencySerializer' => 'applications/phortune/currency/PhortuneCurrencySerializer.php', 'PhortuneCurrencySerializer' => 'applications/phortune/currency/PhortuneCurrencySerializer.php',
'PhortuneCurrencyTestCase' => 'applications/phortune/currency/__tests__/PhortuneCurrencyTestCase.php', 'PhortuneCurrencyTestCase' => 'applications/phortune/currency/__tests__/PhortuneCurrencyTestCase.php',
'PhortuneDAO' => 'applications/phortune/storage/PhortuneDAO.php', 'PhortuneDAO' => 'applications/phortune/storage/PhortuneDAO.php',
'PhortuneDisplayException' => 'applications/phortune/exception/PhortuneDisplayException.php',
'PhortuneErrCode' => 'applications/phortune/constants/PhortuneErrCode.php', 'PhortuneErrCode' => 'applications/phortune/constants/PhortuneErrCode.php',
'PhortuneInvoiceView' => 'applications/phortune/view/PhortuneInvoiceView.php', 'PhortuneInvoiceView' => 'applications/phortune/view/PhortuneInvoiceView.php',
'PhortuneLandingController' => 'applications/phortune/controller/PhortuneLandingController.php', 'PhortuneLandingController' => 'applications/phortune/controller/PhortuneLandingController.php',
@ -11263,6 +11264,7 @@ phutil_register_library_map(array(
'PhortuneCurrencySerializer' => 'PhabricatorLiskSerializer', 'PhortuneCurrencySerializer' => 'PhabricatorLiskSerializer',
'PhortuneCurrencyTestCase' => 'PhabricatorTestCase', 'PhortuneCurrencyTestCase' => 'PhabricatorTestCase',
'PhortuneDAO' => 'PhabricatorLiskDAO', 'PhortuneDAO' => 'PhabricatorLiskDAO',
'PhortuneDisplayException' => 'Exception',
'PhortuneErrCode' => 'PhortuneConstants', 'PhortuneErrCode' => 'PhortuneConstants',
'PhortuneInvoiceView' => 'AphrontTagView', 'PhortuneInvoiceView' => 'AphrontTagView',
'PhortuneLandingController' => 'PhortuneController', 'PhortuneLandingController' => 'PhortuneController',

View file

@ -76,6 +76,7 @@ final class FundBacker extends FundDAO
public function getCapabilities() { public function getCapabilities() {
return array( return array(
PhabricatorPolicyCapability::CAN_VIEW, PhabricatorPolicyCapability::CAN_VIEW,
PhabricatorPolicyCapability::CAN_EDIT,
); );
} }
@ -91,6 +92,8 @@ final class FundBacker extends FundDAO
return $initiative->getPolicy($capability); return $initiative->getPolicy($capability);
} }
return PhabricatorPolicies::POLICY_NOONE; return PhabricatorPolicies::POLICY_NOONE;
case PhabricatorPolicyCapability::CAN_EDIT:
return PhabricatorPolicies::POLICY_NOONE;
} }
} }

View file

@ -73,6 +73,7 @@ final class PhortunePaymentMethodCreateController
$provider = $providers[$provider_id]; $provider = $providers[$provider_id];
$errors = array(); $errors = array();
$display_exception = null;
if ($request->isFormPost() && $request->getBool('isProviderForm')) { if ($request->isFormPost() && $request->getBool('isProviderForm')) {
$method = id(new PhortunePaymentMethod()) $method = id(new PhortunePaymentMethod())
->setAccountPHID($account->getPHID()) ->setAccountPHID($account->getPHID())
@ -107,14 +108,23 @@ final class PhortunePaymentMethodCreateController
} }
if (!$errors) { if (!$errors) {
$errors = $provider->createPaymentMethodFromRequest( try {
$request, $provider->createPaymentMethodFromRequest(
$method, $request,
$client_token); $method,
$client_token);
} catch (PhortuneDisplayException $exception) {
$display_exception = $exception;
} catch (Exception $ex) {
$errors = array(
pht('There was an error adding this payment method:'),
$ex->getMessage(),
);
}
} }
} }
if (!$errors) { if (!$errors && !$display_exception) {
$method->save(); $method->save();
// If we added this method on a cart flow, return to the cart to // If we added this method on a cart flow, return to the cart to
@ -133,13 +143,17 @@ final class PhortunePaymentMethodCreateController
return id(new AphrontRedirectResponse())->setURI($next_uri); return id(new AphrontRedirectResponse())->setURI($next_uri);
} else { } else {
$dialog = id(new AphrontDialogView()) if ($display_exception) {
->setUser($viewer) $dialog_body = $display_exception->getView();
->setTitle(pht('Error Adding Payment Method')) } else {
->appendChild(id(new PHUIInfoView())->setErrors($errors)) $dialog_body = id(new PHUIInfoView())
->addCancelButton($request->getRequestURI()); ->setErrors($errors);
}
return id(new AphrontDialogResponse())->setDialog($dialog); return $this->newDialog()
->setTitle(pht('Error Adding Payment Method'))
->appendChild($dialog_body)
->addCancelButton($request->getRequestURI());
} }
} }

View file

@ -0,0 +1,15 @@
<?php
final class PhortuneDisplayException
extends Exception {
public function setView($view) {
$this->view = $view;
return $this;
}
public function getView() {
return $this->view;
}
}

View file

@ -233,8 +233,6 @@ final class PhortuneStripePaymentProvider extends PhortunePaymentProvider {
array $token) { array $token) {
$this->loadStripeAPILibraries(); $this->loadStripeAPILibraries();
$errors = array();
$secret_key = $this->getSecretKey(); $secret_key = $this->getSecretKey();
$stripe_token = $token['stripeCardToken']; $stripe_token = $token['stripeCardToken'];
@ -253,7 +251,15 @@ final class PhortuneStripePaymentProvider extends PhortunePaymentProvider {
// the card more than once. We create one Customer for each card; // the card more than once. We create one Customer for each card;
// they do not map to PhortuneAccounts because we allow an account to // they do not map to PhortuneAccounts because we allow an account to
// have more than one active card. // have more than one active card.
$customer = Stripe_Customer::create($params, $secret_key); try {
$customer = Stripe_Customer::create($params, $secret_key);
} catch (Stripe_CardError $ex) {
$display_exception = $this->newDisplayExceptionFromCardError($ex);
if ($display_exception) {
throw $display_exception;
}
throw $ex;
}
$card = $info->card; $card = $info->card;
@ -267,8 +273,6 @@ final class PhortuneStripePaymentProvider extends PhortunePaymentProvider {
'stripe.customerID' => $customer->id, 'stripe.customerID' => $customer->id,
'stripe.cardToken' => $stripe_token, 'stripe.cardToken' => $stripe_token,
)); ));
return $errors;
} }
public function renderCreatePaymentMethodForm( public function renderCreatePaymentMethodForm(
@ -383,4 +387,84 @@ final class PhortuneStripePaymentProvider extends PhortunePaymentProvider {
require_once $root.'/externals/stripe-php/lib/Stripe.php'; require_once $root.'/externals/stripe-php/lib/Stripe.php';
} }
private function newDisplayExceptionFromCardError(Stripe_CardError $ex) {
$body = $ex->getJSONBody();
if (!$body) {
return null;
}
$map = idx($body, 'error');
if (!$map) {
return null;
}
$view = array();
$message = idx($map, 'message');
$view[] = id(new PHUIInfoView())
->setErrors(array($message));
$view[] = phutil_tag(
'div',
array(
'class' => 'mlt mlb',
),
pht('Additional details about this error:'));
$rows = array();
$rows[] = array(
pht('Error Code'),
idx($map, 'code'),
);
$rows[] = array(
pht('Error Type'),
idx($map, 'type'),
);
$param = idx($map, 'param');
if (strlen($param)) {
$rows[] = array(
pht('Error Param'),
$param,
);
}
$decline_code = idx($map, 'decline_code');
if (strlen($decline_code)) {
$rows[] = array(
pht('Decline Code'),
$decline_code,
);
}
$doc_url = idx($map, 'doc_url');
if ($doc_url) {
$rows[] = array(
pht('Learn More'),
phutil_tag(
'a',
array(
'href' => $doc_url,
'target' => '_blank',
),
$doc_url),
);
}
$view[] = id(new AphrontTableView($rows))
->setColumnClasses(
array(
'header',
'wide',
));
return id(new PhortuneDisplayException(get_class($ex)))
->setView($view);
}
} }

View file

@ -175,9 +175,10 @@ final class PhabricatorPolicyFilter extends Phobject {
if (!in_array($capability, $object_capabilities)) { if (!in_array($capability, $object_capabilities)) {
throw new Exception( throw new Exception(
pht( pht(
"Testing for capability '%s' on an object which does ". 'Testing for capability "%s" on an object ("%s") which does '.
"not have that capability!", 'not support that capability.',
$capability)); $capability,
get_class($object)));
} }
$policy = $this->getObjectPolicy($object, $capability); $policy = $this->getObjectPolicy($object, $capability);

View file

@ -45,16 +45,20 @@
background: inherit; background: inherit;
} }
.aphront-table-view th { .aphront-table-view th,
.aphront-table-view td.header {
font-weight: bold; font-weight: bold;
white-space: nowrap; white-space: nowrap;
color: {$bluetext}; color: {$bluetext};
text-shadow: 0 1px 0 white;
font-weight: bold; font-weight: bold;
border-bottom: 1px solid {$thinblueborder}; text-shadow: 0 1px 0 white;
background-color: {$lightbluebackground}; background-color: {$lightbluebackground};
} }
.aphront-table-view th {
border-bottom: 1px solid {$thinblueborder};
}
th.aphront-table-view-sortable-selected { th.aphront-table-view-sortable-selected {
background-color: {$greybackground}; background-color: {$greybackground};
} }
@ -74,12 +78,8 @@ th.aphront-table-view-sortable-selected {
} }
.aphront-table-view td.header { .aphront-table-view td.header {
padding: 4px 8px;
white-space: nowrap;
text-align: right; text-align: right;
color: {$bluetext}; border-right: 1px solid {$thinblueborder};
font-weight: bold;
vertical-align: top;
} }
.aphront-table-view td { .aphront-table-view td {