From b14ce77343d544f2895954c1469f6a7c5d828446 Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 15 May 2013 22:54:25 +1200 Subject: [PATCH] CRM12547, CRM-12357 fix Mailing Contact api to comply with api conformance tests fix includes - - using spec function to provide documentation, enforce required fields, provide default, enforce options - using standard api function to process 'limit', 'sort' etc - skipping checks for getByID because in this area the api cannot be made standard NOTE that the default limit has been set to the api default (25). Making this non standard for a particular use-case doesn't make sense. Preferred calling method is $params['options' => array('sort' => 'contact_id DESC', 'limit' => 50, 'offset' => 200) re-fix mailing contact api --- api/v3/MailingContact.php | 82 +++++++++---------- tests/phpunit/api/v3/MailingContactTest.php | 49 ++++++----- .../v3/SyntaxConformanceAllEntitiesTest.php | 18 +++- 3 files changed, 82 insertions(+), 67 deletions(-) diff --git a/api/v3/MailingContact.php b/api/v3/MailingContact.php index a6fa23d0b8..1da7473435 100644 --- a/api/v3/MailingContact.php +++ b/api/v3/MailingContact.php @@ -51,37 +51,44 @@ * */ function civicrm_api3_mailing_contact_get($params) { - if (empty($params['contact_id'])) { - return civicrm_api3_create_error('contact_id is a required field'); - } - - if (empty($params['type'])) { - $params['type'] = 'Delivered'; - } - - $validTypeValues = array('Delivered', 'Bounced'); - if (!in_array($params['type'], $validTypeValues)) { - return civicrm_api3_create_error( - 'type should be one of the following: ' . - implode(', ', $validTypeValues) - ); - } - - if (!isset($params['offset'])) { - $params['offset'] = 0; - } - - if (!isset($params['limit'])) { - $params['limit'] = 50; + return civicrm_api3_create_success(_civicrm_api3_mailing_contact_getresults($params, FALSE)); +} +/** + * This is a wrapper for the functions that return the results from the 'quasi-entity' + * mailing contact + * @param array $params + * @param Boolean $count + * @throws Exception + */ +function _civicrm_api3_mailing_contact_getresults($params, $count){ + if(empty($params['type'])){ + //ie. because the api is an anomoly & passing in id is not valid + throw new Exception('This api call does not accept api as a parameter'); } - + $options = _civicrm_api3_get_options_from_params($params, TRUE,'contribution','get'); $fnName = '_civicrm_api3_mailing_contact_get_' . strtolower($params['type']); return $fnName( - $params['contact_id'], - $params['offset'], - $params['limit'], - CRM_Utils_Array::value('sort', $params), - CRM_Utils_Array::value('getcount', $params) + $params['contact_id'], + $options['offset'], + $options['limit'], + $options['sort'], + $count + ); +} +/** + * Adjust Metadata for Get action + * + * @param array $params array or parameters determined by getfields + */ +function _civicrm_api3_mailing_contact_get_spec(&$params) { + $params['contact_id']['api.required'] = 1; + $params['type'] = array( + 'api.default' => 'Delivered', + 'type' => CRM_Utils_Type::T_STRING, + 'options' => array( + 'Delivered' => 'Delivered', + 'Bounced' => 'Bounced', + ) ); } @@ -121,7 +128,7 @@ GROUP BY m.id 'contact_id' => $contactID ); - $results = array('count' => $dao->N); + $results = $dao->N; } else { $defaultFields = array( @@ -182,16 +189,9 @@ LIMIT %2, %3 $results[$dao->mailing_id][$l] = $dao->$l; } } - - $params = array( - 'type' => $type, - 'contact_id' => $contactID, - 'offset' => $offset, - 'limit' => $limit - ); } - return civicrm_api3_create_success($results, $params); + return $results; } function _civicrm_api3_mailing_contact_get_delivered( @@ -264,11 +264,5 @@ INNER JOIN civicrm_mailing_event_bounce meb ON meb.event_queue_id = meq.id * */ function civicrm_api3_mailing_contact_getcount($params) { - if (empty($params['contact_id'])) { - return civicrm_api3_create_error('contact_id is a required field'); - } - - // set the count mode for the api - $params['getcount'] = 1; - return civicrm_api3_mailing_contact_get($params); + return _civicrm_api3_mailing_contact_getresults($params, TRUE); } diff --git a/tests/phpunit/api/v3/MailingContactTest.php b/tests/phpunit/api/v3/MailingContactTest.php index bdc7eaa27c..08d7647781 100644 --- a/tests/phpunit/api/v3/MailingContactTest.php +++ b/tests/phpunit/api/v3/MailingContactTest.php @@ -52,7 +52,7 @@ class api_v3_MailingContactTest extends CiviUnitTestCase { 'version' => $this->_apiversion, ); $this->_contact = civicrm_api("contact", "create", $this->_contact_params); - + /*$this->quickCleanup( array( 'civicrm_mailing', @@ -67,33 +67,42 @@ class api_v3_MailingContactTest extends CiviUnitTestCase { function tearDown() { parent::tearDown(); civicrm_api("contact", "delete", $this->_contact_id); - + } - + /* * Test that the api responds correctly to null params */ - + public function testMailingNullParams() { $result = civicrm_api('MailingContact', 'get', null); $this->assertEquals($result['is_error'], 1, "In line " . __LINE__); } - + public function testMailingContactGetFields() { + $result = civicrm_api('MailingContact', 'getfields', array( + 'version' => 3, + 'action' => 'get', + ) + ); + $this->assertAPISuccess($result); + $this->assertEquals('Delivered', $result['values']['type']['api.default']); + } + /* * Test that the api will return the proper error when you do not * supply the contact_id */ - + public function testMailingNoContactID() { $params = array( 'something' => 'This is not a real field', 'version' => $this->_apiversion, ); - + $result = civicrm_api('MailingContact', 'get', $params); $this->assertEquals($result['is_error'], 1, "In line " . __LINE__); } - + /* * Test that invalid contact_id return with proper error messages */ @@ -106,7 +115,7 @@ class api_v3_MailingContactTest extends CiviUnitTestCase { $result = civicrm_api('MailingContact', 'get', $params); $this->assertEquals($result['is_error'], 1, "In line " . __LINE__); } - + /* * Test that invalid types are returned with appropriate errors */ @@ -120,8 +129,8 @@ class api_v3_MailingContactTest extends CiviUnitTestCase { $result = civicrm_api('MailingContact', 'get', $params); $this->assertEquals($result['is_error'], 1, "In line " . __LINE__); } - - + + /* * Test that the API returns properly when there are no mailings * for a the given contact @@ -133,12 +142,12 @@ class api_v3_MailingContactTest extends CiviUnitTestCase { ); $result = civicrm_api('MailingContact', 'get', $params); - + $this->assertEquals($result['is_error'], 0, "In line " . __LINE__); $this->assertEquals($result['count'], 0, "In line " . __LINE__); $this->assertTrue(empty($result['values']), "In line " . __LINE__); } - + /* * Test that the API returns a mailing properly when there is only one */ @@ -156,7 +165,7 @@ class api_v3_MailingContactTest extends CiviUnitTestCase { dirname(__FILE__) . '/dataset/mailing_delivered.xml' ) ); - + $params = array( 'contact_id' => 23, 'type' => 'Delivered', @@ -172,8 +181,8 @@ class api_v3_MailingContactTest extends CiviUnitTestCase { $this->assertEquals($result['values'][1]['creator_id'], 1, "In line " . __LINE__); $this->assertEquals($result['values'][1]['creator_name'], "xyz1, abc1", "In line " . __LINE__); } - - + + /* * Test that the API returns only the "Bounced" mailings when instructed to do so */ @@ -191,7 +200,7 @@ class api_v3_MailingContactTest extends CiviUnitTestCase { dirname(__FILE__) . '/dataset/mailing_bounced.xml' ) ); - + $params = array( 'contact_id' => 23, 'type' => 'Bounced', @@ -207,7 +216,7 @@ class api_v3_MailingContactTest extends CiviUnitTestCase { $this->assertEquals($result['values'][2]['creator_id'], 1, "In line " . __LINE__); $this->assertEquals($result['values'][2]['creator_name'], "xyz1, abc1", "In line " . __LINE__); } - - - + + + } diff --git a/tests/phpunit/api/v3/SyntaxConformanceAllEntitiesTest.php b/tests/phpunit/api/v3/SyntaxConformanceAllEntitiesTest.php index a820b51119..dc6bd3e46a 100644 --- a/tests/phpunit/api/v3/SyntaxConformanceAllEntitiesTest.php +++ b/tests/phpunit/api/v3/SyntaxConformanceAllEntitiesTest.php @@ -115,7 +115,16 @@ class api_v3_SyntaxConformanceAllEntitiesTest extends CiviUnitTestCase { } return $entities; } - + /** + * Mailing Contact Just doesn't support id. We have always insisted on finding a way to + * support id in API but in this case the underlying tables are crying out for a restructue + * & it just doesn't make sense + * @param unknown_type $sequential + * @return multitype:string |multitype:multitype:string + */ + public static function toBeSkipped_getByID($sequential = FALSE) { + return array('MailingContact'); + } public static function toBeSkipped_create($sequential = FALSE) { $entitiesWithoutCreate = array('MailingGroup', 'Constant', 'Entity', 'Location', 'Profile', 'MailingRecipients'); @@ -146,7 +155,7 @@ class api_v3_SyntaxConformanceAllEntitiesTest extends CiviUnitTestCase { * @return multitype:string |multitype:multitype:string */ public static function toBeSkipped_automock($sequential = FALSE) { - $entitiesWithoutGet = array('EntityTag', 'Participant', 'ParticipantPayment', 'Setting', 'SurveyRespondant', 'MailingRecipients', 'CustomSearch', 'Extension', 'ReportTemplate', 'System'); + $entitiesWithoutGet = array('MailingContact', 'EntityTag', 'Participant', 'ParticipantPayment', 'Setting', 'SurveyRespondant', 'MailingRecipients', 'CustomSearch', 'Extension', 'ReportTemplate', 'System'); if ($sequential === TRUE) { return $entitiesWithoutGet; } @@ -224,6 +233,7 @@ class api_v3_SyntaxConformanceAllEntitiesTest extends CiviUnitTestCase { 'PaymentProcessor', 'MailSettings', 'Setting', + 'MailingContact', ); if ($sequential === TRUE) { return $entitiesWithout; @@ -390,7 +400,9 @@ class api_v3_SyntaxConformanceAllEntitiesTest extends CiviUnitTestCase { public function testAcceptsOnlyID_get($Entity) { // big random number. fun fact: if you multiply it by pi^e, the result is another random number, but bigger ;) $nonExistantID = 30867307034; - if (in_array($Entity, $this->toBeImplemented['get'])) { + if (in_array($Entity, $this->toBeImplemented['get']) + || in_array($Entity, $this->toBeSkipped_getByID()) + ) { return; } -- 2.25.1