Skip to content
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

Address BAO - Handle standard 'custom' param as well as individual fields #14535

Merged
merged 1 commit into from
Jun 14, 2019

Conversation

colemanw
Copy link
Member

Overview

The Address BAO was an oddball in that it took raw unformatted custom field input and processed it.
Other BAOs expect the custom field input to alreay be processed.
This allows it to handle either format.

Before

Api4 test fails.

After

Api4 test passes

…elds

The Address BAO was an oddball in that it took raw unformatted custom field input and processed it.
Other BAOs expect the custom field input to alreay be processed.
This allows it to handle either format.
@civibot
Copy link

civibot bot commented Jun 14, 2019

(Standard links)

@eileenmcnaughton
Copy link
Contributor

@colemanw can we add a deprecation notice in the one that is not like other BAO?

@colemanw
Copy link
Member Author

I don't think I'd venture to say that any BAO is exemplary wrt custom field handling. The approach of the Address BAO is IMO smarter, but more verbose and all that boilerplate ought to get moved to a utility function.

@eileenmcnaughton
Copy link
Contributor

@colemanw right & the v3 api handles custom fields within the outer wrapper....

@colemanw
Copy link
Member Author

Yea v4 is handling them in a single point too. But it then expects the underlying BAO to consistently accept an already-formatted $params['custom'], which most do but the Address api did not.

But IMO formatting & saving custom field data is in the domain of a BAO and not an api, so maybe with sufficient unit tests we could do a refactor to make the BAOs more useful and consistent.

@eileenmcnaughton
Copy link
Contributor

right - the problem is we support custom data on ones without BAOs too - so more like a listener almost makes sense.... (punt)

@colemanw
Copy link
Member Author

What's an example of an api entity with no BAO that supports custom fields? I need to make sure Api4 tests cover that.

@eileenmcnaughton
Copy link
Contributor

eileenmcnaughton commented Jun 14, 2019

@colemanw any entity with no BAO - you can have custom fields on any entity (if you add an option_value to cg_extends)

@monishdeb
Copy link
Member

Sorry for being late, agree with this patch, tested on APi4 API3 and via UI. Works like a charm

@eileenmcnaughton eileenmcnaughton merged commit 43fee10 into civicrm:master Jun 14, 2019
@eileenmcnaughton eileenmcnaughton deleted the addressCustom branch June 14, 2019 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants