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-21321 - Membership fields not loading in 'On behalf of' profile #11148

Merged
merged 1 commit into from
Oct 24, 2017

Conversation

yashodha
Copy link
Contributor

@yashodha yashodha commented Oct 17, 2017

Overview

Membership fields not loading in 'On behalf of' profile

Before

behalf_before

After

membership_behalf


@yashodha
Copy link
Contributor Author

@monishdeb can you please look at this? I think this is an oversight where you can't add the membership fields on contribution page when behalf profile is activated.

@jusfreeman
Copy link
Contributor

@yashodha great addition - thanks for your work 👍

@monishdeb
Copy link
Member

monishdeb commented Oct 18, 2017

@yashodha @eileenmcnaughton there is a validation written about not to allow membership profile IF onbehalf profile is set, it rather suggests us to move the membership fields to onBehalfOf profile. I don't see what was the need for this as removing those two validations allows me to add membership profile and upon submission, it correctly records custom field data. So instead of amending support for Membership type fields here in buildComponentForm(...) can we remove those two validation rules? i.e. https://gist.github.com/monishdeb/02ce35549222b84c2e6b304362549b08

@eileenmcnaughton
Copy link
Contributor

I assume the issue arises if fields get added to the profile that are about the membership itself - not just custom data fields?

@monishdeb
Copy link
Member

yes, issue is membership fields w/o custom fields are not shown on onBehalf profile.

Actually due to the formrule in place, if the contribution page is already configured with onbehalf profile then you can not add membership Profile (including custom fields), it rather instruct you to add the membership fields (w/o custom field) in onbehalf profile. Which looks a bit odd to me. So I was suggesting why not remove the validation in first place.

@eileenmcnaughton
Copy link
Contributor

So removing this doesn't allow 'membership_status_id' or anything functional to be added?

@monishdeb
Copy link
Member

No it allows, see :
screen shot 2017-10-19 at 10 25 32 am

@eileenmcnaughton
Copy link
Contributor

So I think that is why the block is there - it was considered too tricky to filter appropriate fields so it was just blocked

@monishdeb
Copy link
Member

Yeah but then core doesn't respect this block, see #11151 where it doesn't allow membership fields to be added to onBehalf profile. Strange. So should I merge this PR then?

@eileenmcnaughton

@yashodha
Copy link
Contributor Author

@monishdeb yes #11151 is a precursor to this. The underlying documentation also suggests that the exception be made while adding membership fields to an 'on behalf' profile but is not working cause the logic doesn't handle the conditions well.

@monishdeb
Copy link
Member

Agree, tested the patch and works fine. Merging now.

@monishdeb monishdeb merged commit 4383dc9 into civicrm:master Oct 24, 2017
@eileenmcnaughton
Copy link
Contributor

So to clarify - is it now possible to add inappropriate fields (membership type/ membership status) to an on behalf profile and have them show up on a public page?

@monishdeb
Copy link
Member

monishdeb commented Oct 26, 2017

@eileenmcnaughton still no as you need to apply/merge #11151 first, which will allows you to add membership fields to on-behalf profile. This patch will then allows you to load n show membership fields on on-behalf profile and yes it will show the inappropriate fields on public page too based on what 'Visibility' chosen. Isn't this appropriate as per the formrule written?

sluc23 pushed a commit to ixiam/civicrm-core that referenced this pull request Jan 10, 2018
CRM-21321 - Membership fields not loading in 'On behalf of' profile
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.

5 participants