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

[REF] Centralize BAO handling of custom data #22426

Merged
merged 3 commits into from
Jan 9, 2022

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Jan 8, 2022

Overview

Refactors the BAO to handle custom data from a central writeRecords function, and updates APIv3 accordingly.

Technical Details

  • Refactors a couple BAOs which use writeRecord, as they no longer need to write the custom data themselves.
  • Stops APIv3 passing the deprecated $ids param to BAO::create
  • Switches APIv3 to use BAO::writeRecord when BAO::create or BAO::add are @deprecated
  • In APIv3, trust BAO::writeRecord to handle custom data

Comments

The possible breaking change here is not passing the long-deprecated $ids from APIv3 to BAO::create/add functions, but I note that APIv4 calls those same functions without passing the param and works fine.

Refactors a couple BAOs which use writeRecords, as they no longer need to
write the custom data themselves.
…ctions are deprecated

- Stops passing the deprecated `$ids` param to BAO::create
- Uses BAO::writeRecord when BAO::create or BAO::add are `@deprecated`
- Trust BAO::writeRecord to handle custom data
@civibot
Copy link

civibot bot commented Jan 8, 2022

(Standard links)

@eileenmcnaughton
Copy link
Contributor

Test Result (1 failure / -1)api_v3_NavigationTest.testDefaultDomain with data set "APIv3"

@eileenmcnaughton
Copy link
Contributor

This looks good and we have tonnes of test cover - so if we can get the tests to pass I think it's good to go

@colemanw
Copy link
Member Author

colemanw commented Jan 9, 2022

@eileenmcnaughton was that a MOP? I just added some api_defaults to the v3 Navigation to fix the test.

@eileenmcnaughton
Copy link
Contributor

@colemanw yep - I think this is good - we have heaps of crud testing on apiv3 - it does mean that we can start deprecating bao crud functions with a view to eventual removal

@eileenmcnaughton eileenmcnaughton merged commit 9e06191 into civicrm:master Jan 9, 2022
@colemanw colemanw deleted the writeRecordsCustom branch January 9, 2022 04:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants