diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb index 3a57509b2..6b9689fa7 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/action/actions.rb @@ -127,6 +127,14 @@ def ensure_served_action(context) "The collection #{context.collection.name} does not have this smart action" end + # A single action runs on its first record, or on nothing: refuse any selection but one id. + def ensure_single_record(context, selection_ids) + return unless context.collection.schema[:actions][@action_name].scope == Types::ActionScope::SINGLE + return unless selection_ids[:are_excluded] || selection_ids[:ids].size != 1 + + raise Http::Exceptions::UnprocessableError, 'This action can only be triggered on one record at a time.' + end + # Recorded as pending before the action runs and confirmed after, so an action that takes the process # down with it still leaves evidence that it started. A failed run is worth recording too — "who tried # to run this" is usually the interesting part — and an action answering with an Error result failed @@ -229,6 +237,7 @@ def get_record_selection(args, context) # Restrict the filter to the selected records for single or bulk actions if context.collection.schema[:actions][@action_name].scope != Types::ActionScope::GLOBAL selection_ids = Utils::Id.parse_selection_ids(context.collection, args[:params]) + ensure_single_record(context, selection_ids) selected_ids = ConditionTreeFactory.match_ids(context.collection, selection_ids[:ids]) selected_ids = selected_ids.inverse if selection_ids[:are_excluded] filter = filter.override( diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb index 2a42a043c..7a0be720a 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/action/actions_spec.rb @@ -159,6 +159,73 @@ def execute(result: { type: 'Success', message: 'done' }, &raising) expect(execute).to eq({ type: 'Success', message: 'done' }) end end + + # A single action context only ever reads the first record, so a selection naming more than one + # would run on that record alone and still answer success. + describe 'a single action given anything but one record' do + let(:action_scope) { ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope::SINGLE } + let(:permissions) { instance_spy(ForestAdminAgent::Services::Permissions) } + let(:context) do + double('context', collection: collection, caller: build_caller, permissions: permissions, + datasource: collection.datasource) + end + let(:message) { 'This action can only be triggered on one record at a time.' } + + def request(attributes) + { params: { data: { attributes: { values: {} }.merge(attributes) } } } + end + + before do + allow(route).to receive_messages(build: context, build_context_filter: filter) + allow(collection).to receive_messages(get_form: [], execute: { type: 'Success', message: 'done' }) + end + + it 'refuses two ids with a 422 and runs nothing' do + expect { route.handle_request(request(ids: %w[4 7])) } + .to raise_error(Http::Exceptions::UnprocessableError, message) + expect(collection).not_to have_received(:execute) + end + + it 'refuses two ids before checking the permissions, so no approval is asked for' do + expect { route.handle_request(request(ids: %w[4 7])) } + .to raise_error(Http::Exceptions::UnprocessableError, message) + expect(permissions).not_to have_received(:can_smart_action?) + end + + it 'refuses the approval of a request naming two ids and runs nothing' do + signed = JWT.encode({ data: { attributes: { ids: %w[4 7] } } }, Facades::Container.cache(:env_secret), 'HS256') + + expect { route.handle_request(request(ids: %w[4 7], signed_approval_request: signed)) } + .to raise_error(Http::Exceptions::UnprocessableError, message) + expect(collection).not_to have_received(:execute) + end + + it 'refuses an empty selection with a 422 and runs nothing' do + expect { route.handle_request(request(ids: [])) } + .to raise_error(Http::Exceptions::UnprocessableError, message) + expect(collection).not_to have_received(:execute) + end + + it 'refuses a select-all selection with a 422 and runs nothing' do + expect { route.handle_request(request(ids: [], all_records: true, all_records_ids_excluded: [])) } + .to raise_error(Http::Exceptions::UnprocessableError, message) + expect(collection).not_to have_received(:execute) + end + + it 'refuses loading the form with two ids and computes no form' do + expect { route.handle_hook_request(request(ids: %w[4 7])) } + .to raise_error(Http::Exceptions::UnprocessableError, message) + expect(collection).not_to have_received(:get_form) + end + + it 'still runs on exactly one id' do + route.handle_request(request(ids: %w[4])) + + expect(collection).to have_received(:execute).with( + context.caller, 'Refund', {}, having_attributes(condition_tree: having_attributes(field: 'id', value: 4)) + ) + end + end end end end