jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/354605 )
Change subject: Consolidate queue message generation.
......................................................................
Consolidate queue message generation.
Pretty silly to have two functions that massage the same thing.
Changes the logging, but there's a CRM patch to adapt to that.
Also gets rid of the php-message-class on DonationInterfaceMessages -
we're not deserializing those messages with FromJsonProxy anywhere.
Bug: T95647
Change-Id: Icffc82d2a14c4e86a705b053a644ce4476f6e33f
---
M adyen_gateway/adyen.adapter.php
M amazon_gateway/amazon.adapter.php
M extras/banner_history/BannerHistoryLogIdProcessor.php
M gateway_common/DonationData.php
M gateway_common/DonationQueue.php
M gateway_common/gateway.adapter.php
M globalcollect_gateway/orphan.adapter.php
M tests/phpunit/Adapter/Adyen/AdyenTest.php
M tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
M tests/phpunit/DonationQueueTest.php
M tests/phpunit/GatewayPageTest.php
M tests/phpunit/LoggingTest.php
12 files changed, 67 insertions(+), 149 deletions(-)
Approvals:
Mepps: Looks good to me, approved
jenkins-bot: Verified
diff --git a/adyen_gateway/adyen.adapter.php b/adyen_gateway/adyen.adapter.php
index c86adb5..8129c3e 100644
--- a/adyen_gateway/adyen.adapter.php
+++ b/adyen_gateway/adyen.adapter.php
@@ -215,8 +215,8 @@
* whether to capture the payment or leave it for manual review.
* @return array
*/
- protected function getStompTransaction() {
- $transaction = parent::getStompTransaction();
+ protected function getQueueDonationMessage() {
+ $transaction = parent::getQueueDonationMessage();
$transaction['risk_score'] = $this->risk_score;
return $transaction;
}
diff --git a/amazon_gateway/amazon.adapter.php
b/amazon_gateway/amazon.adapter.php
index 8d4473a..08e499a 100644
--- a/amazon_gateway/amazon.adapter.php
+++ b/amazon_gateway/amazon.adapter.php
@@ -199,7 +199,7 @@
) );
// Stash their info in pending queue and logs to fill in data
for
// audit and IPN messages
- $details = $this->getStompTransaction();
+ $details = $this->getQueueDonationMessage();
$this->logger->info( 'Got info for Amazon donation: ' .
json_encode( $details ) );
$this->sendPendingMessage();
}
diff --git a/extras/banner_history/BannerHistoryLogIdProcessor.php
b/extras/banner_history/BannerHistoryLogIdProcessor.php
index ecc7206..64c95f1 100644
--- a/extras/banner_history/BannerHistoryLogIdProcessor.php
+++ b/extras/banner_history/BannerHistoryLogIdProcessor.php
@@ -67,7 +67,6 @@
}
$data = array(
- 'freeform' => true,
'banner_history_id' => $bannerHistoryId,
'contribution_tracking_id' => $contributionTrackingId,
);
diff --git a/gateway_common/DonationData.php b/gateway_common/DonationData.php
index 0b2943b..823481d 100644
--- a/gateway_common/DonationData.php
+++ b/gateway_common/DonationData.php
@@ -920,7 +920,6 @@
'contribution_tracking_id',
'optout',
'anonymous',
- 'size',
'utm_source',
'utm_medium',
'utm_campaign',
diff --git a/gateway_common/DonationQueue.php b/gateway_common/DonationQueue.php
index f9fa83c..6971174 100644
--- a/gateway_common/DonationQueue.php
+++ b/gateway_common/DonationQueue.php
@@ -34,12 +34,12 @@
return $sourceRevision;
}
- public function push( $transaction, $queue ) {
+ public function push( $message, $queue ) {
// TODO: This should be checked once, at a higher level.
if ( !GatewayAdapter::getGlobal( 'EnableQueue' ) ) {
return;
}
- $message = $this->buildBody( $transaction );
+ SourceFields::addToMessage( $message );
$this->newBackend( $queue )->push( $message );
}
@@ -59,25 +59,6 @@
$backend = $this->newBackend( $queue );
return $backend->peek();
- }
-
- /**
- * Build a body string, given a donation data array
- *
- * @param array $transaction
- *
- * @return array Message body. Note that we aren't json_encoding here,
cos
- * PHPQueue expects an array.
- */
- protected function buildBody( $transaction ) {
- if ( array_key_exists( 'freeform', $transaction ) &&
$transaction['freeform'] ) {
- $data = $transaction;
- } else {
- // Assume anything else is a regular donation.
- $data = $this->buildTransactionMessage( $transaction );
- }
- SourceFields::addToMessage( $data );
- return $data;
}
/**
@@ -119,81 +100,6 @@
throw new RuntimeException( "Queue backend class not
found: [$className]" );
}
return new $className( $serverConfig );
- }
-
- /**
- * Assign correct values to the array of data to be sent to the
ActiveMQ server
- * TODO: Probably something else. I don't like the way this works and
neither do you.
- *
- * Older notes follow:
- * Currency in receiving module has currency set to USD, should take
passed variable for these
- * PAssed both ISO and country code, no need to look up
- * 'gateway' = globalcollect, e.g.
- * 'date' is sent as $date("r")
- * so it can be translated with strtotime like Paypal transactions
(correct?)
- * Processor txn ID sent in the transaction response is assigned to
'gateway_txn_id' (PNREF)
- * Order ID (generated with transaction) is assigned to
'contribution_tracking_id'?
- * Response from processor is assigned to 'response'
- *
- * @param array $transaction values from gateway adapter
- * @return array values normalized to wire format
- */
- protected function buildTransactionMessage( $transaction ) {
- // specifically designed to match the CiviCRM API that will
handle it
- // edit this array to include/ignore transaction data sent to
the server
-
- $message = array(
- 'contribution_tracking_id' =>
$transaction['contribution_tracking_id'],
- 'country' => $transaction['country'],
- // the following int casting fixes an issue that is
more in Drupal/CiviCRM than here.
- // The code there should also be fixed.
- 'date' => (int)$transaction['date'],
- 'fee' => '0',
- 'gateway_account' => $transaction['gateway_account'],
- 'gateway' => $transaction['gateway'],
- 'gateway_txn_id' => $transaction['gateway_txn_id'],
- 'language' => $transaction['language'],
- 'order_id' => $transaction['order_id'],
- 'payment_method' => $transaction['payment_method'],
- 'payment_submethod' =>
$transaction['payment_submethod'],
- 'response' => $transaction['response'],
- 'user_ip' => $transaction['user_ip'],
- 'utm_source' => $transaction['utm_source'],
- );
-
- // We're using this mapping for optional fields, and to cheat
on not
- // transforming messages a if they are processed through this
function
- // multiple times.
- $optional_keys = array(
- 'anonymous' => 'anonymous',
- 'city' => 'city',
- 'currency' => 'currency',
- 'email' => 'email',
- 'first_name' => 'first_name',
- 'gross' => 'amount',
- 'gateway_session_id' => 'gateway_session_id',
- 'last_name' => 'last_name',
- 'optout' => 'optout',
- 'recurring' => 'recurring',
- 'risk_score' => 'risk_score',
- 'state_province' => 'state_province',
- 'street_address' => 'street_address',
- 'supplemental_address_1' => 'supplemental_address_1',
- 'subscr_id' => 'subscr_id',
- 'utm_campaign' => 'utm_campaign',
- 'utm_medium' => 'utm_medium',
- 'postal_code' => 'postal_code',
- );
- foreach ( $optional_keys as $mkey => $tkey ) {
- if ( isset( $transaction[$tkey] ) ) {
- $message[$mkey] = $transaction[$tkey];
- } elseif ( isset( $transaction[$mkey] ) ) {
- // Just copy if it's already using the correct
key.
- $message[$mkey] = $transaction[$mkey];
- }
- }
-
- return $message;
}
/**
diff --git a/gateway_common/gateway.adapter.php
b/gateway_common/gateway.adapter.php
index f26ffd0..b9a15cd 100644
--- a/gateway_common/gateway.adapter.php
+++ b/gateway_common/gateway.adapter.php
@@ -1763,42 +1763,64 @@
}
/**
- * Formats an array in preparation for dispatch to a STOMP queue
+ * Collect donation details and normalize keys for pending or
+ * donations queue
*
- * @return array Pass this return array to STOMP :)
- *
- * TODO: Stop saying "STOMP".
+ * @return array
*/
- protected function getStompTransaction() {
- $transaction = array(
+ protected function getQueueDonationMessage() {
+ $queueMessage = array(
'gateway_txn_id' => $this->getTransactionGatewayTxnID(),
'response' => $this->getTransactionMessage(),
- // Can this be deprecated?
- 'correlation-id' => $this->getCorrelationID(),
- 'php-message-class' =>
'SmashPig\CrmLink\Messages\DonationInterfaceMessage',
'gateway_account' => $this->account_name,
+ 'fee' => 0, // FIXME: don't we know this for some
gateways?
+ );
+
+ $messageKeys = DonationData::getMessageFields();
+
+ $requiredKeys = array(
+ 'amount',
+ 'contribution_tracking_id',
+ 'country',
+ 'gateway',
+ 'language',
+ 'order_id',
+ 'payment_method',
+ 'payment_submethod',
+ 'user_ip',
+ 'utm_source',
+ );
+
+ $remapKeys = array(
+ 'amount' => 'gross',
);
// Add the rest of the relevant data
// FIXME: This is "normalized" data. We should refer to it as
such,
// and rename the getData_Unstaged_Escaped function.
- $stomp_data = array_intersect_key(
- $this->getData_Unstaged_Escaped(),
- array_flip( $this->dataObj->getMessageFields() )
- );
-
- // The order here is important, values in $transaction are
considered more definitive
- // in case the transaction already had keys with those values
- $transaction = array_merge( $stomp_data, $transaction );
-
+ $data = $this->getData_Unstaged_Escaped();
+ foreach ( $messageKeys as $key ) {
+ if ( isset( $queueMessage[$key] ) ) {
+ // don't clobber the pre-sets
+ continue;
+ }
+ if ( !isset( $data[$key] ) ) {
+ if ( in_array( $key, $requiredKeys ) ) {
+ throw new RuntimeException( "Missing
required message key $key" );
+ }
+ continue;
+ }
+ $value = Encoding::toUTF8( $data[$key] );
+ if ( isset( $remapKeys[$key] ) ) {
+ $queueMessage[$remapKeys[$key]] = $value;
+ } else {
+ $queueMessage[$key] = $value;
+ }
+ }
// FIXME: Note that we're not using any existing date or ts
fields. Why is that?
- $transaction['date'] = time();
+ $queueMessage['date'] = time();
- // Force any incorrect encoding to UTF-8.
- // FIXME: Move down to the PHP-Queue library
- $transaction = Encoding::toUTF8( $transaction );
-
- return $transaction;
+ return $queueMessage;
}
public function makeFreeformStompTransaction( $transaction ) {
@@ -1807,12 +1829,8 @@
$transaction['php-message-class'] =
'undefined-loser-message';
}
- // Mark as freeform so we avoid normalization.
- $transaction['freeform'] = true;
-
//bascially, add all the stuff we have come to take for
granted, because syslog.
$transaction['gateway_txn_id'] =
$this->getTransactionGatewayTxnID();
- $transaction['correlation-id'] = $this->getCorrelationID();
$transaction['date'] = ( int ) time(); //I know this looks odd.
Just trust me here.
$transaction['server'] = WmfFramework::getHostname(); // FIXME:
duplicated in the source fields
@@ -2234,13 +2252,13 @@
protected function pushMessage( $queue ) {
$this->logger->info( "Pushing transaction to queue [$queue]" );
- DonationQueue::instance()->push( $this->getStompTransaction(),
$queue );
+ DonationQueue::instance()->push(
$this->getQueueDonationMessage(), $queue );
}
protected function sendPendingMessage() {
$order_id = $this->getData_Unstaged_Escaped( 'order_id' );
$this->logger->info( "Sending donor details for $order_id to
pending queue" );
- DonationQueue::instance()->push( $this->getStompTransaction(),
'pending' );
+ DonationQueue::instance()->push(
$this->getQueueDonationMessage(), 'pending' );
}
/**
@@ -3617,7 +3635,7 @@
}
protected function logPaymentDetails( $preface = self::REDIRECT_PREFACE
) {
- $details = $this->getStompTransaction();
+ $details = $this->getQueueDonationMessage();
$json = json_encode( $details );
$this->logger->info( $preface . $json );
}
diff --git a/globalcollect_gateway/orphan.adapter.php
b/globalcollect_gateway/orphan.adapter.php
index 24a6a18..4956810 100644
--- a/globalcollect_gateway/orphan.adapter.php
+++ b/globalcollect_gateway/orphan.adapter.php
@@ -163,8 +163,8 @@
*
* FIXME: Carefully move this to the base class and decide when
appropriate.
*/
- protected function getStompTransaction() {
- $transaction = parent::getStompTransaction();
+ protected function getQueueDonationMessage() {
+ $transaction = parent::getQueueDonationMessage();
// Overwrite the time field, if historical date is available.
if ( !is_null( $this->getData_Unstaged_Escaped( 'date' ) ) ) {
diff --git a/tests/phpunit/Adapter/Adyen/AdyenTest.php
b/tests/phpunit/Adapter/Adyen/AdyenTest.php
index cc45e92..e76995d 100644
--- a/tests/phpunit/Adapter/Adyen/AdyenTest.php
+++ b/tests/phpunit/Adapter/Adyen/AdyenTest.php
@@ -103,7 +103,7 @@
$exposed = TestingAccessWrapper::newFromObject( $gateway );
$exposed->risk_score = 57;
- $message = $exposed->getStompTransaction();
+ $message = $exposed->getQueueDonationMessage();
$this->assertEquals( 57, $message['risk_score'], 'Risk score
was not correctly added to queue message.' );
}
diff --git a/tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
b/tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
index 3aa68b7..a57ce09 100644
--- a/tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
+++ b/tests/phpunit/Adapter/PayPal/PayPalExpressTest.php
@@ -182,7 +182,7 @@
'postal_code' => '94105',
'source_name' => 'DonationInterface',
'source_type' => 'payments',
- 'subscr_id' => 'I-88J1M3DLSF0'
+ 'subscr_id' => 'I-88J1M3DLSF0',
);
$this->assertEquals( $expected, $message );
$this->assertNull(
diff --git a/tests/phpunit/DonationQueueTest.php
b/tests/phpunit/DonationQueueTest.php
index e895d99..6d5d453 100644
--- a/tests/phpunit/DonationQueueTest.php
+++ b/tests/phpunit/DonationQueueTest.php
@@ -41,11 +41,10 @@
) );
$this->transaction = array(
- 'amount' => '1.24',
+ 'gross' => '1.24',
+ 'fee' => '0',
'city' => 'Dunburger',
'contribution_tracking_id' => mt_rand(),
- // FIXME: err, we're cheating normalization here.
- 'correlation-id' => 'testgateway-' . mt_rand(),
'country' => 'US',
'currency' => 'USD',
'date' => time(),
@@ -59,7 +58,6 @@
'last_name' => 'Russ',
'payment_method' => 'cc',
'payment_submethod' => 'visa',
- 'php-message-class' =>
'SmashPig\CrmLink\Messages\DonationInterfaceMessage',
'response' => 'Gateway response something',
'state_province' => 'AK',
'street_address' => '1 Fake St.',
@@ -116,7 +114,7 @@
DonationQueue::instance()->push( $this->transaction,
$this->queue_name );
$transaction2 = $this->transaction;
- $transaction2['correlation-id'] = mt_rand();
+ $transaction2['order_id'] = mt_rand();
DonationQueue::instance()->push( $transaction2,
$this->queue_name );
diff --git a/tests/phpunit/GatewayPageTest.php
b/tests/phpunit/GatewayPageTest.php
index 318055f..74b651d 100644
--- a/tests/phpunit/GatewayPageTest.php
+++ b/tests/phpunit/GatewayPageTest.php
@@ -175,7 +175,8 @@
'payment_submethod' => '',
'first_name' => 'Firstname',
'last_name' => 'Surname',
- 'amount' => '1.55',
+ 'gross' => '1.55',
+ 'fee' => 0,
'language' => 'en',
'email' => '[email protected]',
'country' => 'US',
@@ -191,14 +192,12 @@
'city' => 'San Francisco',
'state_province' => 'CA',
'postal_code' => '94105',
- 'php-message-class' =>
'SmashPig\CrmLink\Messages\DonationInterfaceMessage',
);
$actual = json_decode( $detailString, true );
// TODO: when tests use PHPUnit 4.4
// $this->assertArraySubset( $expected, $actual, false, 'Logged
the wrong stuff' );
$expected['order_id'] = $actual['contribution_tracking_id'];
unset( $actual['contribution_tracking_id'] );
- unset( $actual['correlation-id'] );
unset( $actual['date'] );
$this->assertEquals( $expected, $actual, 'Logged the wrong
stuff!' );
}
diff --git a/tests/phpunit/LoggingTest.php b/tests/phpunit/LoggingTest.php
index 7ce4655..87f0bba 100644
--- a/tests/phpunit/LoggingTest.php
+++ b/tests/phpunit/LoggingTest.php
@@ -53,9 +53,10 @@
unset( $init['order_id'] );
$expectedObject = array(
- 'amount' => 23.45,
+ 'gross' => 23.45,
'city' => 'San Francisco',
//'contribution_tracking_id' => '1',
+ 'fee' => 0,
'country' => 'US',
'currency' => 'EUR',
'email' => '[email protected]',
@@ -72,7 +73,6 @@
'utm_source' => '..cc',
'postal_code' => '94105',
'response' => 'Original Response Status
(pre-SET_PAYMENT): 200',
- 'php-message-class' =>
'SmashPig\CrmLink\Messages\DonationInterfaceMessage',
'gateway_account' => 'test',
);
@@ -107,7 +107,8 @@
unset( $init['order_id'] );
$expectedObject = array(
- 'amount' => 23.45,
+ 'gross' => 23.45,
+ 'fee' => 0,
'city' => 'San Francisco',
'country' => 'US',
'currency' => 'EUR',
@@ -125,7 +126,6 @@
'utm_source' => '..cc',
'postal_code' => '94105',
'response' => 'Original Response Status
(pre-SET_PAYMENT): 200',
- 'php-message-class' =>
'SmashPig\CrmLink\Messages\DonationInterfaceMessage',
'gateway_account' => 'test',
);
@@ -147,7 +147,6 @@
protected function stripRandomFields( $data ) {
$toUnset = array(
'contribution_tracking_id',
- 'correlation-id',
'date',
'gateway_txn_id',
'order_id',
--
To view, visit https://gerrit.wikimedia.org/r/354605
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Icffc82d2a14c4e86a705b053a644ce4476f6e33f
Gerrit-PatchSet: 9
Gerrit-Project: mediawiki/extensions/DonationInterface
Gerrit-Branch: master
Gerrit-Owner: Ejegg <[email protected]>
Gerrit-Reviewer: AndyRussG <[email protected]>
Gerrit-Reviewer: Awight <[email protected]>
Gerrit-Reviewer: Cdentinger <[email protected]>
Gerrit-Reviewer: Eileen <[email protected]>
Gerrit-Reviewer: Ejegg <[email protected]>
Gerrit-Reviewer: Katie Horn <[email protected]>
Gerrit-Reviewer: Mepps <[email protected]>
Gerrit-Reviewer: XenoRyet <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
