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

dev/membership#21 Fix regression causing transfer of membership to another contact if they paid #16047

Merged
merged 1 commit into from
Dec 9, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Dec 8, 2019

Overview

Fixes a bug where a membership associated with a contact is transferred to the contact who paid for it on edit, if that contact is different.

Before

  1. Using default config membership type 'general' is inherited to the head of household - add head of household relationship to an individual
  2. Create a membership for the household but check the record payment from a different contact and choose the related individual

Screen Shot 2019-12-09 at 11 19 42 AM

3. Go to edit the relationship -note the wrong contact name

Screen Shot 2019-12-09 at 11 21 39 AM

Screen Shot 2019-12-09 at 11 21 50 AM

4. Edit the end date & save - membership has been transferred to the contact who paid for he membership

After

Contact remains attached to the correct member

Technical Details

This is a regression from work @yashodha did in ac0ef1d to improve form consistency.

Reverting that change fixes this. However, I don't propose we do that because it's a good change and the lines I propose to remove follow a known bad pattern - merging a bunch of parameters into a bunch of parameters with no clarity about what the reason is or why. In fact most of the remove lines exist to reverse the changes made in
CRM_Contribute_BAO_Contribution::getValues($contributionParams, $defaults, $contributionIds);

I couldn't see any negative impact on the renew form & the fields being defaulted didn't even show up on edit

Comments

The gitlab also refers to editing by webform - not sure what that is about

@civibot
Copy link

civibot bot commented Dec 8, 2019

(Standard links)

@civibot civibot bot added the master label Dec 8, 2019
@eileenmcnaughton eileenmcnaughton changed the base branch from master to 5.21 December 8, 2019 22:29
@civibot civibot bot added 5.21 and removed master labels Dec 8, 2019
@seamuslee001
Copy link
Contributor

This broadly makes sense to me, I'll try and give it some r-run this afternoon

@eileenmcnaughton eileenmcnaughton changed the title dev/membership#21 Fix regression on transferring member to another contact if they paid dev/membership#21 Fix regression causing transfer of membership to another contact if they paid Dec 9, 2019
@seamuslee001
Copy link
Contributor

(CiviCRM Review Template WORD-1.2)

  • General standards
    • (r-explain) Pass
    • (r-user) Pass
    • (r-doc) Pass
    • (r-run) Pass i replicated the issue and confirmed this fixed it, i also tested renewing the membership on the back end and confirmed that worked fine as well
  • Developer standards

@seamuslee001 seamuslee001 merged commit a1552cd into civicrm:5.21 Dec 9, 2019
@seamuslee001 seamuslee001 deleted the 521 branch December 9, 2019 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants