-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
[NFC] Minor code cleanup #16823
[NFC] Minor code cleanup #16823
Conversation
(Standard links)
|
*/ | ||
function civicrm_api3_contact_delete($params) { | ||
$contactID = CRM_Utils_Array::value('id', $params); | ||
$contactID = (int) $params['id']; |
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.
Required param so we don't need to check it - must be an int so let's cast to one before passing on
@@ -466,15 +469,13 @@ function civicrm_api3_contact_delete($params) { | |||
// restrict permanent delete if a contact has financial trxn associated with it | |||
$error = NULL; | |||
if ($skipUndelete && CRM_Financial_BAO_FinancialItem::checkContactPresent([$contactID], $error)) { | |||
return civicrm_api3_create_error($error['_qf_default']); | |||
throw new API_Exception($error['_qf_default']); |
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.
@eileenmcnaughton i note here your throwing an \API_Exception where your otherwise throwing a \CiviCRM_API3_Exception which one should it be?
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.
I was actually just noticing that - I think API_Exception is right - not for a great logical reason - just more consistent. The other is thrown by the wrapper
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.
Fixed
Fix some exception declaration, comparisons, handling for non-present param
2449677
to
17d8f8a
Compare
Agree with the cleanup |
Overview
Fix some exception declaration, comparisons, handling for non-present param
Before
Exceptions not in comment blocks, single quotes where doubles are OK, handling for param that can't be null
After
Above cleaned up
Technical Details
Comments