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.

Sign up for GitHub

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

Jump to bottom

Fix for #140, changing the logic to remove or add roles only if neede… #623

Open
wants to merge 4 commits into
base: 7.x-master
Choose a base branch
from
Open

Fix for #140, changing the logic to remove or add roles only if neede… #623

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2d0464c
Fix for #140, changing the logic to remove or add roles only if neede…
sadashivdalvi Sep 9, 2020
9ad4e2f
Fixing bug when memberhsip is deleted
sadashivdalvi Feb 4, 2021
bdd5808
Removing code from CRM-16000 as that case is handled with our code an…
sadashivdalvi Apr 20, 2021
4dd680c
Rolerule is a array of rules so handling that
sadashivdalvi Apr 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 40 additions & 65 deletions modules/civicrm_member_roles/civicrm_member_roles.module
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,83 +558,58 @@ function _civicrm_member_roles_sync($ext_uid = NULL, $cid = NULL, $sync_type = N
$error = $e->getMessage();
return $error;
}
//CRM-16000 remove inactive memberships if member has both active and inactive memberships
if ($memberships['count'] > 1) {
$params = array(
'sequential' => 1,
'name' => array('IN' => array("Deceased", "Cancelled", "Pending", "Expired")),
);
try {
$result = civicrm_api3('MembershipStatus', 'get', $params);
}
catch (CiviCRM_API3_Exception $e) {
$error = $e->getMessage();
return $error;
}
foreach ($result['values'] as $value) {
$inactive[] = $value['id'];
}
foreach ($memberships['values'] as $key => $membership) {
//Do not unset if inactive membership status is chosen as an option for synchronization.
$inactiveStatusSync = FALSE;
if (!empty($memberroles[$membership['membership_type_id']])) {
foreach ($memberroles[$membership['membership_type_id']] as $rolerule) {
if (in_array($membership['status_id'], $rolerule['codes']['current'])) {
$inactiveStatusSync = TRUE;
}
}
}
if (in_array($membership['status_id'], $inactive) && !$inactiveStatusSync) {
unset($memberships['values'][$key]);
}
}
}

$contactMemberships = CRM_Utils_Array::value('values', $memberships);
$addRoles = array();
$expRoles = array();

$roleCondition = is_array($rid) ? 'IN' : '=';

if (empty($contactMemberships) && !empty($rid)) {
db_delete('users_roles')->condition('uid', $uid)->condition('rid', $rid, $roleCondition)->execute();
foreach ($contactMemberships as $membership) {
if (is_array($memberroles[$membership['membership_type_id']])) {
foreach ($memberroles[$membership['membership_type_id']] as $rolerule) {
if (in_array($membership['status_id'], $rolerule['codes']['current'])) {
$addRoles[] = $rolerule['rid'];
}
elseif (in_array($membership['status_id'], $rolerule['codes']['expired'])) {
$expRoles[] = $rolerule['rid'];
}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

is this no longer happening now? I just replicated the following problem which wasn't happening with the previous code.

  • Create 2 member sync rule. Eg role1 => memtype1 & role2 => memtype2.
  • Create a contact (with drupal user) and add a membership of type = memtype1.
  • Sync the rule. role1 is added to the user (Correct).
  • Delete the membership memtype1 from the contact and add the membership of type = memtype2.
  • Sync the rule again. role2 is added (Correct). But role1 is not removed from the contact (Incorrect).

As memtype1 is not associated with the civi contact, role1 should have been removed after the sync. It was working as per the above condition.

@sadashivdalvi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jitendrapurohit can you please recheck, I missed that case, which I fixed by adding additional check now.

foreach ($memberroles as $membership_type_id => $rolerule) {
  if (user_has_role($rolerule['rid'], $account) && !in_array($rolerule['rid'], $addRoles)) {
    // This will only happen if the membership is deleted.
    $expRoles[] = $rolerule['rid'];
  }
}

Simple check that if the user has a role in the rule and doesn't have it addRoles then remove that role.

Have also improved the saving logic by moving the save out of loops, something I missed last time.

Thanks,
Sadashiv

else {
foreach ($contactMemberships as $membership) {
if (!empty($rid)) {
db_delete('users_roles')->condition('uid', $uid)->condition('rid', $rid, $roleCondition)->execute();
$account = user_load($uid, TRUE);
foreach ($memberroles as $membership_type_id => $rolerule) {
foreach ($rolerule as $rule) {
if (user_has_role($rule['rid'], $account) && !in_array($rule['rid'], $addRoles)) {
// This will only happen if the membership is deleted.
$expRoles[] = $rule['rid'];
}
if (is_array($memberroles[$membership['membership_type_id']])) {
foreach ($memberroles[$membership['membership_type_id']] as $rolerule) {
if (in_array($membership['status_id'], $rolerule['codes']['current'])) {
$addRoles[] = $rolerule['rid'];
}
elseif (in_array($membership['status_id'], $rolerule['codes']['expired'])) {
$expRoles[] = $rolerule['rid'];
}
}
if (count($addRoles) > 0 || count($expRoles) > 0) {
$addRoles = array_unique($addRoles);
$expRoles = array_unique($expRoles);
if ($account !== FALSE) {
// Remove expired roles
$userNeedsUpdate = FALSE;
if (!empty($expRoles)) {
foreach ($expRoles as $expiredRole) {
if (isset($account->roles[$expiredRole])) {
unset($account->roles[$expiredRole]);
$userNeedsUpdate = TRUE;
}
}
}
if (count($addRoles) > 0 || count($expRoles) > 0) {
$addRoles = array_unique($addRoles);
$expRoles = array_unique($expRoles);
$account = user_load($uid, TRUE);
if ($account !== FALSE) {
// Retain a user's existing roles that aren't explicitly expired--the assumption is that roles granted
// manually won't have expired memberships that correspond to those roles. If a status is neither
// current nor expired, the membership will have no effect on the role.

$rolesRetain = array_diff_key($account->roles, array_flip($expRoles));

// Certainly add all roles that correspond to current memberships

foreach ($addRoles as $addRole) {
$rolesRetain[$addRole] = $allRoles[$addRole];
}

// Overwrite the user's roles to the ones we've set
user_save($account, array('roles' => $rolesRetain));
// Add new roles
foreach ($addRoles as $addRole) {
if (!user_has_role($addRole, $account)) {
$account->roles[$addRole] = $allRoles[$addRole];
$userNeedsUpdate = TRUE;
}
}

if ($userNeedsUpdate) {
// Overwrite the user's roles to the ones we've set
user_save($account, ['roles' => $account->roles]);
}
}
}
}
Expand Down