DTAB-591: Stop edit-participant prefill leaking sibling participations#92
Open
hitesh-jain wants to merge 3 commits into
Open
DTAB-591: Stop edit-participant prefill leaking sibling participations#92hitesh-jain wants to merge 3 commits into
hitesh-jain wants to merge 3 commits into
Conversation
When allow_url_load is on and the URL pins specific events (event{n} or
c{c}event{n}), narrow loadParticipants() to those events instead of
querying across every type-matching event in CiviCRM.
Without this, a sibling participation of the same contact in another
event whose status is in the form's exposed status dropdown overwrites
the form defaults, so edit-participant flows show the wrong participant's
status and custom data and can silently corrupt records on save.
Falls back to the original wide query when the intersection is empty.
No behaviour change when no URL event pin is present.
There was a problem hiding this comment.
Code Review
This pull request modifies the participant loading process to narrow prefill lookups based on event IDs provided in the URL when the 'allow_url_load' setting is active. This prevents unrelated event data from overwriting form defaults. A new helper method, getUrlPinnedEventIds, was added to parse these IDs. Feedback from the reviewer suggests refactoring this new method to consolidate duplicated logic used for parsing different URL parameter formats, which would improve code maintainability.
Address review feedback (PR #92): the two regex branches in getUrlPinnedEventIds were each running the same explode/foreach to collect ids. Merge them into a single branch gated by either match.
viraj-compuco
approved these changes
May 22, 2026
Address review feedback (PR #92): $_GET parameters can come in as arrays (e.g. ?event1[]=123), and casting an array to string in PHP 8+ emits a deprecation warning. Check for array first and skip the explode in that case.
viraj-compuco
approved these changes
May 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
A webform set up to let an admin open and edit someone's existing event registration could quietly pre-fill the form with data from a different event the same person was registered for, not the one the admin actually opened. Saving the form would then overwrite the correct record with the wrong values — silently, with no warning that anything was off.
Bug flow
Before — wide query lets a sibling participation overwrite the prefill:
After — query narrows to the URL-pinned event:
Before
For a contact with participations in multiple events of the same
event_type_id:participant_status_iddropdown — typically a restricted set like Attended / Cancelled / Trainer Paid) with a sibling participation's status (e.g. "Attended") instead of the URL-pinned participant's actual status.After
event{n}/c{c}event{n}— explained below under Triggering URLs).Technical Details
Affected webform_civicrm configuration (click to expand)
The bug only triggers when all of the following are true on a given webform:
reg_options.allow_url_load = TRUE— webform_civicrm setting on the Settings tab labelled "Allow URL Loading". It opts the form into readingcid/event{n}/c{c}event{n}from the URL at render time to prefill existing CiviCRM data. Forms without this setting never read URL event pins, so the wide query never runs in a way that matters.participant_event_idcomponent, rendered as a hidden field with no static items. This is the webform component that holds the event id for a given participant slot. When it'stype=hiddenwith noextra.items, webform_civicrm resolves its option list live at render time viawf_crm_get_events($reg_options)— which returns every active event whoseevent_type_idmatches the form'sreg_options.event_typefilter.participant_status_idexposed with a restricted set of options (e.g. only Attended / Cancelled / Trainer Paid).participant_status_idis the CiviCRM enum for participant statuses (Registered, Attended, Cancelled, No-show, etc). The status filter insideloadParticipants()keys off the form's exposed options — that's what allows a sibling participation with one of those statuses to overwrite the prefill. Forms exposing every status, or no status field at all, behave differently and weren't observed leaking.number_of_participant >= 1(webform_civicrm data setting that says "contact N has at least one participant slot on the form").Triggering URLs
The URL convention webform_civicrm reads depends on the form's
participant_reg_typesetting, which decides how multiple contacts on the same form relate to events:participant_reg_type = 'all'— every contact on the form registers for the same set of events. The URL pin for the event applies to everyone, so it's a single param per participant slot:event{n}.participant_reg_type = 'separate'— each contact has their own independent event(s). The URL pin needs to address a specific contact's participant slot, so the param includes the contact index:c{c}event{n}.Legacy aliases also supported:
?cid=<contact_id>— treated ascid1for backward compatibility.?event1=<event_id>— now also honored for contact 1 onparticipant_reg_type = separateforms with this patch (admin URLs that setevent1regardless of reg type narrow the prefill correctly).Why this matters
Forms that expose a hidden
participant_event_idfield resolve its options viawf_crm_get_events($reg_options). That function returns every active event of the configured type, so$this->events(the internal map of events the form knows about) ends up holding the whole event catalogue, not just the events the form actually cares about for prefill.Root cause
wf_crm_webform_preprocess::loadParticipants()(the routine that prefills existing participant data when a form opens for a known contact) queriesParticipant.getwithevent_id IN array_keys($this->events). With the wide events list, that returns every participation the contact has across all those events, not just the one pinned in the URL. The per-row status filter inside the function then accepts any record whosestatus_idis in the form's exposed dropdown options, so the last-iterated Attended / Cancelled / Trainer-Paid sibling overwrites the prefill. End result: the form shows that sibling's data, not the participant the URL pinned.Worked example
Contact A has:
Registered, no custom data.Attended, with participant custom values populated (e.g. H&S training selections).Admin opens an edit-participant webform with URL
?cid1=A&event1=E1.The fix
getUrlPinnedEventIds($c)that scans$_GETfor both URL conventions used byloadURLEvents():event{n}(used byparticipant_reg_type = 'all') andc{c}event{n}(used by'separate'). For contact 1 it also accepts the'all'convention even onseparateforms, so admin URLs that setevent1regardless of reg type still narrow the prefill.loadParticipants($c), whenreg_options.allow_url_loadis on andgetUrlPinnedEventIds()returns ids, narrow the participant query to the intersection of$this->eventsand the URL-pinned ids. Falls back to the original wide query when the intersection is empty so a stray URL pin cannot silently zero out the prefill.Participant.getquery narrows from "every type-matching event in CiviCRM" to a single id. Materially cheaper on sites with many events of the configured type, but unchanged when no URL pin is present.Verification
Test data and scenarios
Setup: Contact A registered for two events E1 and E2 of the same event type. Form's status dropdown exposes a restricted set (e.g. Attended / Cancelled / Trainer Paid). Webform configured with
allow_url_load = TRUE,participant_reg_type = separate, one contact + one participant.?cid1=A&event1=E1?cid1=A&event1=E1?cid1=A&event1=E1event{n}/c{c}event{n}in URLgetUrlPinnedEventIds()returns empty, short-circuit ✓event1onparticipant_reg_type = separateformevent1instead ofc1event1on a separate-reg form?cid1=A&event1=E1loadURLEvents()ignoresevent1; prefill stays wide; leak still occursgetUrlPinnedEventIds()acceptsevent1for contact 1 regardless of reg type; narrows correctly ✓Comments
Tip
The fix is conservative: gated behind
allow_url_loadAND the presence ofevent{n}/c{c}event{n}URL params. Forms without URL pins behave exactly as before.getUrlPinnedEventIds()also resolves a latent inconsistency where admin URLs usingevent1on aparticipant_reg_type = separateform were not honored byloadURLEvents()either.loadParticipants()is unchanged — participants whosestatus_idisn't in the form's exposed dropdown still get skipped. This patch only narrows which participants are considered, not how each is then filtered.prepareForm()(resumed drafts, multi-page submissions, edit-op with an existing#submission): all three short-circuit beforeloadParticipants()is called, so the patch has no effect on them.8.x-5.x) will be raised as a follow-up. The bug shape is likely the same but requires reproduction + a regression test in the D8 test suite before opening upstream / fork PRs.