CiviCRM Community Forums (archive)

I think this function is incorrect:
function civicrm_member_roles_sync_user($account) {
  if (!civicrm_initialize()) {
    return;
  }
  if (
    variable_get('civicrm_member_roles_sync_method', 0) == 0 ||
    variable_get('civicrm_member_roles_sync_method', 0) == 3
  ) {
    _civicrm_member_roles_sync($account->uid);
  }
}
Sync method 3 is during a membership change, 0 is during login or logout. Checking for method 3 here is not needed to fire the role sync when a membership is updated.

I had a client with a problem where their rule was supposed to fire during role updates, was being triggered when the users logged in or out. I traced it back to this function. If i delete the check for method 3 the behavior is corrected and the role sync still works during membership change as expected.

Back in 4.1.1 this used to have an additions check "$op == 'insert" that got removed at some point. I'm wondering if this is some legacy code from supporting D6?

So my question is shouldn't we just delete this: "|| variable_get('civicrm_member_roles_sync_method', 0) == 3" ?