Stop submitting registration_form_id as an unpermitted event param#1542
Stop submitting registration_form_id as an unpermitted event param#1542maebeale wants to merge 1 commit into
Conversation
The registration form selector is not an Event attribute — it is persisted through the event_forms join in assign_event_forms. Nesting the field under event[registration_form_id] meant strong params rejected it as unpermitted and logged a warning on every event create/update. Move the field out of the event[...] namespace to a top-level registration_form_id param so the value reaches assign_event_forms without tripping the unpermitted-parameter filter. Closes #1536 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
|
||
| def assign_event_forms(event) | ||
| form_id = params.dig(:event, :registration_form_id) | ||
| form_id = params[:registration_form_id] |
There was a problem hiding this comment.
Reading the top-level params[:registration_form_id] instead of params.dig(:event, :registration_form_id). This value isn't an Event column — it's resolved into the event_forms join below — so keeping it out of the event[...] namespace is what stops strong params from logging it as unpermitted.
| @registration_forms.find { |rf| rf.name == ShortEventRegistrationFormBuilder::FORM_NAME }&.id | ||
| end %> | ||
| <select name="event[registration_form_id]" class="w-full rounded border-gray-300 shadow-sm px-3 py-2 text-sm"> | ||
| <select name="registration_form_id" class="w-full rounded border-gray-300 shadow-sm px-3 py-2 text-sm"> |
There was a problem hiding this comment.
Field renamed from event[registration_form_id] to registration_form_id. The selector drives assign_event_forms, not mass assignment, so it belongs at the top level of the params rather than nested under the event resource.
There was a problem hiding this comment.
Pull request overview
This PR removes registration_form_id from the nested event[...] params namespace (where it is not a real Event attribute) and instead submits it as a top-level parameter, matching how registration forms are actually persisted via the event_forms join.
Changes:
- Update the event form to submit
registration_form_idas a top-level param (notevent[registration_form_id]). - Update
EventsController#assign_event_formsto readparams[:registration_form_id]. - Add request specs covering create/update/blank submission, including an assertion that the param is not reported as unpermitted.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
app/views/events/_form.html.erb |
Moves the registration form selector out of event[...] to avoid strong-params “unpermitted” noise. |
app/controllers/events_controller.rb |
Reads registration_form_id from the top-level params in assign_event_forms. |
spec/requests/events_spec.rb |
Adds request coverage for linking/unlinking the registration form and checking for unpermitted-param notifications. |
| subscriber = ActiveSupport::Notifications.subscribe("unpermitted_parameters.action_controller") do |*args| | ||
| captured.concat(args.last[:keys]) | ||
| end |
Closes #1536
What is the goal of this PR and why is this important?
event[registration_form_id], butregistration_form_idis not an Event attribute.event_formsjoin (seeEventsController#assign_event_forms), not via mass assignment.event[...], strong params rejected it as an unpermitted parameter and logged a warning on every event create/update — which is exactly what the issue flagged ("Why is this submitted and do we need it?").We do need the value (it drives which registration form gets linked) — it just shouldn't live inside the
event[...]namespace.How did you approach the change?
event[registration_form_id]to a top-levelregistration_form_idinapp/views/events/_form.html.erb.assign_event_formsto readparams[:registration_form_id]instead ofparams.dig(:event, :registration_form_id).Anything else to add?
registration_form_idis no longer reported via theunpermitted_parameters.action_controllernotification.GET /index,/new,/show) currently return 500 in this local environment even on a cleanmaincheckout (asset/CSP rendering), so they are not affected by this change.🤖 Generated with Claude Code