diff --git a/lib/activeadmin_mcp.rb b/lib/activeadmin_mcp.rb index 481e791..00c020d 100644 --- a/lib/activeadmin_mcp.rb +++ b/lib/activeadmin_mcp.rb @@ -3,6 +3,7 @@ require_relative "activeadmin_mcp/version" require_relative "activeadmin_mcp/configuration" require_relative "activeadmin_mcp/resource_registry" +require_relative "activeadmin_mcp/form_field_collector" require_relative "activeadmin_mcp/record_updater" require_relative "activeadmin_mcp/request_handler" require_relative "activeadmin_mcp/engine" diff --git a/lib/activeadmin_mcp/form_field_collector.rb b/lib/activeadmin_mcp/form_field_collector.rb new file mode 100644 index 0000000..6e8e1d6 --- /dev/null +++ b/lib/activeadmin_mcp/form_field_collector.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module ActiveadminMcp + # Records the field names declared by an ActiveAdmin `form do ... end` block. + # + # ActiveAdmin form blocks are arbitrary Formtastic DSL — `input`, `inputs`, + # `actions`, helper calls, conditionals — so the block is run against this + # stand-in form builder. Every `input :field` records `:field`; every other + # message (including unknown helpers) is swallowed and returns self, so the + # block executes without a real view context. Nested `has_many` associations + # are intentionally not descended into: the updater only writes flat + # attributes, and descending would record association fields as top-level. + class FormFieldCollector + attr_reader :fields + + def initialize + @fields = [] + end + + def collect(&block) + instance_exec(self, &block) + @fields.uniq + end + + def input(name, *_args, **_opts, &_block) + @fields << name.to_sym if name.respond_to?(:to_sym) + self + end + + def inputs(*_args, **_opts, &block) + instance_exec(self, &block) if block + self + end + + def has_many(*_args, **_opts) + self + end + + def method_missing(_name, *_args, **_opts, &block) + instance_exec(self, &block) if block + self + end + + def respond_to_missing?(_name, _include_private = false) + true + end + end +end diff --git a/lib/activeadmin_mcp/record_updater.rb b/lib/activeadmin_mcp/record_updater.rb index c4e1dcc..37a79c6 100644 --- a/lib/activeadmin_mcp/record_updater.rb +++ b/lib/activeadmin_mcp/record_updater.rb @@ -58,24 +58,47 @@ def authorized?(config, record) adapter_class.new(config, @current_user).authorized?(UPDATE, record) end - # Runs the incoming attributes through the resource controller's own - # permitted_params (compiled from `permit_params`), so we accept exactly what - # the admin form accepts. Fails closed if that cannot be resolved. + # Resolves the fields we may write, accepting exactly what the admin form + # accepts. Prefers the resource's own `permit_params` (via the controller's + # compiled permitted_params); when a resource declares its writable fields + # through a `form do ... end` block instead — as ActiveAdmin's default + # permitted_params then returns nil — derives them from the form inputs. + # Fails closed if neither can be resolved. def permitted_attributes(config, attributes) + from_permit_params(config, attributes) || + from_form(config, attributes) || + raise(PermitError, "Could not determine permitted attributes: #{@resource[:name]}") + end + + def from_permit_params(config, attributes) param_key = config.param_key.to_sym controller = config.controller.new - - unless controller.respond_to?(:permitted_params, true) - raise PermitError, "Resource does not declare permit_params: #{@resource[:name]}" - end + return nil unless controller.respond_to?(:permitted_params, true) controller.params = ActionController::Parameters.new(param_key => attributes) - permitted = controller.send(:permitted_params)[param_key] - permitted ? permitted.to_h.symbolize_keys : {} - rescue PermitError - raise - rescue StandardError => e - raise PermitError, "Could not determine permitted attributes: #{e.message}" + permitted = controller.send(:permitted_params) + scoped = permitted && permitted[param_key] + scoped ? scoped.to_h.symbolize_keys : nil + rescue StandardError + nil + end + + def from_form(config, attributes) + fields = form_fields(config) + return nil if fields.empty? + + ActionController::Parameters.new(attributes).permit(*fields).to_h.symbolize_keys + rescue StandardError + nil + end + + def form_fields(config) + return [] unless config.respond_to?(:page_presenters) + + block = config.page_presenters[:form]&.block + return [] unless block + + FormFieldCollector.new.collect(&block) end def error(message, details: nil) diff --git a/spec/activeadmin_mcp/record_updater_spec.rb b/spec/activeadmin_mcp/record_updater_spec.rb index 49dc0e0..0f8cc2a 100644 --- a/spec/activeadmin_mcp/record_updater_spec.rb +++ b/spec/activeadmin_mcp/record_updater_spec.rb @@ -128,4 +128,91 @@ def update(resource, id: 1, attributes:, current_user: :admin) expect(record).to have_received(:update).with(error: "boom") expect(result[:updated]).to eq([:error]) end + + # Resources that declare writable fields through a `form do ... end` block + # rather than `permit_params`. ActiveAdmin's default `permitted_params` for + # such a resource returns nil, so the updater derives the allowed fields from + # the form inputs instead. + describe "deriving permitted fields from the form block" do + # A controller that has not declared permit_params: `permitted_params` is nil. + def build_formless_controller + Class.new do + attr_accessor :params + def permitted_params = nil + private :permitted_params + end + end + + def build_form_resource(model:, adapter:, param_key: :widget, form_block:, + page_presenters: nil) + namespace = double("namespace", authorization_adapter: adapter) + presenters = page_presenters + presenters ||= { form: double("form presenter", block: form_block) } if form_block + config = double( + "config", + defined_actions: %i[index show new create edit update destroy], + namespace: namespace, + controller: build_formless_controller, + param_key: param_key, + page_presenters: presenters || {}, + ) + { name: "Widget", model: model, config: config } + end + + it "permits fields declared as form inputs and drops the rest" do + record = double("record", id: 1, as_json: {}) + allow(record).to receive(:update).and_return(true) + form_block = proc do |_f| + inputs do + input :name + input :role + end + actions + end + resource = build_form_resource(model: model_finding(record), adapter: permit_all, form_block: form_block) + + update(resource, attributes: { "name" => "Renamed", "role" => "admin", "secret" => "x" }) + + expect(record).to have_received(:update).with(name: "Renamed", role: "admin") + end + + it "tolerates arbitrary input options, helper calls and nesting in the form block" do + record = double("record", id: 1, as_json: {}) + allow(record).to receive(:update).and_return(true) + form_block = proc do |_f| + semantic_errors + inputs "Details" do + input :user_id, as: :hidden + input :name, as: :string, hint: some_undefined_helper + input :country, as: :select, collection: %w[UK IE] + end + actions + end + resource = build_form_resource(model: model_finding(record), adapter: permit_all, form_block: form_block) + + update(resource, attributes: { "user_id" => 5, "name" => "N", "country" => "UK", "nope" => 1 }) + + expect(record).to have_received(:update).with(user_id: 5, name: "N", country: "UK") + end + + it "fails closed when neither permit_params nor a form block is available" do + record = double("record", id: 1) + resource = build_form_resource(model: model_finding(record), adapter: permit_all, + form_block: nil, page_presenters: {}) + + result = update(resource, attributes: { "name" => "x" }) + + expect(result[:error]).to match(/could not determine permitted attributes/i) + end + + it "fails closed when the form block raises during introspection" do + record = double("record", id: 1) + form_block = proc { |_f| raise "boom" } + resource = build_form_resource(model: model_finding(record), adapter: permit_all, form_block: form_block) + + result = update(resource, attributes: { "name" => "x" }) + + expect(result[:error]).to match(/could not determine permitted attributes/i) + end + end end