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

CRM-21164 membership inheritance co-opted #10960

Closed
wants to merge 1 commit into from

Conversation

lcdservices
Copy link
Contributor

@lcdservices lcdservices commented Sep 10, 2017

Overview

Check if existing membership and renewing membership have the same owner before renewing.

Before

An existing inherited membership could be co-opted by a new membership that does not support inheritance.

After

A new membership will be created if this condition is found.

Comments

It may be worth discussing if this check should be broader in scope. It currently is geared toward a specific use case.


@eileenmcnaughton
Copy link
Contributor

@lcdservices - this pretty much needs a test to be mergeable which is likely what stalled it. I think @mattwire or @agh1 would be good candidates to advise whether the change should be accepted from a 'is this a good change to make POV'.

If you don't want to work on a test in the near future I think it makes sense to close & re-open when there is one

@agh1
Copy link
Contributor

agh1 commented Mar 27, 2018

@lcdservices this sounds like a great check. This is not an uncommon problem for one organization we work with: a law firm or law school subscribes to a journal, and employees inherit the membership so they get online benefits, but often an individual will want their own copy to have in their office (instead of it being sent to the library). They might subscribe online, but that subscription should not renew the organization's membership.

One scenario that would be important to make sure works smoothly: what happens a year later when the individual member attempts to renew? I'd want to make sure the process goes through all of the memberships that the individual might have.

Right now, it looks like CRM_Member_BAO_Membership::getContactMembership() just returns the most recently started membership that is neither cancelled, pending, nor a test. If that one has the wrong owner, there's no opportunity to go back and find one that has the right owner. I think the result would be a new membership each time that person renews.

@lcdservices
Copy link
Contributor Author

@eileenmcnaughton leave this open -- I'll work on a test and on addressing @agh1's additional observations.

@eileenmcnaughton
Copy link
Contributor

Thanks both

@petednz
Copy link

petednz commented Apr 24, 2018

seems to crossover with this new issue - https://lab.civicrm.org/dev/membership/issues/3#note_4034

@joannechester
Copy link
Contributor

There has been some discussion about advisibility of changing long established membership functionalilty at https://lab.civicrm.org/dev/membership/issues/3. You are considering one specific use case of memberships inherited by employees of a company. What about 'memberships' inherited by family members, perhaps the current functionalilty works for them. Should we be telling people that memberships that can be inherited should be owned by a different 'organisation' if they don''t want the upsell feature to work.

@alanpuccinelli
Copy link

It seems to me if something is truly an "Upsell" we need to at least make sure the individual has permissions to act on the behalf of the parent organization. In which case wouldn't this be an upsell for everyone that's inherited the membership?

I would really like to see this pulled as is meshes with our use case well, we're not trying to upsell, we're trying to create a new membership. If it potentially breaks the upsell feature then I would argue the upsell feature implementation is incomplete.

@eileenmcnaughton
Copy link
Contributor

This is stalled because it needs a test - it has been stalled for a while so I think it's time to close it & you can re-open if / when you write a test. The code /discussion are not lost as they are linked from the JIRA issue

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