Eileen has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/328607 )
Change subject: Do not write address to the database if all data is empty.
......................................................................
Do not write address to the database if all data is empty.
Bug: T153804
Change-Id: I65b1b410dc5f3401174fa5dd19bd6900990a97ad
---
M sites/all/modules/queue2civicrm/tests/phpunit/ProcessMessageTest.php
M sites/all/modules/wmf_civicrm/wmf_civicrm.module
2 files changed, 100 insertions(+), 22 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/wikimedia/fundraising/crm
refs/changes/07/328607/1
diff --git
a/sites/all/modules/queue2civicrm/tests/phpunit/ProcessMessageTest.php
b/sites/all/modules/queue2civicrm/tests/phpunit/ProcessMessageTest.php
index 5c23e3e..c1d2284 100644
--- a/sites/all/modules/queue2civicrm/tests/phpunit/ProcessMessageTest.php
+++ b/sites/all/modules/queue2civicrm/tests/phpunit/ProcessMessageTest.php
@@ -232,19 +232,14 @@
public function testRecurring() {
civicrm_initialize();
$subscr_id = mt_rand();
- $values = array( 'subscr_id' => $subscr_id );
- $signup_message = new RecurringSignupMessage( $values );
+ $values = $this->processRecurringSignup($subscr_id);
+
$message = new RecurringPaymentMessage( $values );
$message2 = new RecurringPaymentMessage( $values );
- $subscr_time = strtotime( $signup_message->get( 'subscr_date' ) );
- exchange_rate_cache_set( 'USD', $subscr_time, 1 );
- exchange_rate_cache_set( $signup_message->get('mc_currency'),
$subscr_time, 3 );
$payment_time = strtotime( $message->get( 'payment_date' ) );
exchange_rate_cache_set( 'USD', $payment_time, 1 );
exchange_rate_cache_set( $message->get('mc_currency'), $payment_time,
3 );
-
- $this->recurringConsumer->processMessage($signup_message->getBody());
$msg = $message->getBody();
db_insert('contribution_tracking')
@@ -280,6 +275,39 @@
db_delete('contribution_tracking')
->condition('id', $msg['custom'])
->execute();
+ }
+
+ /**
+ * Test that the a blank address is not written to the DB.
+ */
+ public function testRecurringBlankEmail() {
+ civicrm_initialize();
+ $subscr_id = mt_rand();
+ $values = $this->processRecurringSignup($subscr_id);
+
+ $message = new RecurringPaymentMessage($values);
+ $this->setExchangeRates($message->get('payment_date'), array('USD' => 1,
$message->get('mc_currency') => 3));
+ $messageBody = $message->getBody();
+
+ $addressFields = array('address_city', "address_country_code",
"address_country", "address_state","address_street", "address_zip");
+ foreach ($addressFields as $addressField) {
+ $messageBody[$addressField] = '';
+ }
+
+ db_insert('contribution_tracking')
+ ->fields(array('id' => $messageBody['custom']))
+ ->execute();
+
+ $this->recurringConsumer->processMessage($messageBody);
+
+ $contributions = wmf_civicrm_get_contributions_from_gateway_id(
+ $message->getGateway(),
+ $message->getGatewayTxnId()
+ );
+ $addresses = $this->callAPISuccess('Address', 'get', array('contact_id'
=> $contributions[0]['contact_id'], 'sequential' => 1));
+ $this->assertEquals(1, $addresses['count']);
+ // The address created by the sign up (Lockwood Rd) should not have been
overwritten by the blank.
+ $this->assertEquals('5109 Lockwood Rd',
$addresses['values'][0]['street_address']);
}
/**
@@ -427,4 +455,20 @@
),
);
}
+
+ /**
+ * Process the original recurring sign up message.
+ *
+ * @param string $subscr_id
+ * @return array
+ */
+ private function processRecurringSignup($subscr_id) {
+ $values = array('subscr_id' => $subscr_id);
+ $signup_message = new RecurringSignupMessage($values);
+ $subscr_time = strtotime($signup_message->get('subscr_date'));
+ exchange_rate_cache_set('USD', $subscr_time, 1);
+ exchange_rate_cache_set($signup_message->get('mc_currency'), $subscr_time,
3);
+ $this->recurringConsumer->processMessage($signup_message->getBody());
+ return $values;
+ }
}
diff --git a/sites/all/modules/wmf_civicrm/wmf_civicrm.module
b/sites/all/modules/wmf_civicrm/wmf_civicrm.module
index 6ffde7d..a3e8b65 100644
--- a/sites/all/modules/wmf_civicrm/wmf_civicrm.module
+++ b/sites/all/modules/wmf_civicrm/wmf_civicrm.module
@@ -1452,6 +1452,34 @@
}
/**
+ * We do not store empty emails or placeholder emails.
+ *
+ * @param array $address
+ * @return bool
+ */
+function wmf_civicrm_is_address_valid($address) {
+ $meaningfulFields = array(
+ 'street_address',
+ 'supplemental_address_1',
+ 'city',
+ 'postal_code',
+ 'country_id',
+ 'state_province_id',
+ );
+ foreach ($meaningfulFields as $field) {
+ if (!empty($address[$field])) {
+ // We could also filter for strings like 'None' here
+ // I feel like there is one like 'None provided' that is common.
+ // only 241 instances of 'none' in street address and second place in the
+ // none-stakes has only 15 instances of 'None of your business'
+ // 'None of your goddamn business' got no traction with only one
instance.
+ return TRUE;
+ }
+ }
+ return FALSE;
+}
+
+/**
* Update address for a contact.
*
* @param array $msg
@@ -1464,26 +1492,29 @@
// CiviCRM does a DB lookup instead of checking the pseudoconstant.
// @todo fix Civi to use the pseudoconstant.
$country_id = wmf_civicrm_get_country_id( $msg[ 'country' ] );
+ $address = array(
+ 'is_primary' => 1,
+ 'street_address' => $msg['street_address'],
+ 'supplemental_address_1' => $msg['supplemental_address_1'],
+ 'city' => $msg['city'],
+ 'postal_code' => $msg['postal_code'],
+ 'country_id' => $country_id,
+ 'country' => $msg['country' ],
+ 'is_billing' => 1,
+ );
+ if (!empty($msg['state_province'])) {
+ $address['state_province'] = $msg['state_province'];
+ $address['state_province_id'] = wmf_civicrm_get_state_id($country_id,
$msg['state_province']);
+ }
+ if (!wmf_civicrm_is_address_valid($address)) {
+ return;
+ }
$address_params = array(
'contact_id' => $contact_id,
'location_type_id' => wmf_civicrm_get_default_location_type_id(),
- 'values' => array(array(
- 'is_primary' => 1,
- 'street_address' => $msg['street_address'],
- 'supplemental_address_1' => $msg['supplemental_address_1'],
- 'city' => $msg['city'],
- 'postal_code' => $msg['postal_code'],
- 'country_id' => $country_id,
- 'country' => $msg['country' ],
- 'is_billing' => 1,
- )),
+ 'values' => array($address),
);
-
- if (!empty($msg['state_province'])) {
- $address_params['values'][0]['state_province'] = $msg['state_province'];
- $address_params['values'][0]['state_province_id'] =
wmf_civicrm_get_state_id($country_id, $msg['state_province']);
- }
try {
civicrm_api3('Address', 'replace', $address_params);
@@ -1528,6 +1559,9 @@
$address_params['state_province_id'] = wmf_civicrm_get_state_id(
$country_id, $msg['state_province'] );
}
+ if (!wmf_civicrm_is_address_valid($address_params)) {
+ return;
+ }
// FIXME: api does not offer control over fixAddress flag
// UPDATE - the fixAddress function mostly backs out early based on params
// checks so as long as things like 'country_id' are set it doesn't seem
very
--
To view, visit https://gerrit.wikimedia.org/r/328607
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I65b1b410dc5f3401174fa5dd19bd6900990a97ad
Gerrit-PatchSet: 1
Gerrit-Project: wikimedia/fundraising/crm
Gerrit-Branch: master
Gerrit-Owner: Eileen <emcnaugh...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
