-
-
Notifications
You must be signed in to change notification settings - Fork 824
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add API parameter for fix_address #11372
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contributing! Can you please fill out the rest of the description for this PR and check the code comments I've made?
api/v3/Address.php
Outdated
@@ -110,6 +114,11 @@ function _civicrm_api3_address_create_spec(&$params) { | |||
at once, the job \'Geocode and Parse Addresses\' can execute this task after the import)', | |||
'type' => CRM_Utils_Type::T_BOOLEAN, | |||
); | |||
$params['fix_address'] = array( | |||
'title' => 'Fix address', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add ts()
around the name.
api/v3/Address.php
Outdated
$params['fix_address'] = array( | ||
'title' => 'Fix address', | ||
'description' => 'When true, apply various fixes to the address before insert. Default true.', | ||
'type' => CRM_Utils_Type::T_BOOLEAN, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can add 'api_default' => TRUE
and then you won't need the additions on L79.
Thanks for the review @colemanw ! I've added the ts() calls and finished the PR description. |
oops, seems like the api_default didn't actually guarantee it being set. |
@ejegg my mistake it should have been |
Defaults to true, but lets you skip address fixes if desired.
Note: I had to add the explicit default setting back in because api.default is not consulted when an API call is made with the 'id' parameter set. |
Hi @colemanw , I think I've made all the changes you requested. Is there something I should do here in github to formally indicate that and get rid of the 'Changes requested' status with the red 'X' mark? |
This looks great. Thanks @ejegg |
Overview
Expose fix_address parameter in mailing address creation API call
Before
Every address created via API has fixes applied to it, adding potentially unnecessary database overhead.
After
Address creation API calls can include 'fix_address' => FALSE to skip fixes.
Technical Details
Simply exposes the existing second parameter to CRM_Core_BAO_Address::add.
Comments
Already in use at the Wikimedia Foundation, seems to be working fine.