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.

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

Removing related memberships if parent membership type is changed which does not have relation type associated. #12180

Merged
merged 1 commit into from
May 24, 2018

Conversation

agilewarealok
Copy link
Contributor

Overview

Inherited membership status is retained after the membership type is changed for the Primary member from a membership type with inherit by Relationship Type enabled to a membership type with no inherit by Relationship Type enabled.

Steps to reproduce:

  1. Add membership to organisation which inherits based on relationship "Employee of"
  2. Set the membership status to current
  3. Add individual contacts and add relationship "Employee of" to the organisation
  4. Note that the membership status is inherited from the organisation as the primary member
  5. Change the membership type for the organisation to a new membership type which does not have inherit membership based on relationship enabled
  6. Note that the individual contacts still have an active membership using the new membership type
  7. Expected result is that the individuals do not have a membership after the membership type has been changed.

Before

Individual member still have membership event after type of parent membership is changed which does not have relation type associated.

After

Membership of Individual member is removed.

Comments

Agileware Ref: CIVICRM-832

@eileenmcnaughton
Copy link
Contributor

test this please

@eileenmcnaughton
Copy link
Contributor

@agilewarealok did you look at whether you could do a unit test on this?

…s changed which does not have relation type associated.
@agilewarealok
Copy link
Contributor Author

@eileenmcnaughton Sure we've added Unit test on this. Added for PR #12179 too.

@eileenmcnaughton
Copy link
Contributor

I tested this & it works. The issue appears to be caching in CRM_Core_DAO::getFieldValue (ug)

@eileenmcnaughton eileenmcnaughton merged commit f3ebf7b into civicrm:master May 24, 2018
@agh1
Copy link
Contributor

agh1 commented Jul 3, 2018

So I'm super Johnny-come-lately on this, but it seems that deleting the inherited memberships is a rather drastic thing to do, and it needlessly causes data loss.

As a user, I'd expect that changing an inherited membership type to a non-inherited membership type would cause the inherited members to "lose" their memberships, but by "lose" I expect their membership would be left to expire, switched immediately to the Canceled status, or something like that.

A concrete example: a university professor is a member of the Omphaloskeptical Society through the Department of Anatomy's membership. The department goes through budget cuts, and they no longer cover faculty memberships, but they keep the subscription to Navel News for the library. The professor joins on her own because this is her area of academic focus.

She (and the Society's staff) would expect that her membership would show her join date as many years ago and that her chapter and section membership (custom fields on the membership record) would carry over--she has been a member continuously. Instead, under this change, she'd start getting the welcome email series as if she came in cold, and there would be no record of her ever being a member.

Anyway, I don't know what can be done at this point, and I realize that this doesn't touch the part of the code that does deletion, but I think this situation is worth highlighting, both as this being a potentially significant change, and as people typically understanding the idea of being "not a member" as lacking an active membership rather than lacking any membership ever.

@eileenmcnaughton
Copy link
Contributor

So my take on this is this patch fixed something to how it had been intended to work - but was mis-written - and I think some other paths would possibly have already caused this to happen?

I agree with you it would make more sense to cancel - @jusfreeman any appetite for a follow up on this (also ping @mattwire )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants