Awight has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/341748 )
Change subject: [WIP] Do not pass go. This has gone too far, constructor order
is fragile.
......................................................................
[WIP] Do not pass go. This has gone too far, constructor order is fragile.
Change-Id: I7bf79b71f3c7410c61ccd24889996315e1097815
---
M gateway_common/gateway.adapter.php
M tests/phpunit/GatewayPageTest.php
2 files changed, 24 insertions(+), 20 deletions(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DonationInterface
refs/changes/48/341748/1
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index 69b0f05..65619bc 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -245,18 +245,6 @@
$this->defineDataTransformers();
- $this->session_resetOnSwitch(); // Need to do this before
creating DonationData
-
- // FIXME: this should not have side effects like setting
order_id_meta['final']
- // TODO: On second thought, neither set data nor validate in
this constructor.
- $this->dataObj = new DonationData( $this,
$options['external_data'] );
-
- $this->unstaged_data = $this->dataObj->getData();
- $this->staged_data = $this->unstaged_data;
-
- // checking to see if we have an edit token in the request...
- $this->posted = ( $this->dataObj->wasPosted() && ( !is_null(
WmfFramework::getRequestValue( 'wmf_token', null ) ) ) );
-
$this->findAccount();
$this->defineAccountInfo();
$this->defineTransactions();
@@ -266,9 +254,23 @@
$this->setGatewayDefaults( $options );
- // FIXME: Same as above, don't validate or stage in the
constructor.
- $this->errors = $this->validate();
+ $this->session_resetOnSwitch(); // Need to do this before
creating DonationData
+ // FIXME: this should not have side effects like setting
order_id_meta['final']
+ // TODO: On second thought, neither set data nor validate in
this constructor.
+ $this->dataObj = new DonationData( $this,
$options['external_data'] );
+
+ // FIXME: Same as above, don't validate or stage in the
constructor.
+ // Sets $this->errors if validation fails.
+ $this->validate();
+
+ $this->unstaged_data = $this->dataObj->getData();
+ $this->staged_data = $this->unstaged_data;
+
+ // checking to see if we have an edit token in the request...
+ $this->posted = ( $this->dataObj->wasPosted() && ( !is_null(
WmfFramework::getRequestValue( 'wmf_token', null ) ) ) );
+
+ // FIXME: Don't stage if invalid.
$this->stageData();
BannerHistoryLogIdProcessor::onGatewayReady( $this );
@@ -532,6 +534,8 @@
return $staged_data;
}
+ // FIXME: Rename and drop "escaped" at the least. Better yet, only use
+ // normalized data. See T134548.
public function getData_Unstaged_Escaped( $val = '' ) {
if ( $val === '' ) {
return $this->unstaged_data;
@@ -1228,7 +1232,7 @@
*/
public function getPaymentMethod() {
// FIXME: this should return the final calculated method
- return $this->getData_Unstaged_Escaped( 'payment_method' );
+ return $this->dataObj->getVal( 'payment_method' );
}
/**
@@ -1243,7 +1247,7 @@
}
public function getPaymentSubmethod() {
- return $this->getData_Unstaged_Escaped( 'payment_submethod' );
+ return $this->dataObj->getVal( 'payment_submethod' );
}
public function getPaymentSubmethods() {
@@ -2375,7 +2379,7 @@
// Add any country-specific required fields
if ( isset( $this->config['country_fields'] ) ) {
- $country = $this->getData_Unstaged_Escaped( 'country' );
+ $country = $this->dataObj->getVal( 'country' );
if ( $country && isset(
$this->config['country_fields'][$country] ) ) {
$validation =
$this->config['country_fields'][$country];
}
@@ -2413,7 +2417,7 @@
//however, that's not happening
in this class in the code I'm replacing, so...
//TODO: Something clever in the
DataValidator with data groups like these.
);
- $country =
$this->getData_Unstaged_Escaped( 'country' );
+ $country = $this->dataObj->getVal(
'country' );
if ( $country &&
Subdivisions::getByCountry( $country ) ) {
$check_not_empty[] = 'state';
}
@@ -2450,7 +2454,7 @@
* If it is not, it will return an array of errors ready for any
* DonationInterface form class derivitive to display.
*
- * @return boolean true if validation passes
+ * @return boolean True if validation passes
*/
public function validate() {
$normalized = $this->dataObj->getData();
diff --git a/tests/phpunit/GatewayPageTest.php
b/tests/phpunit/GatewayPageTest.php
index ef7b0c4..f635a87 100644
--- a/tests/phpunit/GatewayPageTest.php
+++ b/tests/phpunit/GatewayPageTest.php
@@ -77,7 +77,7 @@
TestingGenericAdapter::$fakeGlobals['NotifyOnConvert'] = true;
$this->setUpAdapter();
- $this->assertTrue( $this->adapter->validatedOK() );
+ $this->assertFalse( $this->adapter->validatedOK() );
$errors = $this->adapter->getErrors();
$msg = $this->page->msg(
'donate_interface-fallback-currency-notice', 'USD' )->text();
--
To view, visit https://gerrit.wikimedia.org/r/341748
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7bf79b71f3c7410c61ccd24889996315e1097815
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
