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/core#2273) Contact type incorrectly set to Contribution due to '… #19277

Merged
merged 2 commits into from
Jan 3, 2021

Conversation

yashodha
Copy link
Contributor

…Honoree Profile'

Overview

Steps to replicate:

  • Create a contribution page with Honoree Section Enabled.

  • Choose a profile that includes a contribution field.

  • Make a contribution with this page with soft credit contact.

Screenshot from 2020-12-23 19:01:36

  • Check contact type of the newly created soft credit contact in DB. It will be Contribution which is why it will not show up in some places and throws errors on various screens.

Screenshot from 2020-12-23 19:01:14

Screenshot from 2020-12-23 19:01:05

I was able to replicate the scenario on dmaster. We should restrict the profile type to Contact types when calculating for honoree profile.

Before

before

After

after

@civibot civibot bot added the master label Dec 24, 2020
@civibot
Copy link

civibot bot commented Dec 24, 2020

(Standard links)

@eileenmcnaughton
Copy link
Contributor

Stacktrace
api_v3_SyntaxConformanceTest::testCreateSingleValueAlter with data set #82 ('UFField')
api result array comparison failed checking if field_name was correctly updated
Array
(
[update-params] => Array
(
[id] => 110
[field_name] => formatting
)

[update-result] => Array

@eileenmcnaughton
Copy link
Contributor

test this please

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 this ^^ was the fail https://github.com/civicrm/civicrm-core/compare/master...eileenmcnaughton:falsefail?expand=1 addresses (I had a PR open against it a couple of months back but you weren't sold at the time so I closed it)

@yashodha
Copy link
Contributor Author

@eileenmcnaughton I am surprised that more people haven't reported this.

@eileenmcnaughton
Copy link
Contributor

@yashodha I took a look at this & I couldn't actually replicate it with the instructions - ie it would not let me add a profile with contribution fields as the profile. I was able to add a profile that didn't have contribution fields and then go and edit it to add a field - but when I went back to the edit contribution page it was not visible - because it was not valid. It does load if you edit things in the right order but it seems like a misconfiguration

All of that suggests that we should prevent people from misconfiguring contribution fields onto honoree profiles rather than make it work. It's a bit unclear how it would make sense to have contribution fields on honoree profiles but perhaps it works just due to code randomness (which would suggest it could break during other changes)

@yashodha
Copy link
Contributor Author

yashodha commented Jan 1, 2021

@eileenmcnaughton

I was able to replicate this again on dmaster with default honoree profile and with added contribution source field (which I disabled). Even then (after disabling) the newly formed contact has type set to Contribution and this is going to affect all the honoree profiles that look like this
honoree

Even if we prevent from the misconfiguration there still will be profiles that pre-date that rule which will mess up the data.

@eileenmcnaughton
Copy link
Contributor

@yashodha I'm not convinced the contribution fields should be supported in this case - but the patch makes sense in that the parameter ensures that the only value that could be returned is a valid contact type so I'm going to merge it on that narrow criteria rather than worrying about the whole scenario. (in other words merging this doesn't imply contribution fields are supported in honoree profiles)

@eileenmcnaughton eileenmcnaughton merged commit 8150c6c into civicrm:master Jan 3, 2021
@yashodha
Copy link
Contributor Author

yashodha commented Jan 4, 2021

@eileenmcnaughton yes my point exactly, that contribution fields need not be supported but if by chance someone has added/removed such field from honoree profile don't return Contribution type for contact.

Thanks for merging!

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