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#1597 & dev/core#1595 fix regression when deduping on custom fields #16558

Merged
merged 1 commit into from
Feb 17, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Feb 16, 2020

Overview

Fixes a regression where dedupe rules were failing when a custom field is the match field

Before

Create a dedupe rule with just a custom field as the critieria & try to use it -> fatal (bonus points use a custom date field - which would have already been broken)

After

Custom fields work in a rule

Technical Details

Some time back we started retrieving the metadata for fields because date fields need to be treated differently in the sql. The retrieval method assumed table_name was the Entity but for custom fields that was not the case - so marriage date would have resulted in bad sql but birth_date would have worked. More recently we tightened civicrm_api3() to fail if $entity was NULL so passing NULL entity became a had error (making it fail on all custom fields -not just date custom fields).

This fixes to get the entity correctly for custom fields & adds a test ensuring no fatal

Comments

@civibot
Copy link

civibot bot commented Feb 16, 2020

(Standard links)

@civibot civibot bot added the master label Feb 16, 2020
@eileenmcnaughton eileenmcnaughton changed the title dev/core#1597 & dev/core#1595 fix regression when deduping on custom … dev/core#1597 & dev/core#1595 fix regression when deduping on custom fields Feb 16, 2020
@eileenmcnaughton eileenmcnaughton changed the base branch from master to 5.23 February 16, 2020 21:33
@civibot civibot bot added 5.23 and removed master labels Feb 16, 2020
@seamuslee001
Copy link
Contributor

@eileenmcnaughton i tested this however i ran into an e-notice

Notice: Undefined index: most_important_issue_1 in CRM_Dedupe_BAO_Rule->getFieldSpec() (line 259 of /home/seamus/buildkit/build/47-test/web/sites/all/modules/civicrm/CRM/Dedupe/BAO/Rule.php).

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 try again now

@seamuslee001
Copy link
Contributor

(CiviCRM Review Template WORD-1.2)

@seamuslee001
Copy link
Contributor

@eileenmcnaughton test fails relate maybe need to supply a test data provider or something

@eileenmcnaughton
Copy link
Contributor Author

It must be cleanup

@eileenmcnaughton
Copy link
Contributor Author

test this please

@seamuslee001
Copy link
Contributor

Jenkins re test this please

CRM/Core/DAO.php Outdated
@@ -2578,7 +2578,7 @@ public static function buildOptionsContext($context = NULL) {
* @param string $fieldName
* @return bool|array
*/
public function getFieldSpec($fieldName) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eileenmcnaughton this is change is causing karma to not run successfully

@eileenmcnaughton
Copy link
Contributor Author

unrelated fail

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