-
-
Notifications
You must be signed in to change notification settings - Fork 825
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#5071 Use first pricefield ID when inferring membership type #29653
Conversation
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
The issue associated with the Pull Request can be viewed at https://lab.civicrm.org/dev/core/-/issues/5071 |
@agileware-fj on latest master it looks like |
I changed the branch to 5.72 as we have very recent changes in this code (including the change that made that variable unused) so I think the rc is more appropriate than master |
a0ad016
to
8918b96
Compare
8918b96
to
85b33a3
Compare
@eileenmcnaughton That's the line that I was suggesting would make sense to remove because I'm not sure it even did anything useful before. Resubmitted as just its removal against 5.72. |
thanks @agileware-fj - I'm thinking we should include in 5.71.1? |
Use first pricefield ID when inferring membership type ID from checkboxes
Overview
Prevents a crash on Contribution pages for membership sign up when you try to add a checkboxes price fields and assign a default that doesn't link to a membership type
See dev/core#5071
Before
TypeError fatal when loading contribution pages as described, as it's trying to use an array to as an ID.
After
Uses the first key of the array as the relevant ID to load a membership type from the database
Comments
This code actually feels like a redundant way to confirm that the PriceFieldValue doesn't link to a membership type ID anyway, so a better solution might be to just remove the line causing the crash entirely.