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 Fix Phone:add to be a pseudonym for Phone.create #18588

Merged
merged 1 commit into from
Sep 25, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Per https://lab.civicrm.org/dev/core/-/issues/2046 we have discussed rationalising add vs create

In the case of phone making this change only affects one place - a place that currently has a
bug that relates specifically to the handling of is_primary & which is more solvable by
calling create with it's extra is_primary handling.

Before

Phone::create calls Phone::add, duplicating CRM_Core_DAO::checkSqlFunctionsExist();
Phone::add does not handle 'is_primary'

After

CRM_Core_DAO::checkSqlFunctionsExist(); only called once - see https://lab.civicrm.org/dev/core/-/issues/27 for further proposal. is_primary handled in add which calls create & is deprecated

Technical Details

Comments

The related bug is that the is_primary handling in Block::create is unreliable &
to mitigate that we are doing an additional 10 queries on every contact create - so
handling is_primary right in the first place helps address that - see https://lab.civicrm.org/dev/core/-/issues/2039

@civibot
Copy link

civibot bot commented Sep 24, 2020

(Standard links)

@colemanw
Copy link
Member

I think unit tests will pick up on any problems with this due to the deprecation notice being triggered.

Per https://lab.civicrm.org/dev/core/-/issues/2046 we have discussed rationalising add vs create

In the case of phone making this change only affects one place - a place that currently has a
bug that relates specifically to the handling of is_primary & which is more solvable by
calling create with it's extra is_primary handling.

(the related bug is that the is_primary handling in Block::create is unreliable &
to mitigate that we are doing an additional 10 queries on every contact create - so
handling is_primary right in the first place helps address that)
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.

2 participants