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

dev/core#2046 Migrate BAO_Address::create towards standardisation #18658

Merged
merged 1 commit into from
Oct 6, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Oct 1, 2020

Overview

This moves us towards standardisation on BAO_Address::create - as part of a wider BAO standardisation fix - per https://lab.civicrm.org/dev/core/-/issues/2046 and also as part of identifying and addressing places in the code where is_primary integrity is mismanaged

Our standard expectation is that each BAO will have a create action that handles a single entity. For legacy
reasons Address.create has special handling for multiple addresses. This extracts that handling
into a separate function (legacyCreate) and updates the (very few) places currently calling create to
call that.

Before

BAO_Address::create does not operate like a standard crud action - it expects a key 'address' and creates multiple addresses based on that key

After

BAO_Address::create still accepts the key 'address' and if present the call will be redirected to the original code (in a function now called legacyCreate. However, it also accepts 'normal' crud calls (in which case it acts as a pseudonym for add

Technical Details

With this merged apis v3 & v4 can call Address.create - currently v3 is hard coded to call add and
v4 bypasses the BAO altogether. The v4 api bypass means we are not managing is_primary integrity on v4 api calls

The next step would be to fix apiv4 to call this

Comments

@civibot
Copy link

civibot bot commented Oct 1, 2020

(Standard links)

@civibot civibot bot added the master label Oct 1, 2020
@seamuslee001
Copy link
Contributor

plenty of fails here

Our standard expectation is that each BAO will have a create action that handles a single entity. For legacy
reasons Address.create has special handling for multiple addresses. This extracts that handling
into a separate function (legacyCreate) and updates the (very few) places currently calling create to
call that.

With this merged apis v3 & v4 can call Address.create  - currently v3 is hard coded to call add and
v4 bypasses the BAO altogher. The v4 api bypass means we are not managing is_primay integrity on v4 api calls
@eileenmcnaughton
Copy link
Contributor Author

Ah yes - I still need to handle the case where the function is called for no reason.... - should work now

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 and it's all happy now!

@seamuslee001
Copy link
Contributor

Looks fine merging

@seamuslee001 seamuslee001 merged commit 2d42dc4 into civicrm:master Oct 6, 2020
@seamuslee001 seamuslee001 deleted the address branch October 6, 2020 22:09
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