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

Fix createEmployerRelationship to work with the data it has when doing a lookup, test #23603

Merged
merged 1 commit into from
Jun 1, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Fix createEmployerRelationship to work with the data it has when doing a lookup, test

Before

When doing a batch merge a contact will fail to merge (as a conflict) if they have an existing employer relationship on the relationship

After

Technical Details

Comments

…g a lookup, test

The duplicate check has a bug in it but also it's really complicated
give we specifically know the ids & type and not any other values
@civibot
Copy link

civibot bot commented May 26, 2022

(Standard links)

@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy any chance I could get you to look at this - the replication is basically as per the test

@demeritcowboy
Copy link
Contributor

Ok will take a look.

@eileenmcnaughton
Copy link
Contributor Author

thanks heaps

@demeritcowboy
Copy link
Contributor

Interestingly it still distinguishes between two relationships that are otherwise the same except for a custom field and so (correctly I think) let's you add two of the same that only differ by a custom field. Not sure where that code is happening now but I can see a valid use for that so ok good. It might be more common for a relationship type other than employer, but maybe if you were using relationships to track multiple job roles for a person at the same company or something like that.

@demeritcowboy demeritcowboy merged commit 4d6d6a3 into civicrm:master Jun 1, 2022
@eileenmcnaughton eileenmcnaughton deleted the rel2 branch June 1, 2022 00:43
@eileenmcnaughton
Copy link
Contributor Author

@demeritcowboy yeah - I think there are some conflicting use cases around the whole relationship matching issue tbh

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