[REF] clarify variable (very minor change with good test cover) #14724
Conversation
|
(Standard links)
|
eileenmcnaughton
force-pushed
the
cust_field3
branch
from
9da4787 to
ee9a5d5
Compare
The variable is clearer being defined directly as 'add' or 'modify' in the main function to reduce wrangling
eileenmcnaughton
force-pushed
the
cust_field3
branch
from
ee9a5d5 to
e43db21
Compare
|
test this please |
| } | ||
|
|
||
| // complete transaction | ||
| $transaction->commit(); | ||
|
|
||
| CRM_Utils_Hook::post($op, 'CustomField', $customField->id, $customField); | ||
| CRM_Utils_Hook::post(($op === 'add' ? 'create' : 'edit'), 'CustomField', $customField->id, $customField); |
There was a problem hiding this comment.
Can we use same concept here?
empty($params['id']) ? 'create' : 'edit';
Since hook_pre() allows once to change the $params and its possible that one can set $params['id'](very rare case though)
There was a problem hiding this comment.
@pradpnayak I thought about your comment here but in other BAO the op is determined before calling the 'pre' hook & retained for the post hook so that would be inconsistent with other BAO. I am going to add some code comments but to a subsequent PR as they need rebasing after the merges that have been done so far
|
Did a QA on current state of code. Testcase: Added/Updated custom field via UI and api. Safe to merge(If the change requested doesn't make sense) |
Overview
Very minor code cleanup
Before
Less readable
After
More readable
Technical Details
I'm trying to reduce the cognitive load in #14694 - this has thorough test coverage
Comments