CRM-14758 API ( contact, create ) does not always create related membership

This seems to have been introduced by commit https://github.com/civicrm/civicrm-core/commit/90f0b591be665b80265f3b06ca5b523d3d7883e6 file CRM/Contact/BAO/Contact/Utils.php around line 310:
// In case we change employer, clean prveovious employer related records.
if (!$previousEmployerID)

$previousEmployerID is correctly set to null when entering the function from CRM_Contact_BAO_Contact::add(), but this piece of code sets it to the current employer, which is obviously wrong. So then the function self::currentEmployerRelatedMembership called a few lines below does not create the related membership since the $previousEmpID == $employerID.

This is a bit far from JMA's work on CiviAccounts for us to take on.

Recent 'current employer cleanup' changes https://github.com/civicrm/civicrm-core/commit/fad0497c6cd4d50de069403018947b667b37fb36 has fixed the issue. Thanks to Coleman . The issue no longer persist as I have confirmed it via api explorer in my local.

Nicolas I am assigning you back for QA to ensure that the changes work for this issue.

Thanks,
Monish

Monish - Would it be possible to backport this fix in the 4.4 branch?

PS. I have no way to test in a 4.5 environment as all my customers are on 4.4.

Nicolas: Unfortunately the changes aren't compatible with 4.4 so we can't backport it right now. Although you can check that on sandbox for 4.5.

T

Monish: I have checked in the Rupal 4.5 sandbox and the bug is still there. Steps to reproduce:

• created a Corporate memebership (inherited via the Employer of relationship)
• created a couple of employees to an existing organization
• added the membership to the organization, checked that the employees do inherit the membership
• used the API explorer to add an Individual, with current_employer_id = organization above
• go to the organization screen:
• the relationship tab shows 3 current employees
• in the membership tab clicking on the membership shows that the new employee has not inherited the membership

See attachd screenshots for API call and resulting membership record.

Nicolas - as discussed on Skype - if you submit a unit test form 4.4.6 then I think it would make sense to fix in 4.4.7 (if the fix is not a backport then once it is merged we also need a merge PR to master from 4.4 that resolves the merge issue - rather than waiting for it to become problematic to merge - )

Submitted the PR https://github.com/civicrm/civicrm-core/pull/3603

Let me explain the bug for 4.5 and also why it is not occurring at 4.4.5. Nicolas was right about the spot
CRM/Contact/BAO/Contact/Utils.php around line 310:
// In case we change employer, clean prveovious employer related records.
if (!$previousEmployerID)

So in 4.5 contact got saved with all the given param info, before this function createCurrentEmployerRelationship() get called so its obvious that CRM_Core_DAO::getFieldValue('CRM_Contact_DAO_Contact', $contactID, 'employer_id') always get the employer_id and at condition $previousEmployerID will be null because the simply newly created contact didn't have any previous employer id. So
if ($previousEmpID != $employerID)

condition fails (as at this state $previousEmpID == $employerID) , as relatedMemberships() responsible for creating inherited membership.

Now reason why it works for 4.4.5 is that employer id got saved in current_employer but in 4.5 the field name has been changed to employer_id in this commit https://github.com/civicrm/civicrm-core/commit/66aa3a9f6854a8540029d76bc513b6748c7b6c11 that's why in 4.4 $prevEmployer never get updated as current_employer get set not employer_id as in 4.5.

Eileen, Monish,

I have created a test suite for this issue (and other related features). You can cherry-pick this commit in your source tree.
https://github.com/cividesk/civicrm-core/commit/7f8698172129442651202070d36df01ccb7b8d24
Right not this does not pass on 4.4.4, but that is to be expected and proves the test works.

Monish - The above test fails on 4.4.6, like it did on 4.5, so this issue is not fixed at all.
Returning to Development.

Nicolas: There are two minor issues with your test suite. Please the check my comment on https://github.com/cividesk/civicrm-core/commit/7f8698172129442651202070d36df01ccb7b8d24

Can you please check my changes https://github.com/civicrm/civicrm-core/pull/3623 for 4.4. But first you need to make those changes which I mentioned on test suite as I have already tried in my local and the tests results are clean.

And this is the PR for 4.5/master https://github.com/civicrm/civicrm-core/pull/3603

T

welcome to the joys of tests Nicolas - they can be pretty quirky but they are worth the effort

Updated my PR https://github.com/civicrm/civicrm-core/pull/3623 and included test-suite

Thanks for your hard work on this one Monish, I am glad it is fixed on 4.4 & 4.5, and that there is a test suite so it does not creep up again in the future!

Thanks Nicolas. Also, can you please submit a PR for the changes you mentioned in the test suite?

I have commented directly in your PR code on GitHub. I cannot submit PRs as working from a long running fork.

No worries, will bring that in my next PR sooner/later.

T

Only the api test was remained and recent changes have fixed the api failure as I confirmed it on jenkin and on my local. Hence closing the issue.