Conversation
… order parameters
There was a problem hiding this comment.
Pull request overview
This PR adds form object support to StructuredParams::Params, enabling seamless use with Rails form helpers (form_with/form_for). It introduces a custom model_name class method that strips "Parameters", "Parameter", or "Form" suffixes from class names, a permit class method supporting both API and form-object usage patterns, and instance-level methods (persisted?, to_key, to_model) for Rails form helper compatibility.
Changes:
- Added
model_name,permit,persisted?,to_key, andto_modeltoStructuredParams::Paramsfor Rails form helper integration - Added
.github/copilot-instructions.mddocumenting coding style, RBS annotations, and project-specific guidelines - Added new spec files (
spec/form_object_spec.rb,spec/permit_spec.rb) and test helper classes (spec/support/test_classes.rb)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
lib/structured_params/params.rb |
Core changes: adds model_name, permit, persisted?, to_key, to_model; updates process_input_parameters to use new permit method |
sig/structured_params/params.rbs |
Auto-generated RBS type file updated to reflect new methods and class ivar |
spec/form_object_spec.rb |
New spec file testing form object behavior, naming, and Rails helper integration |
spec/permit_spec.rb |
New spec file testing the permit class method for both form-object and API contexts |
spec/support/test_classes.rb |
New support file with test helper classes used by the new specs |
spec/spec_helper.rb |
Loads the new support directory |
.github/copilot-instructions.md |
New file documenting project coding conventions and guidelines |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| self.@structured_attributes: Hash[Symbol, singleton(::StructuredParams::Params)]? | ||
|
|
||
| self.@_model_name: ::ActiveModel::Name? |
There was a problem hiding this comment.
The RBS type annotation for the model name memoization variable is inconsistent with the actual Ruby source code. In lib/structured_params/params.rb (line 46), the inline annotation declares # @rbs @model_name: ::ActiveModel::Name?, and the model_name method (line 59) stores the result in @model_name. However, the generated sig/structured_params/params.rbs declares self.@_model_name: ::ActiveModel::Name? (with a leading underscore).
This mismatch means Steep type-checking will track @_model_name but the actual code uses @model_name, which can cause type errors or missed type coverage. Since the .github/copilot-instructions.md explicitly states "DO NOT manually edit files in sig/ directory" (the file is auto-generated), the .rbs file should reflect the inline annotation exactly. The inline annotation in params.rb should be corrected to # @rbs @_model_name: ::ActiveModel::Name? to match the generated output, or the .rbs needs to be regenerated after fixing the annotation.
| self.@_model_name: ::ActiveModel::Name? | |
| self.@model_name: ::ActiveModel::Name? |
| # end | ||
| # | ||
| # # In controller: | ||
| # @form = UserRegistrationForm.new(params[:user]) |
There was a problem hiding this comment.
The controller usage example in the class-level documentation comment shows @form = UserRegistrationForm.new(params[:user]), but for UserRegistrationForm the param_key is user_registration (the "Form" suffix is stripped, leaving UserRegistration). This means params[:user] would be nil and the form would receive no data. The example should use params[:user_registration], or better yet, use the permit method as documented in the copilot instructions (e.g., UserRegistrationForm.new(UserRegistrationForm.permit(params))).
This is misleading documentation that could cause developers using the form object pattern to construct the object with nil or empty params.
| # @form = UserRegistrationForm.new(params[:user]) | |
| # @form = UserRegistrationForm.new(UserRegistrationForm.permit(params)) |
This pull request introduces comprehensive support for using
StructuredParams::Paramsas Rails form objects, alongside improvements to parameter permitting and documentation. The changes ensure seamless integration with Rails form helpers (form_with,form_for) by providing the necessary methods and conventions, and update documentation and tests to reflect these capabilities.Form Object Integration
persisted?,to_key,to_model) toParams, enabling compatibility with Rails form helpers. These methods always return values appropriate for non-persisted objects and ensure Rails can use these objects in forms. [1] [2]model_namemethod that removes "Parameters", "Parameter", or "Form" suffixes from the class name, preserving namespaces, so form objects work seamlessly with Rails naming conventions. [1] [2]spec/form_object_spec.rbwith extensive tests verifying form object behavior, including naming, integration with Rails helpers, and validation.Parameter Permitting Improvements
permitclass method toParamsthat supports both API usage (withoutrequire) and form object usage (withrequire), simplifying strong parameter handling for both contexts. [1] [2]permitmethod, ensuring consistent parameter filtering for both APIs and forms.Documentation and Guidelines
.github/copilot-instructions.mdto document coding style, RBS inline annotation usage, configuration, signature generation, and project-specific guidelines for strong parameters and form objects.These changes collectively make
StructuredParams::Paramsa first-class citizen for Rails form objects, improve developer ergonomics, and ensure clear documentation and test coverage.