CRM-21325 - Cannot add membership fields to on behalf profile

[yashodha]

Overview

Cannot add membership fields to on behalf profile
The code says that it needs to make special expection for allowing membership + organization fields for on behalf feature but does NOT handle properly. Also check for behalf profile is flawed.

Before

After

---

• CRM-21325: Cannot add membership fields to on behalf profile

[yashodha]

@monishdeb can you please look at this?

[monishdeb]

@yashodha I did some optimisation on top of the patch - https://gist.github.com/monishdeb/357c1f19a0d580c4d34ae20d3ccd5266

Can you please include that and check if it works for ya? (Tested on my local, works for me)

[yashodha]

@monishdeb I didn't pass the flag and let it evaluate later because it might be easier if we wanted to do more checks later rather than passing more flags.
Were there any more concerns apart from optimization?

[eileenmcnaughton]

Just looking to see the status of this as it has been stalled for quite a while. I see there was feedback in Oct last year within a week of it being put up for review & then there was a 5 week+ delay in response by which time I guess @monishdeb was no longer engaged.

If I were to review this I would insist on a unit test & that would probably be a lot easier without passing $self (I also think that passing forms around is bad practice because it encourages people to tack things onto the function -this seems like a clear cut & narrowly defined function we don't really want to be opening up options for people to overload it more easily). So I'm definitely in favour of Monish's parameter change suggestion

@yashodha is this still a PR you want to put more work into upstreaming? You might want to trade some review with @monishdeb if you are still wanting to get it re-reviewed.

[eileenmcnaughton]

Per comment on #11684 have pinged others to try to get input as I have doubts about the unintended consequences of this change

[eileenmcnaughton]

I'm going to close this for now since it's not active and I think it needs concept approval (in gitlab) before it would be ready for code review.

This change removes a check / limit deliberately placed in that form on the assumption that it would not work properly. We would need to build a case for why we can be sure that we are not setting ourselves up for 'bug-reports' down the track when people enable these PRs and then hit problems because the combination of profiles don't work in some obscure scenario.

It would be pretty easy to use an extension to change this rule on this form and then we could see how that works for a while (& what the adoption is) ?