Ejegg has uploaded a new change for review.
https://gerrit.wikimedia.org/r/286261
Change subject: WIP encapsulated amount validation for better messages
......................................................................
WIP encapsulated amount validation for better messages
TODO: clean up some special cases in DataValidator
'Amount' desperately needs a namespace
Bug: T105618
Change-Id: If9d60491cdc992a38a0ea459f5ece3f38ce08184
---
M DonationInterface.php
M adyen_gateway/config/transformers.yaml
M amazon_gateway/config/transformers.yaml
M astropay_gateway/config/transformers.yaml
A gateway_common/Amount.php
M gateway_common/DataValidator.php
M gateway_common/DonationData.php
M gateway_common/FiscalNumber.php
M gateway_common/ValidationHelper.php
M globalcollect_gateway/config/transformers.yaml
M paypal_gateway/config/transformers.yaml
M tests/DataValidatorTest.php
M worldpay_gateway/config/transformers.yaml
13 files changed, 133 insertions(+), 99 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DonationInterface
refs/changes/61/286261/1
diff --git a/DonationInterface.php b/DonationInterface.php
index ad0a0dd..7d16eec 100644
--- a/DonationInterface.php
+++ b/DonationInterface.php
@@ -37,6 +37,7 @@
/**
* CLASSES
*/
+$wgAutoloadClasses['Amount'] = __DIR__ . '/gateway_common/Amount.php';
$wgAutoloadClasses['AmountInCents'] = __DIR__ .
'/gateway_common/AmountInCents.php';
$wgAutoloadClasses['ArrayHelper'] = __DIR__ .
'/gateway_common/ArrayHelper.php';
$wgAutoloadClasses['FiscalNumber'] = __DIR__ .
'/gateway_common/FiscalNumber.php';
diff --git a/adyen_gateway/config/transformers.yaml
b/adyen_gateway/config/transformers.yaml
index 7150041..5a85ddd 100644
--- a/adyen_gateway/config/transformers.yaml
+++ b/adyen_gateway/config/transformers.yaml
@@ -1,4 +1,5 @@
# Core
+- Amount
- DonorEmail
- DonorFullName
- AmountInCents
diff --git a/amazon_gateway/config/transformers.yaml
b/amazon_gateway/config/transformers.yaml
index 89e70d8..260f807 100644
--- a/amazon_gateway/config/transformers.yaml
+++ b/amazon_gateway/config/transformers.yaml
@@ -1,4 +1,5 @@
# Skip AmountInCents, we want to pass the real amount x1.
+- Amount
- DonorEmail
- DonorFullName
- StreetAddress
diff --git a/astropay_gateway/config/transformers.yaml
b/astropay_gateway/config/transformers.yaml
index caaa153..bae2a9c 100644
--- a/astropay_gateway/config/transformers.yaml
+++ b/astropay_gateway/config/transformers.yaml
@@ -1,7 +1,9 @@
# Skip AmountInCents.
+- Amount
- AstroPayFinancialNumbers
- AstroPayMethodCodec
- FiscalNumber
+- DonorEmail
- DummyFiscalNumber # see class comment
- DonorFullName
- StreetAddress
diff --git a/gateway_common/Amount.php b/gateway_common/Amount.php
new file mode 100644
index 0000000..1026d03
--- /dev/null
+++ b/gateway_common/Amount.php
@@ -0,0 +1,106 @@
+<?php
+
+class Amount implements ValidationHelper {
+
+ function validate( GatewayType $gateway, $normalized, &$errors ) {
+ if (
+ !isset( $normalized['amount'] ) ||
+ !isset( $normalized['currency_code'] )
+ ) {
+ // Not enough info to validate
+ return;
+ }
+ if ( isset( $errors['currency_code'] ) ) {
+ // Already displaying an error
+ return;
+ }
+ $value = $normalized['amount'];
+
+ if ( self::isZeroIsh( $value ) ) {
+ $errors['amount'] = DataValidator::getErrorMessage(
+ 'amount',
+ 'not_empty',
+ $normalized['language']
+ );
+ return;
+ }
+ $currency = $normalized['currency_code'];
+ $min = self::convert( $gateway->getGlobal( 'PriceFloor' ),
$currency );
+ $max = self::convert( $gateway->getGlobal( 'PriceCeiling' ),
$currency );
+ if (
+ !is_numeric( $value ) ||
+ $value > $max
+ ) {
+ $errors['amount'] = WmfFramework::formatMessage(
+ 'donate_interface-error-msg-invalid-amount'
+ );
+ } else if ( $value < $min ) {
+ $errors['amount'] = WmfFramework::formatMessage(
+ 'donate_interface-smallamount-error',
+ self::round( $min, $currency ) . ' ' . $currency
+ );
+ }
+ }
+
+ /**
+ * Checks if the $value is missing or equivalent to zero.
+ *
+ * @param string $value The value to check for zero-ness
+ * @return boolean True if the $value is missing or zero, otherwise
false
+ */
+ protected static function isZeroIsh( $value ) {
+ if (
+ $value === null ||
+ $value === '' ||
+ preg_match( '/^0+(\.0+)?$/', $value )
+ ) {
+ return true;
+ }
+
+ return false;
+ }
+
+ /**
+ * Convert an amount in USD to a particular currency
+ *
+ * This is grossly rudimentary and likely wildly inaccurate.
+ * This mimics the hard-coded values used by the WMF to convert
currencies
+ * for validation on the front-end on the first step landing pages of
their
+ * donation process - the idea being that we can get a close
approximation
+ * of converted currencies to ensure that contributors are not going
above
+ * or below the price ceiling/floor, even if they are using a non-US
currency.
+ *
+ * In reality, this probably ought to use some sort of webservice to
get real-time
+ * conversion rates.
+ *
+ * @param float $amount
+ * @param string $currency
+ * @return float
+ * @throws UnexpectedValueException
+ */
+ public static function convert( $amount, $currency ) {
+ $rates = CurrencyRates::getCurrencyRates();
+ $code = strtoupper( $currency );
+ if ( array_key_exists( $code, $rates ) ) {
+ return $amount * $rates[$code];
+ }
+ throw new UnexpectedValueException(
+ 'Bad programmer! Bad currency made it too far through
the portcullis'
+ );
+ }
+
+ /**
+ * Some currencies, like JPY, don't exist in fractional amounts.
+ * This rounds an amount to the appropriate number of decimal places.
+ *
+ * @param float $amount
+ * @param string $currencyCode
+ * @return string rounded amount
+ */
+ public static function round( $amount, $currencyCode ) {
+ if ( DataValidator::is_fractional_currency( $currencyCode ) ){
+ return number_format( $amount, 2, '.', '' );
+ }
+ return (string) floor( $amount );
+ }
+}
diff --git a/gateway_common/DataValidator.php b/gateway_common/DataValidator.php
index 009b8dd..ba90f84 100644
--- a/gateway_common/DataValidator.php
+++ b/gateway_common/DataValidator.php
@@ -25,10 +25,6 @@
*/
public static function getErrorToken( $field ){
switch ( $field ) {
- case 'amountGiven' :
- case 'amountOther' :
- $error_token = 'amount';
- break;
case 'email' :
case 'amount' :
case 'currency_code' :
@@ -200,7 +196,6 @@
// Define all default validations.
$validations = array(
'not_empty' => array(
- 'amount' => 'validate_non_zero',
'country',
'currency_code',
'gateway',
@@ -208,9 +203,6 @@
'valid_type' => array(
'_cache_' => 'validate_boolean',
'account_number' => 'validate_numeric',
- 'amount' => 'validate_numeric',
- 'amountGiven' => 'validate_numeric',
- 'amountOther' => 'validate_numeric',
'anonymous' => 'validate_boolean',
'contribution_tracking_id' =>
'validate_numeric',
'currency_code' => 'validate_alphanumeric',
@@ -232,13 +224,8 @@
'fname' => 'validate_name',
'lname' => 'validate_name',
'name' => 'validate_name',
-
- // Depends on currency_code and gateway.
- 'amount' => 'validate_amount',
-
),
);
-
$errors = array();
$results = array();
@@ -285,13 +272,6 @@
$result = null;
// Handle special cases.
switch ( $validation_function ) {
- case 'validate_amount':
- if (
self::checkValidationPassed( array( 'currency_code', 'gateway' ), $results ) ){
- $priceFloor =
$gateway->getGlobal( 'PriceFloor' );
- $priceCeiling =
$gateway->getGlobal( 'PriceCeiling' );
- $result =
call_user_func( $callable, $data[$field], $data['currency_code'], $priceFloor,
$priceCeiling );
- } //otherwise, just don't do
the validation. The other stuff will be complaining already.
- break;
case 'validate_currency_code':
$result = call_user_func(
$callable, $data[$field], $gateway->getCurrencies( $data ) );
break;
@@ -347,31 +327,6 @@
protected static function validate_email( $value ) {
return WmfFramework::validateEmail( $value )
&& !DataValidator::cc_number_exists_in_str( $value );
- }
-
- /**
- * validate_amount
- *
- * Determines if the $value passed in is a valid amount.
- * @param string $value The piece of data that is supposed to be an
amount.
- * @param string $currency_code The amount was given in this currency.
- * @param float $priceFloor Minimum valid amount (USD).
- * @param float $priceCeiling Maximum valid amount (USD).
- *
- * @return boolean True if $value is a valid amount, otherwise false.
- */
- protected static function validate_amount( $value, $currency_code,
$priceFloor, $priceCeiling ) {
- if ( !$value || !$currency_code || !is_numeric( $value ) ) {
- return false;
- }
-
- if ( !preg_match( '/^\d+(\.(\d+)?)?$/', $value ) ||
- ( ( float ) self::convert_to_usd( $currency_code,
$value ) < ( float ) $priceFloor ||
- ( float ) self::convert_to_usd( $currency_code, $value
) > ( float ) $priceCeiling ) ) {
- return false;
- }
-
- return true;
}
protected static function validate_currency_code( $value,
$acceptedCurrencies ) {
@@ -557,22 +512,6 @@
}
/**
- * Checks to make sure that the $value is present in the $data array,
and not equivalent to zero.
- * @param string $value The value to check for non-zeroness.
- * @param array $data The whole data set.
- * @return boolean True if the $value is not missing or zero, otherwise
false.
- */
- public static function validate_non_zero( $value ) {
- if ( $value === null || $value === ''
- || preg_match( '/^0+(\.0+)?$/', $value )
- ) {
- return false;
- }
-
- return true;
- }
-
- /**
* Analyzes a string to see if any credit card numbers are hiding out
in it
*
* @param $str
@@ -660,35 +599,6 @@
$odd = !$odd;
}
return( ( $sum % 10 ) == 0 );
- }
-
- /**
- * Convert an amount for a particular currency to an amount in USD
- *
- * This is grosley rudimentary and likely wildly inaccurate.
- * This mimicks the hard-coded values used by the WMF to convert
currencies
- * for validatoin on the front-end on the first step landing pages of
their
- * donation process - the idea being that we can get a close
approximation
- * of converted currencies to ensure that contributors are not going
above
- * or below the price ceiling/floor, even if they are using a non-US
currency.
- *
- * In reality, this probably ought to use some sort of webservice to
get real-time
- * conversion rates.
- *
- * @param string $currency_code
- * @param float $amount
- * @return float
- * @throws UnexpectedValueException
- */
- public static function convert_to_usd( $currency_code, $amount ) {
- $rates = CurrencyRates::getCurrencyRates();
- $code = strtoupper( $currency_code );
- if ( array_key_exists( $code, $rates ) ) {
- $usd_amount = $amount / $rates[$code];
- } else {
- throw new UnexpectedValueException( 'Bad programmer!
Bad currency made it too far through the portcullis' );
- }
- return $usd_amount;
}
diff --git a/gateway_common/DonationData.php b/gateway_common/DonationData.php
index 9337673..3bc3345 100644
--- a/gateway_common/DonationData.php
+++ b/gateway_common/DonationData.php
@@ -545,12 +545,11 @@
$this->setVal('amount', 'invalid');
return;
}
-
- if ( DataValidator::is_fractional_currency( $this->getVal(
'currency_code' ) ) ){
- $this->setVal( 'amount', number_format( $this->getVal(
'amount' ), 2, '.', '' ) );
- } else {
- $this->setVal( 'amount', floor( $this->getVal( 'amount'
) ) );
- }
+
+ $this->setVal(
+ 'amount',
+ Amount::round( $this->getVal( 'amount' ),
$this->getVal( 'currency_code' ) )
+ );
}
/**
@@ -981,7 +980,11 @@
$transformers = $this->gateway->getDataTransformers();
foreach ( $transformers as $transformer ) {
if ( $transformer instanceof ValidationHelper )
{
- $transformer->validate(
$this->normalized, $this->validationErrors );
+ $transformer->validate(
+ $this->gateway,
+ $this->normalized,
+ $this->validationErrors
+ );
}
}
}
diff --git a/gateway_common/FiscalNumber.php b/gateway_common/FiscalNumber.php
index 5bb458b..eb4dff2 100644
--- a/gateway_common/FiscalNumber.php
+++ b/gateway_common/FiscalNumber.php
@@ -50,10 +50,11 @@
/**
* Rudimentary tests of fiscal number format for different countries
*
+ * @param GatewayType $unused
* @param array $normalized Normalized donation data
* @param array $errors Results of validation to this point
*/
- public function validate( $normalized, &$errors ) {
+ public function validate( GatewayType $unused, $normalized, &$errors ) {
if (
!isset( $normalized['country'] ) ||
!isset( $normalized['fiscal_number'] )
diff --git a/gateway_common/ValidationHelper.php
b/gateway_common/ValidationHelper.php
index 8926315..e2c0c50 100644
--- a/gateway_common/ValidationHelper.php
+++ b/gateway_common/ValidationHelper.php
@@ -4,6 +4,11 @@
/**
* Run validation on whatever normalized data we're responsible for,
* and set errors per field, or under the "general" key.
+ *
+ * @param GatewayType $adapter
+ * @param array $normalized Donation data in normalized form.
+ * @param array $errors Reference to error array
+ * @return void
*/
- function validate( $normalized, &$errors );
+ function validate( GatewayType $adapter, $normalized, &$errors );
}
diff --git a/globalcollect_gateway/config/transformers.yaml
b/globalcollect_gateway/config/transformers.yaml
index cea8f45..082adfe 100644
--- a/globalcollect_gateway/config/transformers.yaml
+++ b/globalcollect_gateway/config/transformers.yaml
@@ -1,4 +1,5 @@
# Core
+- Amount
- DonorEmail
- DonorFullName
- AmountInCents
diff --git a/paypal_gateway/config/transformers.yaml
b/paypal_gateway/config/transformers.yaml
index 5dbeeca..a69272f 100644
--- a/paypal_gateway/config/transformers.yaml
+++ b/paypal_gateway/config/transformers.yaml
@@ -1,4 +1,5 @@
# Core
+- Amount
- DonorEmail
- DonorFullName
- AmountInCents
diff --git a/tests/DataValidatorTest.php b/tests/DataValidatorTest.php
index 0a2ecb3..221a64c 100644
--- a/tests/DataValidatorTest.php
+++ b/tests/DataValidatorTest.php
@@ -137,6 +137,7 @@
$validator = new FiscalNumber();
$errors = array();
$validator->validate(
+ new TestingGenericAdapter(),
array( 'country' => $country, 'fiscal_number' =>
$value, 'language' => 'en' ),
$errors
);
diff --git a/worldpay_gateway/config/transformers.yaml
b/worldpay_gateway/config/transformers.yaml
index 3cc2749..6fe36d3 100644
--- a/worldpay_gateway/config/transformers.yaml
+++ b/worldpay_gateway/config/transformers.yaml
@@ -1,4 +1,5 @@
# Core
+- Amount
- DonorEmail
- DonorFullName
- AmountInCents
--
To view, visit https://gerrit.wikimedia.org/r/286261
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If9d60491cdc992a38a0ea459f5ece3f38ce08184
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
