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 <awi...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to