From 7a5f62282017acd8a33aa7d2e1787fcd931f6268 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 25 Apr 2013 09:49:32 -0700 Subject: [PATCH] General cleanup for adding payment methods in Phortune Summary: This has no real behavioral changes (except better error handling), it just factors things out to be a bit cleaner. In particular: - Move more shared form behaviors into the common JS form component. - Move more error handling into shared pathways. - Make the specialized Stripe / Balanced methods do less work. This needs some more polish for nontrival errors (especially on the Balanced side) but none of the error behavior is worse than it was and a lot of it is much better. Ref T2787. Test Plan: Hit all invalid form errors, added valid payment methods with Stripe and Balacned. Reviewers: btrahan, chad Reviewed By: btrahan CC: aran Maniphest Tasks: T2787 Differential Revision: https://secure.phabricator.com/D5771 --- .../20130423.phortunepaymentrevised.sql | 19 ++ src/__celerity_resource_map__.php | 17 +- src/__phutil_library_map__.php | 3 + .../phortune/constants/PhortuneConstants.php | 5 + .../phortune/constants/PhortuneErrCode.php | 11 + .../PhortuneAccountViewController.php | 6 +- .../PhortunePaymentMethodEditController.php | 90 +++++++- .../PhortuneBalancedPaymentProvider.php | 150 ++++++------- .../provider/PhortunePaymentProvider.php | 27 ++- .../PhortuneStripePaymentProvider.php | 206 +++++++++--------- .../storage/PhortunePaymentMethod.php | 15 +- .../phortune/view/PhortuneCreditCardForm.php | 34 +-- .../patch/PhabricatorBuiltinPatchList.php | 4 + .../behavior-balanced-payment-form.js | 56 ++--- .../phortune/behavior-stripe-payment-form.js | 54 ++--- .../phortune/phortune-credit-card-form.js | 33 ++- 16 files changed, 443 insertions(+), 287 deletions(-) create mode 100644 resources/sql/patches/20130423.phortunepaymentrevised.sql create mode 100644 src/applications/phortune/constants/PhortuneConstants.php create mode 100644 src/applications/phortune/constants/PhortuneErrCode.php diff --git a/resources/sql/patches/20130423.phortunepaymentrevised.sql b/resources/sql/patches/20130423.phortunepaymentrevised.sql new file mode 100644 index 0000000000..e497b01535 --- /dev/null +++ b/resources/sql/patches/20130423.phortunepaymentrevised.sql @@ -0,0 +1,19 @@ +TRUNCATE TABLE {$NAMESPACE}_phortune.phortune_paymentmethod; + +ALTER TABLE {$NAMESPACE}_phortune.phortune_paymentmethod + ADD brand VARCHAR(64) NOT NULL; + +ALTER TABLE {$NAMESPACE}_phortune.phortune_paymentmethod + ADD expires VARCHAR(16) NOT NULL; + +ALTER TABLE {$NAMESPACE}_phortune.phortune_paymentmethod + ADD providerType VARCHAR(16) NOT NULL; + +ALTER TABLE {$NAMESPACE}_phortune.phortune_paymentmethod + ADD providerDomain VARCHAR(64) NOT NULL; + +ALTER TABLE {$NAMESPACE}_phortune.phortune_paymentmethod + ADD lastFourDigits VARCHAR(16) NOT NULL; + +ALTER TABLE {$NAMESPACE}_phortune.phortune_paymentmethod + DROP expiresEpoch; diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index a1c0f375ff..0eedf11062 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -1276,15 +1276,13 @@ celerity_register_resource_map(array( ), 'javelin-behavior-balanced-payment-form' => array( - 'uri' => '/res/2a850a31/rsrc/js/application/phortune/behavior-balanced-payment-form.js', + 'uri' => '/res/6876492d/rsrc/js/application/phortune/behavior-balanced-payment-form.js', 'type' => 'js', 'requires' => array( 0 => 'javelin-behavior', 1 => 'javelin-dom', - 2 => 'javelin-json', - 3 => 'javelin-workflow', - 4 => 'phortune-credit-card-form', + 2 => 'phortune-credit-card-form', ), 'disk' => '/rsrc/js/application/phortune/behavior-balanced-payment-form.js', ), @@ -2272,15 +2270,13 @@ celerity_register_resource_map(array( ), 'javelin-behavior-stripe-payment-form' => array( - 'uri' => '/res/2ae12d96/rsrc/js/application/phortune/behavior-stripe-payment-form.js', + 'uri' => '/res/c1a12d77/rsrc/js/application/phortune/behavior-stripe-payment-form.js', 'type' => 'js', 'requires' => array( 0 => 'javelin-behavior', 1 => 'javelin-dom', - 2 => 'javelin-json', - 3 => 'javelin-workflow', - 4 => 'phortune-credit-card-form', + 2 => 'phortune-credit-card-form', ), 'disk' => '/rsrc/js/application/phortune/behavior-stripe-payment-form.js', ), @@ -3605,12 +3601,15 @@ celerity_register_resource_map(array( ), 'phortune-credit-card-form' => array( - 'uri' => '/res/7be5799a/rsrc/js/application/phortune/phortune-credit-card-form.js', + 'uri' => '/res/bc948778/rsrc/js/application/phortune/phortune-credit-card-form.js', 'type' => 'js', 'requires' => array( 0 => 'javelin-install', 1 => 'javelin-dom', + 2 => 'javelin-json', + 3 => 'javelin-workflow', + 4 => 'javelin-util', ), 'disk' => '/rsrc/js/application/phortune/phortune-credit-card-form.js', ), diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 7e01dbb608..6dfd752d10 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1583,9 +1583,11 @@ phutil_register_library_map(array( 'PhortuneBalancedPaymentProvider' => 'applications/phortune/provider/PhortuneBalancedPaymentProvider.php', 'PhortuneCart' => 'applications/phortune/storage/PhortuneCart.php', 'PhortuneCharge' => 'applications/phortune/storage/PhortuneCharge.php', + 'PhortuneConstants' => 'applications/phortune/constants/PhortuneConstants.php', 'PhortuneController' => 'applications/phortune/controller/PhortuneController.php', 'PhortuneCreditCardForm' => 'applications/phortune/view/PhortuneCreditCardForm.php', 'PhortuneDAO' => 'applications/phortune/storage/PhortuneDAO.php', + 'PhortuneErrCode' => 'applications/phortune/constants/PhortuneErrCode.php', 'PhortuneLandingController' => 'applications/phortune/controller/PhortuneLandingController.php', 'PhortuneMonthYearExpiryControl' => 'applications/phortune/control/PhortuneMonthYearExpiryControl.php', 'PhortuneMultiplePaymentProvidersException' => 'applications/phortune/exception/PhortuneMultiplePaymentProvidersException.php', @@ -3308,6 +3310,7 @@ phutil_register_library_map(array( 'PhortuneCharge' => 'PhortuneDAO', 'PhortuneController' => 'PhabricatorController', 'PhortuneDAO' => 'PhabricatorLiskDAO', + 'PhortuneErrCode' => 'PhortuneConstants', 'PhortuneLandingController' => 'PhortuneController', 'PhortuneMonthYearExpiryControl' => 'AphrontFormControl', 'PhortuneMultiplePaymentProvidersException' => 'Exception', diff --git a/src/applications/phortune/constants/PhortuneConstants.php b/src/applications/phortune/constants/PhortuneConstants.php new file mode 100644 index 0000000000..788d8867a0 --- /dev/null +++ b/src/applications/phortune/constants/PhortuneConstants.php @@ -0,0 +1,5 @@ +setHeader($method->getName()); + $item->setHeader($method->getBrand().' / '.$method->getLastFourDigits()); switch ($method->getStatus()) { case PhortunePaymentMethod::STATUS_ACTIVE: @@ -126,10 +126,6 @@ final class PhortuneAccountViewController extends PhortuneController { phabricator_datetime($method->getDateCreated(), $user), $this->getHandle($method->getAuthorPHID())->renderLink())); - if ($method->getExpiresEpoch() < time() + (60 * 60 * 24 * 30)) { - $item->addAttribute(pht('Expires Soon!')); - } - $list->addItem($item); } diff --git a/src/applications/phortune/controller/PhortunePaymentMethodEditController.php b/src/applications/phortune/controller/PhortunePaymentMethodEditController.php index 4623ea67d1..387fd0b0e0 100644 --- a/src/applications/phortune/controller/PhortunePaymentMethodEditController.php +++ b/src/applications/phortune/controller/PhortunePaymentMethodEditController.php @@ -48,9 +48,38 @@ final class PhortunePaymentMethodEditController ->setAccountPHID($account->getPHID()) ->setAuthorPHID($user->getPHID()) ->setStatus(PhortunePaymentMethod::STATUS_ACTIVE) - ->setMetadataValue('providerKey', $provider->getProviderKey()); + ->setProviderType($provider->getProviderType()) + ->setProviderDomain($provider->getProviderDomain()); - $errors = $provider->createPaymentMethodFromRequest($request, $method); + if (!$errors) { + $errors = $this->processClientErrors( + $provider, + $request->getStr('errors')); + } + + if (!$errors) { + $client_token_raw = $request->getStr('token'); + $client_token = json_decode($client_token_raw, true); + if (!is_array($client_token)) { + $errors[] = pht( + 'There was an error decoding token information submitted by the '. + 'client. Expected a JSON-encoded token dictionary, received: %s.', + nonempty($client_token_raw, pht('nothing'))); + } else { + if (!$provider->validateCreatePaymentMethodToken($client_token)) { + $errors[] = pht( + 'There was an error with the payment token submitted by the '. + 'client. Expected a valid dictionary, received: %s.', + $client_token_raw); + } + } + if (!$errors) { + $errors = $provider->createPaymentMethodFromRequest( + $request, + $method, + $client_token); + } + } if (!$errors) { $method->save(); @@ -152,4 +181,61 @@ final class PhortunePaymentMethodEditController )); } + private function processClientErrors( + PhortunePaymentProvider $provider, + $client_errors_raw) { + + $errors = array(); + + $client_errors = json_decode($client_errors_raw, true); + if (!is_array($client_errors)) { + $errors[] = pht( + 'There was an error decoding error information submitted by the '. + 'client. Expected a JSON-encoded list of error codes, received: %s.', + nonempty($client_errors_raw, pht('nothing'))); + } + + foreach (array_unique($client_errors) as $key => $client_error) { + $client_errors[$key] = $provider->translateCreatePaymentMethodErrorCode( + $client_error); + } + + foreach (array_unique($client_errors) as $client_error) { + switch ($client_error) { + case PhortuneErrCode::ERR_CC_INVALID_NUMBER: + $message = pht( + 'The card number you entered is not a valid card number. Check '. + 'that you entered it correctly.'); + break; + case PhortuneErrCode::ERR_CC_INVALID_CVC: + $message = pht( + 'The CVC code you entered is not a valid CVC code. Check that '. + 'you entered it correctly. The CVC code is a 3-digit or 4-digit '. + 'numeric code which usually appears on the back of the card.'); + break; + case PhortuneErrCode::ERR_CC_INVALID_EXPIRY: + $message = pht( + 'The card expiration date is not a valid expiration date. Check '. + 'that you entered it correctly. You can not add an expired card '. + 'as a payment method.'); + break; + default: + $message = $provider->getCreatePaymentErrorMessage($client_error); + if (!$message) { + $message = pht( + "There was an unexpected error ('%s') processing payment ". + "information.", + $client_error); + + phlog($message); + } + break; + } + + $errors[$client_error] = $message; + } + + return $errors; + } + } diff --git a/src/applications/phortune/provider/PhortuneBalancedPaymentProvider.php b/src/applications/phortune/provider/PhortuneBalancedPaymentProvider.php index ae18b6aa5e..3cea5b87fb 100644 --- a/src/applications/phortune/provider/PhortuneBalancedPaymentProvider.php +++ b/src/applications/phortune/provider/PhortuneBalancedPaymentProvider.php @@ -55,71 +55,61 @@ final class PhortuneBalancedPaymentProvider extends PhortunePaymentProvider { return true; } + public function validateCreatePaymentMethodToken(array $token) { + return isset($token['balancedMarketplaceURI']); + } + /** + * @phutil-external-symbol class Balanced\Card * @phutil-external-symbol class Balanced\Settings * @phutil-external-symbol class Balanced\Marketplace * @phutil-external-symbol class RESTful\Exceptions\HTTPError */ public function createPaymentMethodFromRequest( AphrontRequest $request, - PhortunePaymentMethod $method) { - - $card_errors = $request->getStr('cardErrors'); - $balanced_data = $request->getStr('balancedCardData'); + PhortunePaymentMethod $method, + array $token) { $errors = array(); - if ($card_errors) { - $raw_errors = json_decode($card_errors); - $errors = $this->parseRawCreatePaymentMethodErrors($raw_errors); + + $root = dirname(phutil_get_library_root('phabricator')); + require_once $root.'/externals/httpful/bootstrap.php'; + require_once $root.'/externals/restful/bootstrap.php'; + require_once $root.'/externals/balanced-php/bootstrap.php'; + + $account_phid = $method->getAccountPHID(); + $author_phid = $method->getAuthorPHID(); + $description = $account_phid.':'.$author_phid; + + try { + Balanced\Settings::$api_key = $this->getSecretKey(); + + $card = Balanced\Card::get($token['balancedMarketplaceURI']); + + $buyer = Balanced\Marketplace::mine()->createBuyer( + null, + $card->uri, + array( + 'description' => $description, + )); + + } catch (RESTful\Exceptions\HTTPError $error) { + // NOTE: This exception doesn't print anything meaningful if it escapes + // to top level. Replace it with something slightly readable. + throw new Exception($error->response->body->description); } - if (!$errors) { - $data = json_decode($balanced_data, true); - if (!is_array($data)) { - $errors[] = pht('An error occurred decoding card data.'); - } - } - - if (!$errors) { - $root = dirname(phutil_get_library_root('phabricator')); - require_once $root.'/externals/httpful/bootstrap.php'; - require_once $root.'/externals/restful/bootstrap.php'; - require_once $root.'/externals/balanced-php/bootstrap.php'; - - $account_phid = $method->getAccountPHID(); - $author_phid = $method->getAuthorPHID(); - $description = $account_phid.':'.$author_phid; - - try { - - Balanced\Settings::$api_key = $this->getSecretKey(); - $buyer = Balanced\Marketplace::mine()->createBuyer( - null, - $data['uri'], - array( - 'description' => $description, - )); - - } catch (RESTful\Exceptions\HTTPError $error) { - // NOTE: This exception doesn't print anything meaningful if it escapes - // to top level. Replace it with something slightly readable. - throw new Exception($error->response->body->description); - } - - $exp_string = $data['expiration_year'].'-'.$data['expiration_month']; - $epoch = strtotime($exp_string); - - $method - ->setName($data['brand'].' / '.$data['last_four']) - ->setExpiresEpoch($epoch) - ->setMetadata( - array( - 'type' => 'balanced.account', - 'balanced.accountURI' => $buyer->uri, - 'balanced.cardURI' => $data['uri'], - )); - } + $method + ->setBrand($card->brand) + ->setLastFourDigits($card->last_four) + ->setExpires($card->expiration_year, $card->expiration_month) + ->setMetadata( + array( + 'type' => 'balanced.account', + 'balanced.accountURI' => $buyer->uri, + 'balanced.cardURI' => $card->uri, + )); return $errors; } @@ -130,9 +120,7 @@ final class PhortuneBalancedPaymentProvider extends PhortunePaymentProvider { $ccform = id(new PhortuneCreditCardForm()) ->setUser($request->getUser()) - ->setCardNumberError(isset($errors['number']) ? pht('Invalid') : true) - ->setCardCVCError(isset($errors['cvc']) ? pht('Invalid') : true) - ->setCardExpirationError(isset($errors['exp']) ? pht('Invalid') : null) + ->setErrors($errors) ->addScript('https://js.balancedpayments.com/v1/balanced.js'); Javelin::initBehavior( @@ -145,27 +133,43 @@ final class PhortuneBalancedPaymentProvider extends PhortunePaymentProvider { return $ccform->buildForm(); } - private function parseRawCreatePaymentMethodErrors(array $raw_errors) { - $errors = array(); + private function getBalancedShortErrorCode($error_code) { + $prefix = 'cc:balanced:'; + if (strncmp($error_code, $prefix, strlen($prefix))) { + return null; + } + return substr($error_code, strlen($prefix)); + } - foreach ($raw_errors as $error) { - switch ($error) { - case 'number': - $errors[$error] = pht('Card number is incorrect or invalid.'); - break; - case 'cvc': - $errors[$error] = pht('CVC code is incorrect or invalid.'); - break; - case 'exp': - $errors[$error] = pht('Card expiration date is incorrect.'); - break; - default: - $errors[] = $error; - break; + public function translateCreatePaymentMethodErrorCode($error_code) { + $short_code = $this->getBalancedShortErrorCode($error_code); + + if ($short_code) { + static $map = array( + ); + + if (isset($map[$short_code])) { + return $map[$short_code]; } } - return $errors; + return $error_code; + } + + public function getCreatePaymentMethodErrorMessage($error_code) { + $short_code = $this->getBalancedShortErrorCode($error_code); + if (!$short_code) { + return null; + } + + switch ($short_code) { + + default: + break; + } + + + return null; } } diff --git a/src/applications/phortune/provider/PhortunePaymentProvider.php b/src/applications/phortune/provider/PhortunePaymentProvider.php index 036c6f2b26..c0ca9cd492 100644 --- a/src/applications/phortune/provider/PhortunePaymentProvider.php +++ b/src/applications/phortune/provider/PhortunePaymentProvider.php @@ -92,12 +92,37 @@ abstract class PhortunePaymentProvider { } + /** + * @task addmethod + */ + public function translateCreatePaymentMethodErrorCode($error_code) { + throw new PhortuneNotImplementedException($this); + } + + + /** + * @task addmethod + */ + public function getCreatePaymentMethodErrorMessage($error_code) { + throw new PhortuneNotImplementedException($this); + } + + + /** + * @task addmethod + */ + public function validateCreatePaymentMethodToken(array $token) { + throw new PhortuneNotImplementedException($this); + } + + /** * @task addmethod */ public function createPaymentMethodFromRequest( AphrontRequest $request, - PhortunePaymentMethod $method) { + PhortunePaymentMethod $method, + array $token) { throw new PhortuneNotImplementedException($this); } diff --git a/src/applications/phortune/provider/PhortuneStripePaymentProvider.php b/src/applications/phortune/provider/PhortuneStripePaymentProvider.php index 0717f3aede..eee9e11b8a 100644 --- a/src/applications/phortune/provider/PhortuneStripePaymentProvider.php +++ b/src/applications/phortune/provider/PhortuneStripePaymentProvider.php @@ -81,63 +81,45 @@ final class PhortuneStripePaymentProvider extends PhortunePaymentProvider { */ public function createPaymentMethodFromRequest( AphrontRequest $request, - PhortunePaymentMethod $method) { - - $card_errors = $request->getStr('cardErrors'); - $stripe_token = $request->getStr('stripeToken'); + PhortunePaymentMethod $method, + array $token) { $errors = array(); - if ($card_errors) { - $raw_errors = json_decode($card_errors); - $errors = $this->parseRawCreatePaymentMethodErrors($raw_errors); - } - if (!$errors) { - if (!$stripe_token) { - $errors[] = pht('There was an unknown error processing your card.'); - } - } + $root = dirname(phutil_get_library_root('phabricator')); + require_once $root.'/externals/stripe-php/lib/Stripe.php'; - if (!$errors) { - $root = dirname(phutil_get_library_root('phabricator')); - require_once $root.'/externals/stripe-php/lib/Stripe.php'; + $secret_key = $this->getSecretKey(); + $stripe_token = $token['stripeCardToken']; - try { - // First, make sure the token is valid. - $secret_key = $this->getSecretKey(); + // First, make sure the token is valid. + $info = id(new Stripe_Token())->retrieve($stripe_token, $secret_key); - $info = id(new Stripe_Token())->retrieve($stripe_token, $secret_key); + $account_phid = $method->getAccountPHID(); + $author_phid = $method->getAuthorPHID(); - $account_phid = $method->getAccountPHID(); - $author_phid = $method->getAuthorPHID(); + $params = array( + 'card' => $stripe_token, + 'description' => $account_phid.':'.$author_phid, + ); - $params = array( - 'card' => $stripe_token, - 'description' => $account_phid.':'.$author_phid, - ); + // Then, we need to create a Customer in order to be able to charge + // the card more than once. We create one Customer for each card; + // they do not map to PhortuneAccounts because we allow an account to + // have more than one active card. + $customer = Stripe_Customer::create($params, $secret_key); - // Then, we need to create a Customer in order to be able to charge - // the card more than once. We create one Customer for each card; - // they do not map to PhortuneAccounts because we allow an account to - // have more than one active card. - $customer = Stripe_Customer::create($params, $secret_key); - - $card = $info->card; - $method - ->setName($card->type.' / '.$card->last4) - ->setExpiresEpoch(strtotime($card->exp_year.'-'.$card->exp_month)) - ->setMetadata( - array( - 'type' => 'stripe.customer', - 'stripe.customerID' => $customer->id, - 'stripe.tokenID' => $stripe_token, - )); - } catch (Exception $ex) { - phlog($ex); - $errors[] = pht( - 'There was an error communicating with the payments backend.'); - } - } + $card = $info->card; + $method + ->setBrand($card->type) + ->setLastFourDigits($card->last4) + ->setExpires($card->exp_year, $card->exp_month) + ->setMetadata( + array( + 'type' => 'stripe.customer', + 'stripe.customerID' => $customer->id, + 'stripe.cardToken' => $stripe_token, + )); return $errors; } @@ -148,9 +130,7 @@ final class PhortuneStripePaymentProvider extends PhortunePaymentProvider { $ccform = id(new PhortuneCreditCardForm()) ->setUser($request->getUser()) - ->setCardNumberError(isset($errors['number']) ? pht('Invalid') : true) - ->setCardCVCError(isset($errors['cvc']) ? pht('Invalid') : true) - ->setCardExpirationError(isset($errors['exp']) ? pht('Invalid') : null) + ->setErrors($errors) ->addScript('https://js.stripe.com/v2/'); Javelin::initBehavior( @@ -163,64 +143,84 @@ final class PhortuneStripePaymentProvider extends PhortunePaymentProvider { return $ccform->buildForm(); } + private function getStripeShortErrorCode($error_code) { + $prefix = 'cc:stripe:'; + if (strncmp($error_code, $prefix, strlen($prefix))) { + return null; + } + return substr($error_code, strlen($prefix)); + } - /** - * Stripe JS and calls to Stripe handle all errors with processing this - * form. This function takes the raw errors - in the form of an array - * where each elementt is $type => $message - and figures out what if - * any fields were invalid and pulls the messages into a flat object. - * - * See https://stripe.com/docs/api#errors for more information on possible - * errors. - */ - private function parseRawCreatePaymentMethodErrors(array $raw_errors) { - $errors = array(); + public function validateCreatePaymentMethodToken(array $token) { + return isset($token['stripeCardToken']); + } - foreach ($raw_errors as $type) { - $error_key = null; - $message = pht('A card processing error has occurred.'); - switch ($type) { - case 'number': - case 'invalid_number': - case 'incorrect_number': - $error_key = 'number'; - $message = pht('Invalid or incorrect credit card number.'); - break; - case 'cvc': - case 'invalid_cvc': - case 'incorrect_cvc': - $error_key = 'cvc'; - $message = pht('Card CVC is invalid or incorrect.'); - break; - case 'expiry': - case 'invalid_expiry_month': - case 'invalid_expiry_year': - $error_key = 'exp'; - $message = pht('Card expiration date is invalid or incorrect.'); - break; - case 'card_declined': - case 'expired_card': - case 'duplicate_transaction': - case 'processing_error': - // these errors don't map well to field(s) being bad - break; - case 'invalid_amount': - case 'missing': - default: - // these errors only happen if we (not the user) messed up so log it - $error = sprintf('[Stripe Error] %s', $type); - phlog($error); - break; - } + public function translateCreatePaymentMethodErrorCode($error_code) { + $short_code = $this->getStripeShortErrorCode($error_code); - if ($error_key === null || isset($errors[$error_key])) { - $errors[] = $message; - } else { - $errors[$error_key] = $message; + if ($short_code) { + static $map = array( + 'error:invalid_number' => PhortuneErrCode::ERR_CC_INVALID_NUMBER, + 'error:invalid_cvc' => PhortuneErrCode::ERR_CC_INVALID_CVC, + 'error:invalid_expiry_month' => PhortuneErrCode::ERR_CC_INVALID_EXPIRY, + 'error:invalid_expiry_year' => PhortuneErrCode::ERR_CC_INVALID_EXPIRY, + ); + + if (isset($map[$short_code])) { + return $map[$short_code]; } } - return $errors; + return $error_code; + } + + /** + * See https://stripe.com/docs/api#errors for more information on possible + * errors. + */ + public function getCreatePaymentMethodErrorMessage($error_code) { + $short_code = $this->getStripeShortErrorCode($error_code); + if (!$short_code) { + return null; + } + + switch ($short_code) { + case 'error:incorrect_number': + $error_key = 'number'; + $message = pht('Invalid or incorrect credit card number.'); + break; + case 'error:incorrect_cvc': + $error_key = 'cvc'; + $message = pht('Card CVC is invalid or incorrect.'); + break; + $error_key = 'exp'; + $message = pht('Card expiration date is invalid or incorrect.'); + break; + case 'error:invalid_expiry_month': + case 'error:invalid_expiry_year': + case 'error:invalid_cvc': + case 'error:invalid_number': + // NOTE: These should be translated into Phortune error codes earlier, + // so we don't expect to receive them here. They are listed for clarity + // and completeness. If we encounter one, we treat it as an unknown + // error. + break; + case 'error:invalid_amount': + case 'error:missing': + case 'error:card_declined': + case 'error:expired_card': + case 'error:duplicate_transaction': + case 'error:processing_error': + default: + // NOTE: These errors currently don't recevive a detailed message. + // NOTE: We can also end up here with "http:nnn" messages. + + // TODO: At least some of these should have a better message, or be + // translated into common errors above. + break; + } + + return null; } } diff --git a/src/applications/phortune/storage/PhortunePaymentMethod.php b/src/applications/phortune/storage/PhortunePaymentMethod.php index 2df720188c..67ca35154e 100644 --- a/src/applications/phortune/storage/PhortunePaymentMethod.php +++ b/src/applications/phortune/storage/PhortunePaymentMethod.php @@ -11,12 +11,16 @@ final class PhortunePaymentMethod extends PhortuneDAO const STATUS_FAILED = 'payment:failed'; const STATUS_REMOVED = 'payment:removed'; - protected $name; + protected $name = ''; protected $status; protected $accountPHID; protected $authorPHID; - protected $expiresEpoch; + protected $expires; protected $metadata = array(); + protected $brand; + protected $lastFourDigits; + protected $providerType; + protected $providerDomain; private $account; @@ -47,7 +51,7 @@ final class PhortunePaymentMethod extends PhortuneDAO } public function getDescription() { - return pht('Expires %s', date('m/y'), $this->getExpiresEpoch()); + return '...'; } public function getMetadataValue($key, $default = null) { @@ -80,6 +84,11 @@ final class PhortunePaymentMethod extends PhortuneDAO return head($accept); } + public function setExpires($year, $month) { + $this->expires = $year.'-'.$month; + return $this; + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/phortune/view/PhortuneCreditCardForm.php b/src/applications/phortune/view/PhortuneCreditCardForm.php index da042ea858..f2280c6fc3 100644 --- a/src/applications/phortune/view/PhortuneCreditCardForm.php +++ b/src/applications/phortune/view/PhortuneCreditCardForm.php @@ -5,6 +5,7 @@ final class PhortuneCreditCardForm { private $formID; private $scripts = array(); private $user; + private $errors = array(); private $cardNumberError; private $cardCVCError; @@ -15,18 +16,8 @@ final class PhortuneCreditCardForm { return $this; } - public function setCardExpirationError($card_expiration_error) { - $this->cardExpirationError = $card_expiration_error; - return $this; - } - - public function setCardCVCError($card_cvc_error) { - $this->cardCVCError = $card_cvc_error; - return $this; - } - - public function setCardNumberError($card_number_error) { - $this->cardNumberError = $card_number_error; + public function setErrors(array $errors) { + $this->errors = $errors; return $this; } @@ -63,6 +54,19 @@ final class PhortuneCreditCardForm { ))); } + $errors = $this->errors; + $e_number = isset($errors[PhortuneErrCode::ERR_CC_INVALID_NUMBER]) + ? pht('Invalid') + : true; + + $e_cvc = isset($errors[PhortuneErrCode::ERR_CC_INVALID_CVC]) + ? pht('Invalid') + : true; + + $e_expiry = isset($errors[PhortuneErrCode::ERR_CC_INVALID_EXPIRY]) + ? pht('Invalid') + : null; + $form ->setID($form_id) ->appendChild( @@ -85,18 +89,18 @@ final class PhortuneCreditCardForm { ->setLabel('Card Number') ->setDisableAutocomplete(true) ->setSigil('number-input') - ->setError($this->cardNumberError)) + ->setError($e_number)) ->appendChild( id(new AphrontFormTextControl()) ->setLabel('CVC') ->setDisableAutocomplete(true) ->setSigil('cvc-input') - ->setError($this->cardCVCError)) + ->setError($e_cvc)) ->appendChild( id(new PhortuneMonthYearExpiryControl()) ->setLabel('Expiration') ->setUser($this->user) - ->setError($this->cardExpirationError)); + ->setError($e_expiry)); return $form; } diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index e2a7b2589b..0c0b55a4f4 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1246,6 +1246,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('20130423.updateexternalaccount.sql'), ), + '20130423.phortunepaymentrevised.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('20130423.phortunepaymentrevised.sql'), + ), ); } } diff --git a/webroot/rsrc/js/application/phortune/behavior-balanced-payment-form.js b/webroot/rsrc/js/application/phortune/behavior-balanced-payment-form.js index 2b5c9b25b1..4fc731e975 100644 --- a/webroot/rsrc/js/application/phortune/behavior-balanced-payment-form.js +++ b/webroot/rsrc/js/application/phortune/behavior-balanced-payment-form.js @@ -2,70 +2,56 @@ * @provides javelin-behavior-balanced-payment-form * @requires javelin-behavior * javelin-dom - * javelin-json - * javelin-workflow * phortune-credit-card-form */ JX.behavior('balanced-payment-form', function(config) { balanced.init(config.balancedMarketplaceURI); - var root = JX.$(config.formID); - var ccform = new JX.PhortuneCreditCardForm(root); + var ccform = new JX.PhortuneCreditCardForm(JX.$(config.formID), onsubmit); - var onsubmit = function(e) { - e.kill(); - - var cardData = ccform.getCardData(); + function onsubmit(card_data) { var errors = []; - if (!balanced.card.isCardNumberValid(cardData.number)) { - errors.push('number'); + if (!balanced.card.isCardNumberValid(card_data.number)) { + errors.push('cc:invalid:number'); } - if (!balanced.card.isSecurityCodeValid(cardData.number, cardData.cvc)) { - errors.push('cvc'); + if (!balanced.card.isSecurityCodeValid(card_data.number, card_data.cvc)) { + errors.push('cc:invalid:cvc'); } - if (!balanced.card.isExpiryValid(cardData.month, cardData.year)) { - errors.push('expiry'); + if (!balanced.card.isExpiryValid(card_data.month, card_data.year)) { + errors.push('cc:invalid:expiry'); } if (errors.length) { - JX.Workflow - .newFromForm(root, {cardErrors: JX.JSON.stringify(errors)}) - .start(); + ccform.submitForm(errors); return; } var data = { - card_number: cardData.number, - security_code: cardData.cvc, - expiration_month: cardData.month, - expiration_year: cardData.year + card_number: card_data.number, + security_code: card_data.cvc, + expiration_month: card_data.month, + expiration_year: card_data.year }; balanced.card.create(data, onresponse); } - var onresponse = function(response) { - + function onresponse(response) { + var token = null; var errors = []; + if (response.error) { - errors = [response.error.type]; + errors = ['cc:balanced:error:' + response.error.type]; } else if (response.status != 201) { - errors = ['balanced:' + response.status]; + errors = ['cc:balanced:http:' + response.status]; + } else { + token = response.data.uri; } - var params = { - cardErrors: JX.JSON.stringify(errors), - balancedCardData: JX.JSON.stringify(response.data) - }; - - JX.Workflow - .newFromForm(root, params) - .start(); + ccform.submitForm(errors, {balancedMarketplaceURI: token}); } - - JX.DOM.listen(root, 'submit', null, onsubmit); }); diff --git a/webroot/rsrc/js/application/phortune/behavior-stripe-payment-form.js b/webroot/rsrc/js/application/phortune/behavior-stripe-payment-form.js index 7c1bbaef82..e3b99078fd 100644 --- a/webroot/rsrc/js/application/phortune/behavior-stripe-payment-form.js +++ b/webroot/rsrc/js/application/phortune/behavior-stripe-payment-form.js @@ -2,72 +2,56 @@ * @provides javelin-behavior-stripe-payment-form * @requires javelin-behavior * javelin-dom - * javelin-json - * javelin-workflow * phortune-credit-card-form */ JX.behavior('stripe-payment-form', function(config) { Stripe.setPublishableKey(config.stripePublishableKey); - var root = JX.$(config.formID); - var ccform = new JX.PhortuneCreditCardForm(root); + var ccform = new JX.PhortuneCreditCardForm(JX.$(config.formID), onsubmit); - var onsubmit = function(e) { - e.kill(); - - // validate the card data with Stripe client API and submit the form - // with any detected errors - var cardData = ccform.getCardData(); + function onsubmit(card_data) { var errors = []; - if (!Stripe.validateCardNumber(cardData.number)) { - errors.push('number'); + if (!Stripe.validateCardNumber(card_data.number)) { + errors.push('cc:invalid:number'); } - if (!Stripe.validateCVC(cardData.cvc)) { - errors.push('cvc'); + if (!Stripe.validateCVC(card_data.cvc)) { + errors.push('cc:invalid:cvc'); } - if (!Stripe.validateExpiry(cardData.month, cardData.year)) { - errors.push('expiry'); + if (!Stripe.validateExpiry(card_data.month, card_data.year)) { + errors.push('cc:invalid:expiry'); } if (errors.length) { - JX.Workflow - .newFromForm(root, {cardErrors: JX.JSON.stringify(errors)}) - .start(); + ccform.submitForm(errors); return; } var data = { - number: cardData.number, - cvc: cardData.cvc, - exp_month: cardData.month, - exp_year: cardData.year + number: card_data.number, + cvc: card_data.cvc, + exp_month: card_data.month, + exp_year: card_data.year }; Stripe.createToken(data, onresponse); } - var onresponse = function(status, response) { + function onresponse(status, response) { var errors = []; var token = null; - if (response.error) { - errors = [response.error.type]; + if (status != 200) { + errors.push('cc:stripe:http:' + status); + } else if (response.error) { + errors.push('cc:stripe:error:' + response.error.type); } else { token = response.id; } - var params = { - cardErrors: JX.JSON.stringify(errors), - stripeToken: token - }; - - JX.Workflow - .newFromForm(root, params) - .start(); + ccform.submitForm(errors, {stripeCardToken: token}); } - JX.DOM.listen(root, 'submit', null, onsubmit); }); diff --git a/webroot/rsrc/js/application/phortune/phortune-credit-card-form.js b/webroot/rsrc/js/application/phortune/phortune-credit-card-form.js index fc6d063303..03298fec97 100644 --- a/webroot/rsrc/js/application/phortune/phortune-credit-card-form.js +++ b/webroot/rsrc/js/application/phortune/phortune-credit-card-form.js @@ -2,6 +2,10 @@ * @provides phortune-credit-card-form * @requires javelin-install * javelin-dom + * javelin-json + * javelin-workflow + * javelin-util + * @javelin */ /** @@ -9,21 +13,21 @@ * * To construct an object for a form: * - * new JX.PhortuneCreditCardForm(form_root_node); + * new JX.PhortuneCreditCardForm(form_root_node, submit_callback); * - * To read card data from a form: - * - * var data = ccform.getCardData(); */ JX.install('PhortuneCreditCardForm', { - construct : function(root) { + construct : function(root, onsubmit) { this._root = root; + this._submitCallback = onsubmit; + JX.DOM.listen(root, 'submit', null, JX.bind(this, this._onsubmit)); }, members : { _root : null, + _submitCallback : null, - getCardData : function() { + _getCardData : function() { var root = this._root; return { @@ -32,7 +36,24 @@ JX.install('PhortuneCreditCardForm', { month : JX.DOM.find(root, 'select', 'month-input' ).value, year : JX.DOM.find(root, 'select', 'year-input' ).value }; + }, + + submitForm : function(errors, token) { + var params = { + errors: JX.JSON.stringify(errors), + token: JX.JSON.stringify(token || {}) + }; + + JX.Workflow + .newFromForm(this._root, params) + .start(); + }, + + _onsubmit : function(e) { + e.kill(); + this._submitCallback(this._getCardData()); } + } });