Skip to content

fix(rpc agent): allow connect as node rpc#310

Merged
arnaud-moncel merged 3 commits into
mainfrom
fix/rpc/stack-connection
May 22, 2026
Merged

fix(rpc agent): allow connect as node rpc#310
arnaud-moncel merged 3 commits into
mainfrom
fix/rpc/stack-connection

Conversation

@arnaud-moncel
Copy link
Copy Markdown
Member

@arnaud-moncel arnaud-moncel commented May 22, 2026

Definition of Done

General

  • Write an explicit title for the Pull Request, following Conventional Commits specification
  • Test manually the implemented changes
  • Validate the code quality (indentation, syntax, style, simplicity, readability)

Security

  • Consider the security impact of the changes made

Note

Fix RPC agent Rails routes to include URL path parameters in request params

  • Adds extract_request_params helper in BaseRoute that merges path_parameters, query_parameters, and request_parameters so that URL segment params like :collection_name are available to route handlers.
  • Route handlers in list, create, aggregate, and action_form previously received only query/body params, causing collection_name to be missing when provided as a path segment.
  • Changes the missing collection_name fallback return value from {} to [] across all affected handlers.

Changes since #310 opened

  • Removed early-return guards from route handlers to raise errors for missing or unknown collections [8e3ff47]
  • Refactored parameter extraction to use indifferent access without deep key symbolization [8e3ff47]
  • Updated route handler specs to expect errors instead of empty results for unknown collections [8e3ff47]
  • Added test coverage for ForestAdminRpcAgent::Routes::Aggregate#handle_request to verify that when the aggregation payload omits the :groups key, Aggregation.new is invoked with groups defaulting to an empty array [e0ffe0d]
  • Added test coverage for ForestAdminRpcAgent::Routes::BaseRoute#extract_request_params to verify parameter merging behavior from path, query, and body sources with indifferent access and proper filtering of Rails-internal keys [e0ffe0d]

Macroscope summarized cc8ced2.

Copy link
Copy Markdown
Member

@matthv matthv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch on the missing path_parameters merge.

I suggest to add a regression test for extract_request_params in base_route_spec.rb.
The specs were updated from eq({}) to eq([]) but none actually test the scenario being fixed (collection_name resolved from path_parameters only)

Comment thread packages/forest_admin_rpc_agent/lib/forest_admin_rpc_agent/routes/create.rb Outdated
Copy link
Copy Markdown
Member

@matthv matthv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

@arnaud-moncel arnaud-moncel merged commit 6f80070 into main May 22, 2026
44 checks passed
@arnaud-moncel arnaud-moncel deleted the fix/rpc/stack-connection branch May 22, 2026 14:20
forest-bot added a commit that referenced this pull request May 22, 2026
## [1.30.6](v1.30.5...v1.30.6) (2026-05-22)

### Bug Fixes

* **rpc agent:** allow connect as node rpc ([#310](#310)) ([6f80070](6f80070))
@forest-bot
Copy link
Copy Markdown
Member

🎉 This PR is included in version 1.30.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants