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-21711 When updating multiple memberships thru a contribution page custom fields are only updated on one #11586

Merged
merged 2 commits into from
Mar 21, 2018

Conversation

alifrumin
Copy link
Contributor

@alifrumin alifrumin commented Jan 25, 2018

Overview

If a user is renewing multiple memberships on one contribution page and that contribution page also has a profile with custom fields on the membership the custom fields are only updated on one membership when they should be updated on all.

Before

When you are renewing or creating multiple memberships thru a front facing contribution page custom fields are only updated on one membership.

After

When you are renewing or creating multiple memberships thru a front facing contribution page custom fields are updated on ALL memberships.

Technical Details

moves the updating of custom fields into the foreach

Comments

I intend to write a test for this as a part of a separate pr


@alifrumin
Copy link
Contributor Author

jenkins, test this please

@alifrumin alifrumin changed the title WIP CRM-21711 When updating multiple memberships thru a contribution page custom fields are only updated on one CRM-21711 When updating multiple memberships thru a contribution page custom fields are only updated on one Jan 29, 2018
@eileenmcnaughton
Copy link
Contributor

There are tests for that form submission in CRM_Contribution_Form_ContributeTest - would be good to get a test on it

@alifrumin alifrumin changed the title CRM-21711 When updating multiple memberships thru a contribution page custom fields are only updated on one WIP CRM-21711 When updating multiple memberships thru a contribution page custom fields are only updated on one Feb 21, 2018
@colemanw
Copy link
Member

@alifrumin when you finish writing the test could you please rebase & squash the commits on this PR into one commit? That will also get rid of that pesky merge commit that snuck in. Thanks.

@alifrumin alifrumin force-pushed the CRM-21711 branch 3 times, most recently from 7f3e80f to 49492bb Compare March 16, 2018 20:12
@alifrumin alifrumin changed the title WIP CRM-21711 When updating multiple memberships thru a contribution page custom fields are only updated on one CRM-21711 When updating multiple memberships thru a contribution page custom fields are only updated on one Mar 16, 2018
@alifrumin
Copy link
Contributor Author

Jenkins test this please

@petednz
Copy link

petednz commented Mar 20, 2018

@jitendrapurohit pls review

@eileenmcnaughton
Copy link
Contributor

eileenmcnaughton commented Mar 20, 2018

@alifrumin any chance you could do @jitendrapurohit's one in return? #10496 (it's pretty straight forward - probably only about 20 mins to do)

@alifrumin
Copy link
Contributor Author

Happy to I will look at it now

Copy link
Contributor

@jitendrapurohit jitendrapurohit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested create and renewal of multiple memberships from the contribution page and this change seems to be providing the correct custom value for all the chosen membership on the Main page.

@eileenmcnaughton
Copy link
Contributor

Merging based on @jitendrapurohit's testing. Code looks sensible & I detect a minor clean up plus.... UNIT TEST

@eileenmcnaughton eileenmcnaughton merged commit 9fad918 into civicrm:master Mar 21, 2018
@alifrumin
Copy link
Contributor Author

Thank you

@alifrumin alifrumin deleted the CRM-21711 branch March 22, 2018 17:10
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.

6 participants