[MediaWiki-commits] [Gerrit] mediawiki...DonationInterface[master]: Comments and todos
jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/342547 )
Change subject: Comments and todos
..
Comments and todos
Change-Id: I16be18f5618cb34b341b6c3b76c730b928f727c9
---
M gateway_common/GatewayPage.php
M gateway_common/donation.api.php
M gateway_common/gateway.adapter.php
3 files changed, 11 insertions(+), 1 deletion(-)
Approvals:
jenkins-bot: Verified
Ejegg: Looks good to me, approved
diff --git a/gateway_common/GatewayPage.php b/gateway_common/GatewayPage.php
index a220050..0e88c00 100644
--- a/gateway_common/GatewayPage.php
+++ b/gateway_common/GatewayPage.php
@@ -122,6 +122,7 @@
return;
}
+ // FIXME: Should have checked this before creating the adapter.
if ( $this->adapter->getGlobal( 'Enabled' ) !== true ) {
$this->logger->info( 'Displaying fail page for disabled
gateway' );
$this->displayFailPage();
@@ -341,7 +342,8 @@
protected function handleDonationRequest() {
$this->setHeaders();
- // TODO: this is where we should feed GPCS parameters into
DonationData.
+ // TODO: This is where we should feed GPCS parameters into the
gateway
+ // and DonationData, rather than harvest params in the adapter
itself.
// dispatch forms/handling
if ( $this->adapter->checkTokens() ) {
diff --git a/gateway_common/donation.api.php b/gateway_common/donation.api.php
index d560699..8b2e1fa 100644
--- a/gateway_common/donation.api.php
+++ b/gateway_common/donation.api.php
@@ -30,6 +30,7 @@
$gatewayObj->revalidate();
if ( $gatewayObj->getAllErrors() ) {
$outputResult['errors'] = $gatewayObj->getAllErrors();
+ // FIXME: What is this junk? Smaller API, like
getResult()->addErrors
$this->getResult()->setIndexedTagName(
$outputResult['errors'], 'error' );
$this->getResult()->addValue( null, 'result',
$outputResult );
return;
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index 84b980d..c65e6f3 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -237,6 +237,7 @@
$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->setValidationErrors(
$this->getOriginalValidationErrors() );
@@ -348,6 +349,8 @@
}
foreach ( $this->config['transformers'] as $className ) {
+ // TODO: Pass $this to the constructor so we can take
gateway out
+ // of the interfaces.
$this->data_transformers[] = new $className();
}
}
@@ -896,10 +899,12 @@
$this->session_addDonorData();
if ( !$this->validatedOK() ){
//If the data didn't validate okay, prevent all data
transmissions.
+ // TODO: Rename variable to "response".
$return = new PaymentTransactionResponse();
$return->setCommunicationStatus( false );
$return->setMessage( 'Failed data validation' );
foreach ( $this->getAllErrors() as $code => $error ) {
+ // TODO: Error should already be in a native
format.
$return->addError( $code, array( 'message' =>
$error, 'logLevel' => LogLevel::INFO, 'debugInfo' => '' ) );
}
// TODO: should we set $this->transaction_response ?
@@ -1470,6 +1475,8 @@
/**
* Parse the response to get the errors in a format we can log and
otherwise deal with.
* @return array a key/value array of codes (if they exist) and
messages.
+* TODO: Move to a parsing class, where these are part of an interface
+* rather than empty although non-abstract.
*/
protected function parseResponseErrors( $response ) {
return array();
--
To view, visit https://gerrit.wikimedia.org/r/342547
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I16be18f5618cb34b341b6c3b76c730b928f727c9
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Awight
Gerrit-Reviewer: AndyRussG
[MediaWiki-commits] [Gerrit] mediawiki...DonationInterface[master]: Comments and todos
Awight has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/342547 )
Change subject: Comments and todos
..
Comments and todos
Change-Id: I16be18f5618cb34b341b6c3b76c730b928f727c9
---
M gateway_common/GatewayPage.php
M gateway_common/donation.api.php
M gateway_common/gateway.adapter.php
3 files changed, 11 insertions(+), 1 deletion(-)
git pull
ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/DonationInterface
refs/changes/47/342547/1
diff --git a/gateway_common/GatewayPage.php b/gateway_common/GatewayPage.php
index 8d32960..104c519 100644
--- a/gateway_common/GatewayPage.php
+++ b/gateway_common/GatewayPage.php
@@ -122,6 +122,7 @@
return;
}
+ // FIXME: Should have checked this before creating the adapter.
if ( $this->adapter->getGlobal( 'Enabled' ) !== true ) {
$this->logger->info( 'Displaying fail page for disabled
gateway' );
$this->displayFailPage();
@@ -341,7 +342,8 @@
protected function handleDonationRequest() {
$this->setHeaders();
- // TODO: this is where we should feed GPCS parameters into
DonationData.
+ // TODO: This is where we should feed GPCS parameters into the
gateway
+ // and DonationData, rather than harvest params in the adapter
itself.
// dispatch forms/handling
if ( $this->adapter->checkTokens() ) {
diff --git a/gateway_common/donation.api.php b/gateway_common/donation.api.php
index d560699..8b2e1fa 100644
--- a/gateway_common/donation.api.php
+++ b/gateway_common/donation.api.php
@@ -30,6 +30,7 @@
$gatewayObj->revalidate();
if ( $gatewayObj->getAllErrors() ) {
$outputResult['errors'] = $gatewayObj->getAllErrors();
+ // FIXME: What is this junk? Smaller API, like
getResult()->addErrors
$this->getResult()->setIndexedTagName(
$outputResult['errors'], 'error' );
$this->getResult()->addValue( null, 'result',
$outputResult );
return;
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index 8d7ed15..24bd3b0 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -237,6 +237,7 @@
$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->setValidationErrors(
$this->getOriginalValidationErrors() );
@@ -348,6 +349,8 @@
}
foreach ( $this->config['transformers'] as $className ) {
+ // TODO: Pass $this to the constructor so we can take
gateway out
+ // of the interfaces.
$this->data_transformers[] = new $className();
}
}
@@ -896,10 +899,12 @@
$this->session_addDonorData();
if ( !$this->validatedOK() ){
//If the data didn't validate okay, prevent all data
transmissions.
+ // TODO: Rename variable to "response".
$return = new PaymentTransactionResponse();
$return->setCommunicationStatus( false );
$return->setMessage( 'Failed data validation' );
foreach ( $this->getAllErrors() as $code => $error ) {
+ // TODO: Error should already be in a native
format.
$return->addError( $code, array( 'message' =>
$error, 'logLevel' => LogLevel::INFO, 'debugInfo' => '' ) );
}
// TODO: should we set $this->transaction_response ?
@@ -1470,6 +1475,8 @@
/**
* Parse the response to get the errors in a format we can log and
otherwise deal with.
* @return array a key/value array of codes (if they exist) and
messages.
+* TODO: Move to a parsing class, where these are part of an interface
+* rather than empty although non-abstract.
*/
protected function parseResponseErrors( $response ) {
return array();
--
To view, visit https://gerrit.wikimedia.org/r/342547
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I16be18f5618cb34b341b6c3b76c730b928f727c9
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Awight
