-
-
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
Clean up code to add custom data to forms, implement on back office participant form #28733
Conversation
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
error directly relates (which is helpful) |
15808eb
to
5df26c7
Compare
5df26c7
to
a23becc
Compare
a23becc
to
cfff80b
Compare
We should also merge #28735 & then hopefully not touch that form again for a while |
It would be really great to bear in mind that the "for contacts only" rule of multi-value custom groups is being lifted. In theory now any entity can have multi-record custom data attached to it... the only obstacle is the quickforms. So if this new refactor is flexible enough to handle those for any entity, that would be a game-changer. |
Co-authored-by: colemanw <[email protected]>
@colemanw I'm not sure it gets us there with the multiple fields - that's probably out of scope here - although cleaner code is easier to fix up... |
Ok I ran this by making various sets of participant custom fields & registering someone on the backoffice form. All fields showed up and saved correctly. |
thanks @colemanw |
Overview
The existing function is crazy complicated & interacts with lots of undefined properties, per tests - but on digging the actual code used by non-contact, non multiple record form is actually quite simple - this adds a simple function that does the things that actually need to be done for processing custom data & implements it on the participant form
Before
Incredibly confusing code with lots of notices & undefined properties, causing php 8.x tests to fail
After
I managed to figure out what the end result of all that building group trees & passing group trees around & reformatting them and the part that is needed for forms that are not doing the actual rendering is https://github.com/civicrm/civicrm-core/pull/28733/files#diff-82b0e2eac7cfc3882a5e3cf5db3be5f7a8b42257f64366e8db74f2ddafb1f18cR45-R91
Which compares favourably to
civicrm-core/CRM/Custom/Form/CustomData.php
Lines 82 to 199 in 7d4bb5e
I suspect there might be a better way to load the custom data entity IDs that @colemanw will suggest - although I would prefer to do that as a follow up at this point (since it has been a slog & what is there is not that bad)
Technical Details
I identified that required fields are enforced by ajax in popup mode but nothing enforces them in non popup mode - that was pre-existing.
I tested with money, number, text, radio and file fields and with custom groups extending role, event id, and event type on create new and on edit
Comments