Duplicate Inherited memberships

[jackrabbithanna]

I have a strange situation. 

I've setup a membership type that organizations can purchase/have set up that is set to create inherited memberships based on the Employee/Employer Relationship type. 

When I add new individual contacts and set their employer to the organization with the group membership, the individual will receive 2 memberships (one dupe).  The individual has only the one relationship and this is verified in the DB. 

no matter whether the relationship is created manually on the relationship tab or by setting the Employer Field.

Also it makes no difference whether the membership was created via the group membership contrib form or via the backend admin.

Makes no difference the status of the membership.

What does seem to make a difference is whether contacts which already have a relationship exist or not when the memberships is created.  From my testing the situation only happens for contacts that are added and related AFTER the organization has the membership.

Can anyone think of why a contact would inherit 2 memberships instead of one when there is only one Relationship?

Civi 4.3.3

[jackrabbithanna]

Caused by this post hook I had:
But why?  A duplicate relationship is NOT created.....

function hook_civicrm_post( $op, $objectName, $objectId, &$objectRef  ){


if($op=='create'){
if ($objectName=='Relationship'){

$params = array(
  'version' => 3,
  'sequential' => 1,
  'id' => $objectId,
  'is_permission_a_b' => 1,
);
$result = civicrm_api('Relationship', 'create', $params);

}
}

}

[jackrabbithanna]

Updated my hook to be more proper, but the condition still exists.   Some how the api 'create' call is actually generating a membership even though it is updateing (including the 'id' in the $params)

function hook_civicrm_post( $op, $objectName, $objectId, &$objectRef  ){

if($op=='create'){
if ($objectName=='Relationship'){


$params = array(
  'version' => 3,
  'sequential' => 1,
  'id' => $objectId,
   'contact_id_a' => $objectRef->contact_id_a,
   'contact_id_b' => $objectRef->contact_id_b,
     'is_permission_a_b' => 1,
);
$result = civicrm_api('Relationship', 'create', $params);

}
}

}

[jackrabbithanna]

This bug still occurs even when using the obsoleted 'update' parameter to the api call as below:

function awccivicrm_civicrm_post( $op, $objectName, $objectId, &$objectRef  ){



if($op=='create'){
if ($objectName=='Relationship'){

$params = array(
  'version' => 3,
  'sequential' => 1,
  'id' => $objectId,
   'contact_id_a' => $objectRef->contact_id_a,
   'contact_id_b' => $objectRef->contact_id_b,
     'is_permission_a_b' => 1,
);
$result = civicrm_api('Relationship', 'update', $params);

}
}

}

[Eileen]

The function that creates the inherited relationship has been causing problems for a while. Put simply the function should check whether it is required to create an inherited relationship & be called from the BAO.

At the moment the form layer does it's own calculations as to whether to create one & calls the function conditionally - making the behaviour unpredicatable from the API (& meaning it has to be replicated in several places)

[jackrabbithanna]

I am looking into the problem....Dlobo has told me to start looking at the code for bao for relationship create code, that the inherited membership creation is triggered there....maybe look for the check or create a check to see if the individual already has a membership based on that relationship.  I'm going to check out 4.4alpha3 to confirm the issue exists there, and try to fix it there.

when you say "(& menaing it has to be replicated in several places)"

Are you meaning that there are multiple places that this may need to be fixed?

[Donald Lobo]

i was mistaken on irc, primarily because the code in the api has changed between 4.3 and 4.4 (eileen might have an explanation for this)

a. in 4.3, the api does the membership stuff

b. in 4.4, the above code has been removed

i suspect eileen can guide u a bit better on next steps with regard to this. maybe refactor the form code a bit and move it into the BAO?

lobo

[jackrabbithanna]

I was just fixing to post that I noticed significant differences between 4.3 and 4.4 with the following code:

function mymodule_civicrm_post( $op, $objectName, $objectId, &$objectRef  ){


dsm($objectRef);

if($op=='create'){
if ($objectName=='Relationship'){

$params = array(
  'version' => 3,
  'sequential' => 1,
  'id' => $objectId,
   'contact_id_a' => $objectRef->contact_id_a,
   'contact_id_b' => $objectRef->contact_id_b,
     'is_permission_a_b' => 1,
);
$result = civicrm_api('Relationship', 'create', $params);
dsm($result);
}
}

}

So same code used with different civi versions gives you this ordering of dsm() output:

in 4.3 I get output from the dsm() calls in this order:
output for object CRM_Contact_BAO_Relationship
output for object CRM_Contact_BAO_Relationship
output for object  CRM_Member_BAO_Membership
output for object CRM_Activity_DAO_Activity
output for $result no errors, actually updates the relationships is_permission_a_b flag in the end
output for object CRM_Member_BAO_Membership
output for object CRM_Activity_DAO_Activity

using 4.4 alpha3
The code in the end doesn't update the is_permission_a_b field for the relationship record at all

I get output from dsm() calls in this order:
output for object CRM_Contact_BAO_Relationship
then another output for object CRM_Contact_BAO_Relationship
then output for $result with an error, with message "Relationship already exists"
then output for $result with no errors, and the output shows the is_permission_a_b set to 1 (but after all is done, the relationship record does not have the is_permission_a_b flag set to 1)
then output for CRM_Member_BAO_Membership
then output for  CRM_Activity_DAO_Activity

[Eileen]

Hmm - looks like   CRM_Contact_BAO_Relationship::relatedMemberships($params['contact_id_a'], $values, $ids, $action);
 was removed from the api & put in the bao then removed from the BAO because the form was calling it as well

The right fix is for that function to only create the memberships if required & be called from the BAO (& preferably released from the forms - but at least if the function is clever enough to not create them twice it won't matter.

However, if you just want to jam the function back into the API & get the tests to pass it may be quicker (& the technical debt will be kicked down the line).

As an aside the Api-team support for the API is technically that tested behaviour is supported, other behaviour is not. I guess the membership create behaviour must not be tested / supported.

[jackrabbithanna]

Which version of Civi are we talking now, 4.4 alpha3 ?

[Eileen]

yes