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

Inherited organisation membership is not transferred to the individual contact if one of two relationship types defined in the membership type is disabled #14410

Closed
wants to merge 1 commit into from
Closed

Inherited organisation membership is not transferred to the individual contact if one of two relationship types defined in the membership type is disabled #14410

wants to merge 1 commit into from

Conversation

agilewarealok
Copy link
Contributor

Overview: Steps to reproduce

  1. Membership Type A has option to inherit membership for Relationship A and Relationship B
  2. Organisation A has Membership Type A, with status Current
  3. Individual A has Relationship A and Relationship B to Organisation A
  4. Individual A inherited membership has a status of Current
  5. Individual A, Relationship A to Organisation A is Disabled
  6. Individual A, Relationship B to Organisation A is Disabled
  7. Individual A inherited membership is automatically removed
  8. Individual A, Relationship A to Organisation A is Enabled
  9. Individual A inherited membership is not re-created

Individual A should have inherited membership re-created if one Relationship is enabled with Organisation A

Before

Membership not re-created if one relationship is not enabled.

After

Membership is re-created after one of the relationships is enabled.

Comments

Agileware Ref: CIVICRM-1063

@civibot
Copy link

civibot bot commented Jun 3, 2019

(Standard links)

@civibot civibot bot added the master label Jun 3, 2019
@seamuslee001
Copy link
Contributor

@agh1 i know you have been dabbling in membership stuff recently do you have any feelings on this PR?

@eileenmcnaughton
Copy link
Contributor

@agilewarealok this one has style issues

@agh1
Copy link
Contributor

agh1 commented Jun 14, 2019

I don't have time to review this now (got less PR review work done in Brooklyn than I hoped), but based purely on the description, it sounds like a bug. For someone to test this out, I'd expect they'd want to:

  • create a new membership for already-related contacts and confirm it inherits
  • create a new relationship to a contact that's already a member and confirm the membership inherits
  • delete the master membership for already-related contacts and confirm it makes the inherited one disappear
  • have a contact with two relationships to separate contacts with memberships that should inherit along those relationships. delete one of the master memberships, and ensure that the contact still inherits a membership from the other.
  • have a contact with two relationships to separate contacts with memberships that should inherit along those relationships. disable one of the relationships, and ensure that the contact still inherits a membership from the other.
  • based on the last, disable the other relationship, and ensure that the contact no longer inherits any membership.
  • based on the last, re-enable either relationship, and ensure that the membership inherits again.
  • have a contact that inherits a membership along two relationships, disable one, and ensure that the membership still inherits.
  • based on the last, disable the other relationship, and ensure the membership disappears
  • based on the last, re-enable either relationship, and ensure the membership reappears

This was referenced Aug 2, 2019
Merged
Merged
@jusfreeman
Copy link
Contributor

@eileenmcnaughton just noting that this PR is not "stale" - just needs some pretty minor work to get rid of the conflicts.

@eileenmcnaughton
Copy link
Contributor

@jusfreeman you are welcome to rebase it & re-open it - that can also have the effect of bringing it onto the front page - which can be good

@agileware-pengyi agileware-pengyi force-pushed the CIVICRM-1063 branch 2 times, most recently from 568bfbd to f016480 Compare August 9, 2019 03:47
@eileenmcnaughton eileenmcnaughton mentioned this pull request Aug 17, 2019
Merged
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 17, 2019
@eileenmcnaughton
Fix inherited relationship being deleted when there is still a valid …
1cfe495 
…relationship

This pulls out some of the good work & test written in civicrm#14410, but treating the parts as separate bugs
with separate cleanup requirements.

I put up cleanup in civicrm#15061

And this addresses the need for the relationship to not be deleted if a valid one still exists
- wrangled out into a separate function
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 17, 2019
@eileenmcnaughton
Fix inherited relationship being deleted when there is still a valid …
85b4689 
…relationship

This pulls out some of the good work & test written in civicrm#14410, but treating the parts as separate bugs
with separate cleanup requirements.

I put up cleanup in civicrm#15061

And this addresses the need for the relationship to not be deleted if a valid one still exists
- wrangled out into a separate function
@eileenmcnaughton eileenmcnaughton mentioned this pull request Aug 17, 2019
Merged
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 18, 2019
@eileenmcnaughton
Fix inherited relationship being deleted when there is still a valid …
aa67cc8 
…relationship

This pulls out some of the good work & test written in civicrm#14410, but treating the parts as separate bugs
with separate cleanup requirements.

I put up cleanup in civicrm#15061

And this addresses the need for the relationship to not be deleted if a valid one still exists
- wrangled out into a separate function
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 21, 2019
@eileenmcnaughton
Fix inherited relationship being deleted when there is still a valid …
f13921a 
…relationship

This pulls out some of the good work & test written in civicrm#14410, but treating the parts as separate bugs
with separate cleanup requirements.

I put up cleanup in civicrm#15061

And this addresses the need for the relationship to not be deleted if a valid one still exists
- wrangled out into a separate function
@eileenmcnaughton eileenmcnaughton mentioned this pull request Aug 23, 2019
Merged
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 29, 2019
@eileenmcnaughton
Fix inherited relationship being deleted when there is still a valid …
e4dd687 
…relationship

This pulls out some of the good work & test written in civicrm#14410, but treating the parts as separate bugs
with separate cleanup requirements.

I put up cleanup in civicrm#15061

And this addresses the need for the relationship to not be deleted if a valid one still exists
- wrangled out into a separate function
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Oct 14, 2019
@eileenmcnaughton
Fix inherited relationship being deleted when there is still a valid …
f02adca 
…relationship

This pulls out some of the good work & test written in civicrm#14410, but treating the parts as separate bugs
with separate cleanup requirements.

I put up cleanup in civicrm#15061

And this addresses the need for the relationship to not be deleted if a valid one still exists
- wrangled out into a separate function
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Oct 25, 2019
@eileenmcnaughton
Fix inherited relationship being deleted when there is still a valid …
5e2be4e 
…relationship

This pulls out some of the good work & test written in civicrm#14410, but treating the parts as separate bugs
with separate cleanup requirements.

I put up cleanup in civicrm#15061

And this addresses the need for the relationship to not be deleted if a valid one still exists
- wrangled out into a separate function
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Oct 26, 2019
@eileenmcnaughton
Fix inherited relationship being deleted when there is still a valid …
8189f8c 
…relationship

This pulls out some of the good work & test written in civicrm#14410, but treating the parts as separate bugs
with separate cleanup requirements.

I put up cleanup in civicrm#15061

And this addresses the need for the relationship to not be deleted if a valid one still exists
- wrangled out into a separate function
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Oct 27, 2019
@eileenmcnaughton
Fix inherited relationship being deleted when there is still a valid …
2b3d6c4 
…relationship

This pulls out some of the good work & test written in civicrm#14410, but treating the parts as separate bugs
with separate cleanup requirements.

I put up cleanup in civicrm#15061

And this addresses the need for the relationship to not be deleted if a valid one still exists
- wrangled out into a separate function
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Oct 28, 2019
@eileenmcnaughton
Fix inherited relationship being deleted when there is still a valid …
310c203 
…relationship

This pulls out some of the good work & test written in civicrm#14410, but treating the parts as separate bugs
with separate cleanup requirements.

I put up cleanup in civicrm#15061

And this addresses the need for the relationship to not be deleted if a valid one still exists
- wrangled out into a separate function
@agileware-pengyi
Copy link
Contributor

@eileenmcnaughton I think this one can be closed then?

@eileenmcnaughton
Copy link
Contributor

@agileware-pengyi OK - I'll close it -but I don't think all the stuff in it has been brought over yet. Still we can experiment with uncommenting more lines of the test Agileware wrote once the other is merged

@agileware-pengyi
Copy link
Contributor

@eileenmcnaughton I think #15731 is the last one?

@eileenmcnaughton
Copy link
Contributor

@agileware-pengyi I think so - that unit test was quite a good 'shopping list' of what should work. The code was just so incomprehensible though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
has-test master
Projects
None yet
Milestone
No milestone
6 participants
@agilewarealok @seamuslee001 @eileenmcnaughton @agh1 @jusfreeman @agileware-pengyi