APIv4 - Don't automatically set domain. #16911
Conversation
In 60142ef this field was set to be handled automatically across all entities, however this caused problems with entities like optionValue where it can be NULL. So setting a default needs to be the job of the BAO.
|
(Standard links)
|
| @@ -31,7 +31,7 @@ class FieldDomainIdSpecProvider implements Generic\SpecProviderInterface { | |||
| public function modifySpec(RequestSpec $spec) { | |||
| $domainIdField = $spec->getFieldByName('domain_id'); | |||
| if ($domainIdField) { | |||
| $domainIdField->setRequired(FALSE)->setDefaultValue('current_domain');; | |||
There was a problem hiding this comment.
Funny that our linter doesn't catch ;;
|
@colemanw I think the test fail relates here |
|
@seamuslee001 I fixed it by adding a default domain to the MailSettings BAO create. Then I noticed a faulty assumption in the code and fixed that too. |
| } | ||
|
|
||
| //handle is_default. | ||
| if (!empty($params['is_default'])) { | ||
| $domain = $params['domain_id'] ?? CRM_Core_DAO::getFieldValue(__CLASS__, $params['id'], 'domain_id'); |
There was a problem hiding this comment.
We can get away with assuming $params['id'] is set here because the latter half of this ?? expression is only reachable if domain_id is not set, and that can only happen if $params['id'] is not empty per above.
|
@colemanw nope - tests not happy |
|
@eileenmcnaughton @seamuslee001 this PR feels like it's getting out of hand with all the BAO adjustments. Here's a possibly simpler alternative if it works: #16917 |
|
closing in favour of #16917 |
Overview
In 60142ef this field was set to be handled automatically across all entities, however this caused problems with entities like optionValue where it can be NULL.
So setting a default needs to be the job of the BAO.
Before
In all api4 entities,
domain_idfield not required and given default value.After
In all api4 entities,
domain_idfield not required but not given default value.The underlying BAO create function can do that.
Comments
Most BAO create functions already set this default, so it should be fine.