Awight has uploaded a new change for review.
https://gerrit.wikimedia.org/r/310495
Change subject: [WIP] Encapsulate country validation in a helper class
......................................................................
[WIP] Encapsulate country validation in a helper class
Cleanup only, shouldn't affect operation.
Bug: T145608
Change-Id: Iee9abc5cf2e8d6f4a86d33cd427f6ac918585979
---
M adyen_gateway/config/transformers.yaml
M amazon_gateway/config/transformers.yaml
M astropay_gateway/config/transformers.yaml
M extension.json
A gateway_common/Country.php
M gateway_common/DataValidator.php
M gateway_common/DonationData.php
M gateway_common/ValidationHelper.php
M gateway_common/gateway.adapter.php
M globalcollect_gateway/config/transformers.yaml
M paypal_gateway/express_checkout/config/transformers.yaml
M paypal_gateway/legacy/config/transformers.yaml
12 files changed, 93 insertions(+), 60 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DonationInterface
refs/changes/95/310495/1
diff --git a/adyen_gateway/config/transformers.yaml
b/adyen_gateway/config/transformers.yaml
index 1c7f45f..e70d506 100644
--- a/adyen_gateway/config/transformers.yaml
+++ b/adyen_gateway/config/transformers.yaml
@@ -1,5 +1,6 @@
# Core
- Amount
+- Country
- DonorEmail
- StreetAddress
# Adyen-specific
diff --git a/amazon_gateway/config/transformers.yaml
b/amazon_gateway/config/transformers.yaml
index 260f807..015bfaa 100644
--- a/amazon_gateway/config/transformers.yaml
+++ b/amazon_gateway/config/transformers.yaml
@@ -1,5 +1,6 @@
# Skip AmountInCents, we want to pass the real amount x1.
- Amount
+- Country
- DonorEmail
- DonorFullName
- StreetAddress
diff --git a/astropay_gateway/config/transformers.yaml
b/astropay_gateway/config/transformers.yaml
index acac9ca..45f225e 100644
--- a/astropay_gateway/config/transformers.yaml
+++ b/astropay_gateway/config/transformers.yaml
@@ -2,6 +2,7 @@
- Amount
- AstroPayFinancialNumbers
- AstroPayMethodCodec
+- Country
- FiscalNumber
- DonorEmail
- DummyFiscalNumber # see class comment
diff --git a/extension.json b/extension.json
index 9ee40cd..bb9f692 100644
--- a/extension.json
+++ b/extension.json
@@ -68,6 +68,7 @@
"FiscalNumber": "gateway_common/FiscalNumber.php",
"ClientSideValidationHelper":
"gateway_common/ClientSideValidationHelper.php",
"ContributionTrackingPlusUnique":
"gateway_common/ContributionTrackingPlusUnique.php",
+ "Country": "gateway_common/Country.php",
"CurrencyRates": "gateway_common/CurrencyRates.php",
"CurrencyRatesModule": "modules/CurrencyRatesModule.php",
"CyclicalArray": "globalcollect_gateway/CyclicalArray.php",
diff --git a/gateway_common/Country.php b/gateway_common/Country.php
new file mode 100644
index 0000000..f10d955
--- /dev/null
+++ b/gateway_common/Country.php
@@ -0,0 +1,73 @@
+<?php
+
+/**
+ * Validation of the donor's country
+ */
+class Country implements ValidationHelper {
+
+ /**
+ * List of valid iso 3166 country codes
+ * This list was generated by
https://phabricator.wikimedia.org/diffusion/WFTO/browse/master/ISO%203166-1%20Country/
+ * as of 2013-10-03.
+ * TODO: Move into its own file. Alphabetize :p
+ */
+ protected static $iso_3166_codes = array(
+ 'AF', 'AX', 'AL', 'DZ', 'AS', 'AD', 'AO', 'AI', 'AQ', 'AG',
'AR', 'AM', 'AW', 'AU',
+ 'AT', 'AZ', 'BS', 'BH', 'BD', 'BB', 'BY', 'BE', 'BZ', 'BJ',
'BM', 'BT', 'BO', 'BQ',
+ 'BA', 'BW', 'BV', 'BR', 'IO', 'BN', 'BG', 'BF', 'BI', 'KH',
'CM', 'CA', 'CV', 'KY',
+ 'CF', 'TD', 'CL', 'CN', 'CX', 'CC', 'CO', 'KM', 'CG', 'CD',
'CK', 'CR', 'CI', 'HR',
+ 'CU', 'CW', 'CY', 'CZ', 'DK', 'DJ', 'DM', 'DO', 'EC', 'EG',
'SV', 'GQ', 'ER', 'EE',
+ 'ET', 'FK', 'FO', 'FJ', 'FI', 'FR', 'GF', 'PF', 'TF', 'GA',
'GM', 'GE', 'DE', 'GH',
+ 'GI', 'GR', 'GL', 'GD', 'GP', 'GU', 'GT', 'GG', 'GN', 'GW',
'GY', 'HT', 'HM', 'VA',
+ 'HN', 'HK', 'HU', 'IS', 'IN', 'ID', 'IR', 'IQ', 'IE', 'IM',
'IL', 'IT', 'JM', 'JP',
+ 'JE', 'JO', 'KZ', 'KE', 'KI', 'KP', 'KR', 'KW', 'KG', 'LA',
'LV', 'LB', 'LS', 'LR',
+ 'LY', 'LI', 'LT', 'LU', 'MO', 'MK', 'MG', 'MW', 'MY', 'MV',
'ML', 'MT', 'MH', 'MQ',
+ 'MR', 'MU', 'YT', 'MX', 'FM', 'MD', 'MC', 'MN', 'ME', 'MS',
'MA', 'MZ', 'MM', 'NA',
+ 'NR', 'NP', 'NL', 'NC', 'NZ', 'NI', 'NE', 'NG', 'NU', 'NF',
'MP', 'NO', 'OM', 'PK',
+ 'PW', 'PS', 'PA', 'PG', 'PY', 'PE', 'PH', 'PN', 'PL', 'PT',
'PR', 'QA', 'RE', 'RO',
+ 'RU', 'RW', 'BL', 'SH', 'KN', 'LC', 'MF', 'PM', 'VC', 'WS',
'SM', 'ST', 'SA', 'SN',
+ 'RS', 'SC', 'SL', 'SG', 'SX', 'SK', 'SI', 'SB', 'SO', 'ZA',
'GS', 'SS', 'ES', 'LK',
+ 'SD', 'SR', 'SJ', 'SZ', 'SE', 'CH', 'SY', 'TW', 'TJ', 'TZ',
'TH', 'TL', 'TG', 'TK',
+ 'TO', 'TT', 'TN', 'TR', 'TM', 'TC', 'TV', 'UG', 'UA', 'AE',
'GB', 'US', 'UM', 'UY',
+ 'UZ', 'VU', 'VE', 'VN', 'VG', 'VI', 'WF', 'EH', 'YE', 'ZM',
'ZW',
+ );
+
+ /**
+ * Check the country to see if it's a real ISO code, and against
configured
+ * countries where we're not allowed to fundraise.
+ *
+ * @param GatewayType $adapter
+ * @param array $normalized Normalized donation data
+ * @param array $errors Results of validation to this point, to be
updated
+ * with any error found by this function.
+ *
+ * TODO: T121728
+ */
+ public function validate( GatewayType $adapter, $normalized, &$errors )
{
+ global $wgDonationInterfaceForbiddenCountries;
+
+ // Country cannot be empty.
+ if ( empty( $normalized['country'] ) ) {
+ // FIXME: Function contract should be
$errors['country'] = 'not_empty';
+ $errors['country'] = DataValidator::getErrorMessage(
'country', 'not_empty', $normalized['language'], null );
+ return;
+ }
+ $country = $normalized['country'];
+
+ // Check that it's a valid ISO 3166 code.
+ if ( !self::is_valid_iso_country_code( $country ) ) {
+ $errors['country'] = DataValidator::getErrorMessage(
'country', 'calculated', $normalized['language'], $country );
+ }
+
+ // Check that we're allowed to fundraise here.
+ if ( in_array( $country, $wgDonationInterfaceForbiddenCountries
) ) {
+ $errors['country'] = DataValidator::getErrorMessage(
'country', 'calculated', $normalized['language'], $country );
+ }
+ }
+
+ // FIXME: This should be protected and calling code split into more
+ // disjoint responsibilities.
+ public static function is_valid_iso_country_code( $country ) {
+ return in_array( $country, self::$iso_3166_codes );
+ }
+}
diff --git a/gateway_common/DataValidator.php b/gateway_common/DataValidator.php
index 51c3f79..fd9bcbe 100644
--- a/gateway_common/DataValidator.php
+++ b/gateway_common/DataValidator.php
@@ -193,7 +193,6 @@
// Define all default validations.
$validations = array(
'not_empty' => array(
- 'country',
'currency_code',
'gateway',
),
@@ -214,7 +213,6 @@
'gateway' => 'validate_gateway',
'address' => 'validate_address',
'city' => 'validate_address',
- 'country' => 'validate_country_allowed',
'email' => 'validate_email',
'street' => 'validate_address',
'currency_code' => 'validate_currency_code',
@@ -475,22 +473,6 @@
}
/**
- * Validate that the country is legally allowed to give us a donation.
- * Failure here should halt everything, all the time.
- * @param string $value The value to check
- * @return boolean true if we are allowed to accept donations from this
- * country, false if not.
- */
- public static function validate_country_allowed( $value ){
- global $wgDonationInterfaceForbiddenCountries;
- if ( in_array( strtoupper($value),
$wgDonationInterfaceForbiddenCountries ) ){
- return false;
- }
- // TODO: return DataValidator::is_valid_iso_country_code(
$value );
- return true;
- }
-
- /**
* Some people are silly and enter their CC numbers as their name. This
performs a luhn check
* on the name to make sure it's not actually a potentially valid CC
number.
*
@@ -730,45 +712,6 @@
} else {
return false;
}
- }
-
- /**
- * Checks to see if $country is a valid iso 3166-1 country code.
- * DOES NOT VERIFY THAT WE FUNDRAISE THERE. Only that the code makes
sense.
- * @param string $country the code we want to check
- * @return boolean
- */
- public static function is_valid_iso_country_code( $country ) {
- /**
- * List of valid iso 3166 country codes, regenerated on
1380836686
- * Code generated by a happy script at
- *
https://gerrit.wikimedia.org/r/#/admin/projects/wikimedia/fundraising/tools,branches
- */
- $iso_3166_codes = array (
- 'AF', 'AX', 'AL', 'DZ', 'AS', 'AD', 'AO', 'AI', 'AQ',
'AG', 'AR', 'AM', 'AW', 'AU',
- 'AT', 'AZ', 'BS', 'BH', 'BD', 'BB', 'BY', 'BE', 'BZ',
'BJ', 'BM', 'BT', 'BO', 'BQ',
- 'BA', 'BW', 'BV', 'BR', 'IO', 'BN', 'BG', 'BF', 'BI',
'KH', 'CM', 'CA', 'CV', 'KY',
- 'CF', 'TD', 'CL', 'CN', 'CX', 'CC', 'CO', 'KM', 'CG',
'CD', 'CK', 'CR', 'CI', 'HR',
- 'CU', 'CW', 'CY', 'CZ', 'DK', 'DJ', 'DM', 'DO', 'EC',
'EG', 'SV', 'GQ', 'ER', 'EE',
- 'ET', 'FK', 'FO', 'FJ', 'FI', 'FR', 'GF', 'PF', 'TF',
'GA', 'GM', 'GE', 'DE', 'GH',
- 'GI', 'GR', 'GL', 'GD', 'GP', 'GU', 'GT', 'GG', 'GN',
'GW', 'GY', 'HT', 'HM', 'VA',
- 'HN', 'HK', 'HU', 'IS', 'IN', 'ID', 'IR', 'IQ', 'IE',
'IM', 'IL', 'IT', 'JM', 'JP',
- 'JE', 'JO', 'KZ', 'KE', 'KI', 'KP', 'KR', 'KW', 'KG',
'LA', 'LV', 'LB', 'LS', 'LR',
- 'LY', 'LI', 'LT', 'LU', 'MO', 'MK', 'MG', 'MW', 'MY',
'MV', 'ML', 'MT', 'MH', 'MQ',
- 'MR', 'MU', 'YT', 'MX', 'FM', 'MD', 'MC', 'MN', 'ME',
'MS', 'MA', 'MZ', 'MM', 'NA',
- 'NR', 'NP', 'NL', 'NC', 'NZ', 'NI', 'NE', 'NG', 'NU',
'NF', 'MP', 'NO', 'OM', 'PK',
- 'PW', 'PS', 'PA', 'PG', 'PY', 'PE', 'PH', 'PN', 'PL',
'PT', 'PR', 'QA', 'RE', 'RO',
- 'RU', 'RW', 'BL', 'SH', 'KN', 'LC', 'MF', 'PM', 'VC',
'WS', 'SM', 'ST', 'SA', 'SN',
- 'RS', 'SC', 'SL', 'SG', 'SX', 'SK', 'SI', 'SB', 'SO',
'ZA', 'GS', 'SS', 'ES', 'LK',
- 'SD', 'SR', 'SJ', 'SZ', 'SE', 'CH', 'SY', 'TW', 'TJ',
'TZ', 'TH', 'TL', 'TG', 'TK',
- 'TO', 'TT', 'TN', 'TR', 'TM', 'TC', 'TV', 'UG', 'UA',
'AE', 'GB', 'US', 'UM', 'UY',
- 'UZ', 'VU', 'VE', 'VN', 'VG', 'VI', 'WF', 'EH', 'YE',
'ZM', 'ZW',
- );
-
- if ( in_array( $country, $iso_3166_codes ) ) {
- return true;
- }
- return false;
}
/**
diff --git a/gateway_common/DonationData.php b/gateway_common/DonationData.php
index 5363e6c..f8b8700 100644
--- a/gateway_common/DonationData.php
+++ b/gateway_common/DonationData.php
@@ -142,6 +142,7 @@
//if we have saved any donation data to the session, pull them
in as well.
$this->integrateDataFromSession();
+ // TODO: Can we avoid populating "normalized" with
non-normalized data?
if ( $this->normalized ) {
$this->normalize();
@@ -315,9 +316,12 @@
* such a way that running them multiple times on the same array won't
cause
* the data to stroll off into the sunset: Normalize will definitely
need to
* be called multiple times against the same array.
+ *
+ * TODO: Introduce NormalizationHelpers to encapsulate this mess?
*/
protected function normalize() {
// FIXME: there's a ghost invocation during DonationData
construction.
+
// This condition should actually be "did data come from
anywhere?"
if ( !empty( $this->normalized ) ) {
$updateCtRequired =
$this->handleContributionTrackingID(); // Before Order ID
@@ -341,6 +345,9 @@
$this->saveContributionTrackingData();
}
+ // FIXME: We're actually setting the validation errors
here, in
+ // order to do fallback currency if necessary. This
should be
+ // handled by calling code.
$this->getValidationErrors();
if ( isset( $this->validationErrors['currency_code'] )
) {
@@ -395,7 +402,7 @@
if ( $this->isSomething( 'country' ) ) {
$country = strtoupper( $this->getVal( 'country' ) );
- if ( DataValidator::is_valid_iso_country_code( $country
) ) {
+ if ( Country::is_valid_iso_country_code( $country ) ) {
$regen = false;
} else {
//check to see if it's one of those other codes
that comes out of CN, for the logs
@@ -431,7 +438,7 @@
}
//still nothing good? Give up.
- if ( !DataValidator::is_valid_iso_country_code(
$country ) ) {
+ if ( !Country::is_valid_iso_country_code( $country ) ) {
$country = 'XX';
}
}
diff --git a/gateway_common/ValidationHelper.php
b/gateway_common/ValidationHelper.php
index e2c0c50..b465aec 100644
--- a/gateway_common/ValidationHelper.php
+++ b/gateway_common/ValidationHelper.php
@@ -5,6 +5,9 @@
* Run validation on whatever normalized data we're responsible for,
* and set errors per field, or under the "general" key.
*
+ * FIXME: Error values are localized messages, but that's too hard.
Return
+ * enums or i18n message keys instead, which can be translated later.
+ *
* @param GatewayType $adapter
* @param array $normalized Donation data in normalized form.
* @param array $errors Reference to error array
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index 227962f..a6ab6c4 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -449,11 +449,11 @@
public function getCoreDataTransformers() {
return array(
- // Always stage email address first, to set default if
missing
new DonorEmail(),
new DonorFullName(),
new Amount(),
new AmountInCents(),
+ new Country(),
new StreetAddress(),
);
}
diff --git a/globalcollect_gateway/config/transformers.yaml
b/globalcollect_gateway/config/transformers.yaml
index 082adfe..58921d2 100644
--- a/globalcollect_gateway/config/transformers.yaml
+++ b/globalcollect_gateway/config/transformers.yaml
@@ -1,5 +1,6 @@
# Core
- Amount
+- Country
- DonorEmail
- DonorFullName
- AmountInCents
diff --git a/paypal_gateway/express_checkout/config/transformers.yaml
b/paypal_gateway/express_checkout/config/transformers.yaml
index e55037c..82461b4 100644
--- a/paypal_gateway/express_checkout/config/transformers.yaml
+++ b/paypal_gateway/express_checkout/config/transformers.yaml
@@ -1,3 +1,4 @@
+- Country
- IsoDate
- DonorLocale
- PaypalExpressReturnUrl
diff --git a/paypal_gateway/legacy/config/transformers.yaml
b/paypal_gateway/legacy/config/transformers.yaml
index 3f63205..eb3932a 100644
--- a/paypal_gateway/legacy/config/transformers.yaml
+++ b/paypal_gateway/legacy/config/transformers.yaml
@@ -1,5 +1,6 @@
# Core
- Amount
+- Country
- DonorEmail
# PayPal
--
To view, visit https://gerrit.wikimedia.org/r/310495
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iee9abc5cf2e8d6f4a86d33cd427f6ac918585979
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Awight <awi...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
