Change of membership type prior to expiry can result in multiple roles/caps

[kcristiano]

Contact has a membership type of 'regular' expires 12/31/16 They renew at a 'sustaining' level expires 12/31/2017 before 'regular' membership expires.

CiviCRM just changes the record from 'regular' to 'sustaining' with a new expiry date.

Both membership types have the same association rules for status but different roles/caps depending on member type. This is done as different member levels have different caps in the CMS.

After the membership is 'changed' they get the appropriate role or cap for the new member type of 'sustaining'. However, the role/cap for 'regular' is not removed.

Is this behavior intended?

[christianwach]

@kcristiano Hmm. How annoying that CiviCRM doesn't retain the previous membership as expired. As you rightly point out, the membership is replaced when renewal is made. The old caps are not removed because there's no record of the user having ever had the corresponding membership.

So, for example, I get the following passed to Civi_WP_Member_Sync_Members::membership_updated (via hook_civicrm_post) with no hint that a different existing membership has been replaced:

[op] => edit
[objectName] => Membership
[objectId] => 16
[objectRef] => CRM_Member_DAO_Membership Object
(
    [id] => 16
    [contact_id] => 29
    [membership_type_id] => 1
    [join_date] => 2017-08-21
    [start_date] => 2017-08-21
    [end_date] => 2020-08-20
    [source] => Payment
    [status_id] => 2
    ... more data ...
)

Which leads me to the following possible routes forward:

1. I see that there is a "Change Membership Type" activity created when a renewal of the type you describe occurs, so one option could be to look for that and act accordingly if it's found. It seems awkward to have to do this however, since the activity contains no reference to the memberships themselves other than in the subject and is also complicated by the different activity statuses which are dependent on the processing of the associated contribution.

2. Clearing all caps granted to a user by this plugin prior to running rule_apply() might be an option, but we'd have to do this for all possible combinations of capability - which sounds horrible!

3. The only other approach I can think of is to inspect the membership via hook_civicrm_pre and compare the membership_type_id to that received in hook_civicrm_post. CiviCRM does something a bit like this in order to find $oldType so it can decide whether or not to create the "Change Membership Type" activity.

Of all these possibilities, I think I prefer (3) but am open to any other suggestions you may have.

[christianwach]

The only other approach I can think of is to inspect the membership via hook_civicrm_pre and compare the membership_type_id to that received in hook_civicrm_post. CiviCRM does something a bit like this in order to find $oldType so it can decide whether or not to create the "Change Membership Type" activity.

So, it seems that there isn't enough information sent via hook_civicrm_pre to determine a change of membership_type_id without hitting the DB to get the actual membership data. As I linked to previously, CiviCRM itself hits the DB to get this data. Ah well, so be it.

@kcristiano How does ec623c2d53c6b67a0d4a52ffe0167897e932745c work for you?

[kcristiano]

@christianwach Thanks for the work on this.

When a membership is renewed and the membership type is switched from student to regular, we still get multiple roles - in this case lapsed student and regular

There are association rules for both types so this does make sense. In the above case we'd want the switch of member type to allow a switch in roles.

let me know if this makes sense. We do have dev/staging site we can give you access to if needed.

[christianwach]

When a membership is renewed and the membership type is switched from student to regular, we still get multiple roles - in this case lapsed student and regular

@kcristiano Can you post details of the association rules? It occurs to me that they could influence how this fix works - if, for example, the expired roles differ.

We do have dev/staging site we can give you access to if needed.

That would be helpful - ping me on MM or by email.

[kcristiano]

@christianwach yes the expired roles do differ, In a number of use cases we need a different expired role.

[christianwach]

@kcristiano Ah, I thought this might be the case. My fix only works when the Expiry Roles are identical - "Anonymous User" for example. I'll see what I can rustle up that also accommodates your setup.

[christianwach]

@kcristiano 92be1ca7b690bfc1411c980520436fbce20d33d9 considers the previous membership to be deleted before syncing with the new data. This makes sense for your situation but I'm concerned that some people may be using multiple roles and that this change will confound their expectations. Perhaps the plugin needs to be aware of the distinction when using roles? Capability-sync does not suffer from the same issue, thankfully.

[kcristiano]

@christianwach Here is what I see:

• Renewing an expired membership from lapsed_student to General - Sync works perfect
• Renewing a current membership and switching type (general to Life) sync works perfectly
• Adding a new membership (via admin) from expired to a different type (lapsed_student to Life) results in multiple roles and the lapsed role still exists
• Running Manual sync causes expired records to put members in multiple roles.

[christianwach]

@kcristiano Thanks again for testing.

Adding a new membership (via admin) from expired to a different type (lapsed_student to Life) results in multiple roles and the lapsed role still exists

That's the logical and expected outcome in my view - multiple memberships which do not shared the same Expiry Role must necessarily result in multiple roles.

The only way of preventing this would be if this plugin contains an option to say something like "allow at most one role per user". The problem then would be when a plugin such as bbEdit is active, since bbEdit assigns forum roles to users in addition to their "standard" WordPress role. Other plugins may do something similar.

How would this plugin handle such a scenario? Would CWMS have to be aware of every other multi-role plugin out there? (FWIW, CWMS is compatible with bbEdit and never touches its roles) Additionally, is there actually a problem with users having multiple roles? Surely this is a genuine reflection of the state of their memberships in CiviCRM?

The logical way to avoid this situation is (you guessed it!) to use caps instead of roles. Caps are, of course, removed from users when the membership expires so the conundrum discussed here is never encountered.

What I have found, however, is that there's a sorting problem when the returned memberships contain a lifetime membership. It appears that the inclusion of end_date in the sort option causes lifetime memberships to be returned first and that the subsequent status_id.is_current_member ASC is ineffective. Switching the order of the sort to status_id.is_current_member ASC, end_date helps to the extent that lifetime memberships are returned after expired memberships. Addressed in 83f242c6c1ce5904ec08fbe68f20b0d764c3d404 - also relevant to #22.

[christianwach]

Running Manual sync causes expired records to put members in multiple roles.

Same logic as above applies here too.

[christianwach]

@kcristiano I don't think there's anything I can do to specifically address your "different expiry roles" conundrum directly. What I've done in 8009955d398c4023dd2f416cef9148ff92ecf376 is to introduce an action that you can hook into once everything has been updated. The params allow you to determine when the update is a renewal (since $previous_membership will not be null) and act accordingly if the user has multiple roles. Does this help?

[kcristiano]

Thanks @christianwach I agree you've done all you can to address this. This will definitely help. Thanks

[christianwach]

Thanks @kcristiano - closing for now.