Add v4 pledge api #19297
Add v4 pledge api #19297
Conversation
|
(Standard links)
|
|
test this please |
1 similar comment
|
test this please |
|
|
|
test this please |
eileenmcnaughton
force-pushed
the
pledge
branch
from
6f31fd0 to
893fd61
Compare
| foreach ($fields as $name => $field) { | ||
| // If a default value in the api field is different than in core, the api should override it. | ||
| if (!isset($params[$name]) && !empty($field['default_value']) && $field['default_value'] != \CRM_Utils_Array::pathGet($coreFields, [$name, 'default'])) { | ||
| protected function fillDefaults(&$params): void { |
There was a problem hiding this comment.
@colemanw @totten @seamuslee001 thoughts on this change?
The previous code only added the default to the params if it was not the same as the db default. However, in pledge BAO it really needs those values to be set in params to do calculations. I could make the pledge BAO do the work of looking up the defaults for apiv4 but I couldn't see why it would make sense to not set them here. It seems to have been committed as part of 'add apiv4' which doesn't give too many clues - especially as much of that has since been refactored. I did have to ensure inappropriate-to-php defaults were not added to the php defaults which I did further down
There was a problem hiding this comment.
@eileenmcnaughton It feels wrong to me to pass in db defaults. I think this could lead to bugs or problems because now every BAO would have no way of knowing the difference between an empty param and a param that was passed in explicitly with the default value.
Generally I would think it's not a good idea to impact every Api in this way because of one oddball BAO, and preferable to fix the oddball.
There was a problem hiding this comment.
Hmm OK - you arrived at a different conclusion about what makes sense than I did - but I'll switch over
eileenmcnaughton
force-pushed
the
pledge
branch
from
893fd61 to
4a5f275
Compare
|
test this please |
eileenmcnaughton
force-pushed
the
pledge
branch
from
4a5f275 to
207a83b
Compare
eileenmcnaughton
force-pushed
the
pledge
branch
from
207a83b to
7c86e53
Compare
|
@colemanw @seamuslee001 it's passing! |
|
Awesome! |
Overview
Add v4 api for pledge
Before
no api
After
tada
Technical Details
I tried to add this in #19289 to support that change but it seems to be tricksie so trying to add the api first (& work through any test issues)
Comments