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 a7a548f4d..3a57509b2 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 @@ -11,6 +11,7 @@ class Actions < AbstractAuthenticatedRoute include ForestAdminDatasourceToolkit::Components::Query include ForestAdminDatasourceToolkit::Components::Query::ConditionTree include ForestAdminDatasourceCustomizer::Decorators::Action + include ForestAdminAgent::Routes::QueryHandler def initialize(collection, action) @action_name = action @@ -223,18 +224,7 @@ def decode_signed_approval_request(signed_request) def get_record_selection(args, context) attributes = args.dig(:params, :data, :attributes) - # Match user filter + search + scope? + segment - scope = context.permissions.get_scope(context.collection) - filter = Filter.new( - condition_tree: ConditionTreeFactory.intersect( - [ - scope, - ForestAdminAgent::Utils::QueryStringParser.parse_condition_tree( - context.collection, args - ) - ] - ) - ) + filter = build_context_filter(context.collection, args, context.permissions, context.caller) # Restrict the filter to the selected records for single or bulk actions if context.collection.schema[:actions][@action_name].scope != Types::ActionScope::GLOBAL diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/query_handler.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/query_handler.rb index fb979f08e..f65769651 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/query_handler.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/query_handler.rb @@ -23,23 +23,25 @@ def execute_query(datasource, query, connection_name, permissions, caller, conte end def parse_query_segment(collection, args, permissions, caller) - return unless args[:params][:segmentQuery] + segment_query = QueryStringParser.subset_or_query(args, :segmentQuery) + return unless segment_query - unless args[:params][:connectionName] + connection_name = QueryStringParser.subset_or_query(args, :connectionName) + unless connection_name raise ForestAdminAgent::Http::Exceptions::UnprocessableError, 'Missing native query connection attribute' end - QueryValidator.valid?(args[:params][:segmentQuery]) + QueryValidator.valid?(segment_query) - permissions.can_execute_query_segment?(collection, args[:params][:segmentQuery], args[:params][:connectionName]) + permissions.can_execute_query_segment?(collection, segment_query, connection_name) ids = execute_query( collection.datasource, - args[:params][:segmentQuery], - args[:params][:connectionName], + segment_query, + connection_name, permissions, caller, - args[:params][:contextVariables] + QueryStringParser.subset_or_query(args, :contextVariables) ).map(&:values) condition_tree_segment = ConditionTree::ConditionTreeFactory.match_ids(collection, ids) @@ -47,6 +49,29 @@ def parse_query_segment(collection, args, permissions, caller) condition_tree_segment end + + def build_context_filter(collection, args, permissions, caller, sort: nil) + condition_tree = QueryStringParser.parse_condition_tree(collection, args) + search = QueryStringParser.parse_search(collection, args) + search_extended = QueryStringParser.parse_search_extended(args) + applied = { condition_tree: condition_tree, search: search, search_extended: search_extended } + applied[:sort] = sort if sort + permissions.assert_can_read_query_fields(collection, **applied) + + Filter.new( + condition_tree: ConditionTree::ConditionTreeFactory.intersect( + [ + permissions.get_scope(collection), + condition_tree, + parse_query_segment(collection, args, permissions, caller) + ] + ), + search: search, + search_extended: search_extended, + sort: sort, + segment: QueryStringParser.parse_segment(collection, args) + ) + end end end end diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/count.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/count.rb index 85ecee003..741008e09 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/count.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/count.rb @@ -19,28 +19,7 @@ def handle_request(args = {}) context.permissions.can?(:browse, context.collection) if context.collection.is_countable? - condition_tree = ForestAdminAgent::Utils::QueryStringParser.parse_condition_tree( - context.collection, args - ) - search = QueryStringParser.parse_search(context.collection, args) - search_extended = QueryStringParser.parse_search_extended(args) - context.permissions.assert_can_read_query_fields( - context.collection, - condition_tree: condition_tree, search: search, search_extended: search_extended - ) - - filter = ForestAdminDatasourceToolkit::Components::Query::Filter.new( - condition_tree: ConditionTreeFactory.intersect( - [ - context.permissions.get_scope(context.collection), - parse_query_segment(context.collection, args, context.permissions, context.caller), - condition_tree - ] - ), - search: search, - search_extended: search_extended, - segment: QueryStringParser.parse_segment(context.collection, args) - ) + filter = build_context_filter(context.collection, args, context.permissions, context.caller) aggregation = ForestAdminDatasourceToolkit::Components::Query::Aggregation.new(operation: 'Count') result = context.collection.aggregate(context.caller, filter, aggregation) diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/csv.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/csv.rb index dd45c45fa..7f493bbd8 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/csv.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/csv.rb @@ -23,28 +23,8 @@ def handle_request(args = {}) context.permissions.can?(:browse, context.collection) context.permissions.can?(:export, context.collection) - condition_tree = QueryStringParser.parse_condition_tree(context.collection, args) - search = QueryStringParser.parse_search(context.collection, args) - search_extended = QueryStringParser.parse_search_extended(args) sort = QueryStringParser.parse_sort(context.collection, args) - context.permissions.assert_can_read_query_fields( - context.collection, - condition_tree: condition_tree, sort: sort, search: search, search_extended: search_extended - ) - - filter = ForestAdminDatasourceToolkit::Components::Query::Filter.new( - condition_tree: ConditionTreeFactory.intersect( - [ - context.permissions.get_scope(context.collection), - parse_query_segment(context.collection, args, context.permissions, context.caller), - condition_tree - ] - ), - search: search, - search_extended: search_extended, - sort: sort, - segment: QueryStringParser.parse_segment(context.collection, args) - ) + filter = build_context_filter(context.collection, args, context.permissions, context.caller, sort: sort) requested = QueryStringParser.parse_requested_projection(context.collection, args) projection = context.permissions.redact_projection( context.collection, diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/delete.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/delete.rb index 5d1c1f3cf..5044b823c 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/delete.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/delete.rb @@ -8,6 +8,9 @@ class Delete < AbstractAuthenticatedRoute include ForestAdminAgent::Builder include ForestAdminDatasourceToolkit::Components::Query include ForestAdminDatasourceToolkit::Components::Query::ConditionTree + include ForestAdminAgent::Routes::QueryHandler + + NULLIFIED_POLYMORPHIC_TYPES = %w[PolymorphicOneToOne PolymorphicOneToMany].freeze def setup_routes add_route('forest_delete_bulk', 'delete', '/:collection_name', ->(args) { handle_request_bulk(args) }) @@ -39,45 +42,44 @@ def delete_records(args, selection_ids, context) selection_ids[:ids]) condition_tree_ids = condition_tree_ids.inverse if selection_ids[:are_excluded] - context.collection.schema[:fields].each_value do |field_schema| - next unless ['PolymorphicOneToOne', 'PolymorphicOneToMany'].include?(field_schema.type) - next if field_schema.respond_to?(:cascade_on_delete) && field_schema.cascade_on_delete + filter = build_context_filter(context.collection, args, context.permissions, context.caller) + filter = filter.override( + condition_tree: ConditionTree::ConditionTreeFactory.intersect([filter.condition_tree, condition_tree_ids]) + ) + + relations = nullified_polymorphic_relations(context.collection) + # ponytail: loads every deleted record's keys at once, page it if select-all deletes outgrow memory + records = if relations.any? + context.collection.list(context.caller, filter, + Projection.new(relations.map(&:origin_key_target).uniq)) + end - origin_values = selection_ids[:ids].map do |pk_hash| - if pk_hash.is_a?(Hash) - pk_hash[field_schema.origin_key_target] - else - pk_names = ForestAdminDatasourceToolkit::Utils::Schema.primary_keys(context.collection) - index = pk_names.index(field_schema.origin_key_target) - pk_hash[index] if index - end - end + context.collection.delete(context.caller, filter) + relations.each { |relation| nullify_polymorphic_references(context, relation, records) } + end - condition_tree = Nodes::ConditionTreeBranch.new( - 'And', - [ - Nodes::ConditionTreeLeaf.new(field_schema.origin_key, Operators::IN, origin_values), - Nodes::ConditionTreeLeaf.new(field_schema.origin_type_field, Operators::EQUAL, - context.collection.name.gsub('__', '::')) - ] - ) - filter = Filter.new(condition_tree: condition_tree) - context.datasource.get_collection(field_schema.foreign_collection) - .update(context.caller, filter, { field_schema.origin_key => nil, - field_schema.origin_type_field => nil }) + def nullified_polymorphic_relations(collection) + collection.schema[:fields].values.select do |field_schema| + NULLIFIED_POLYMORPHIC_TYPES.include?(field_schema.type) && + !(field_schema.respond_to?(:cascade_on_delete) && field_schema.cascade_on_delete) end + end - filter = ForestAdminDatasourceToolkit::Components::Query::Filter.new( - condition_tree: ConditionTree::ConditionTreeFactory.intersect( - [ - Utils::QueryStringParser.parse_condition_tree(context.collection, args), - condition_tree_ids, - context.permissions.get_scope(context.collection) - ] - ) - ) + def nullify_polymorphic_references(context, relation, records) + origin_values = records.map { |record| record[relation.origin_key_target] } + return if origin_values.empty? - context.collection.delete(context.caller, filter) + condition_tree = Nodes::ConditionTreeBranch.new( + 'And', + [ + Nodes::ConditionTreeLeaf.new(relation.origin_key, Operators::IN, origin_values), + Nodes::ConditionTreeLeaf.new(relation.origin_type_field, Operators::EQUAL, + context.collection.name.gsub('__', '::')) + ] + ) + context.datasource.get_collection(relation.foreign_collection) + .update(context.caller, Filter.new(condition_tree: condition_tree), + { relation.origin_key => nil, relation.origin_type_field => nil }) end end end diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/list.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/list.rb index f26244ab5..a6aff6ed5 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/list.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/list.rb @@ -18,29 +18,9 @@ def handle_request(args = {}) context = build(args) context.permissions.can?(:browse, context.collection) - condition_tree = QueryStringParser.parse_condition_tree(context.collection, args) - search = QueryStringParser.parse_search(context.collection, args) - search_extended = QueryStringParser.parse_search_extended(args) sort = QueryStringParser.parse_sort(context.collection, args) - context.permissions.assert_can_read_query_fields( - context.collection, - condition_tree: condition_tree, sort: sort, search: search, search_extended: search_extended - ) - - filter = ForestAdminDatasourceToolkit::Components::Query::Filter.new( - condition_tree: ConditionTreeFactory.intersect( - [ - context.permissions.get_scope(context.collection), - condition_tree, - parse_query_segment(context.collection, args, context.permissions, context.caller) - ] - ), - page: QueryStringParser.parse_pagination(args), - search: search, - search_extended: search_extended, - sort: sort, - segment: QueryStringParser.parse_segment(context.collection, args) - ) + filter = build_context_filter(context.collection, args, context.permissions, context.caller, sort: sort) + .override(page: QueryStringParser.parse_pagination(args)) projection = redacted_projection_with_pks(context, context.collection, args) records = context.collection.list(context.caller, filter, projection) diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/count_related.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/count_related.rb index a55b87602..6b5e816af 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/count_related.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/count_related.rb @@ -8,6 +8,7 @@ class CountRelated < AbstractRelatedRoute include ForestAdminAgent::Builder include ForestAdminDatasourceToolkit::Utils include ForestAdminDatasourceToolkit::Components::Query + include ForestAdminAgent::Routes::QueryHandler def setup_routes add_route( @@ -25,7 +26,7 @@ def handle_request(args = {}) context.permissions.can?(:browse, context.child_collection) if context.child_collection.is_countable? - filter = Filter.new(condition_tree: context.permissions.get_scope(context.child_collection)) + filter = build_context_filter(context.child_collection, args, context.permissions, context.caller) primary_key_values = Utils::Id.unpack_id(context.collection, args[:params]['id'], with_key: true) result = Collection.aggregate_relation( context.collection, diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/csv_related.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/csv_related.rb index 294df1d89..b77032004 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/csv_related.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/csv_related.rb @@ -6,6 +6,7 @@ class CsvRelated < AbstractRelatedRoute include ForestAdminAgent::Utils include ForestAdminDatasourceToolkit::Utils include ForestAdminDatasourceToolkit::Components::Query::ConditionTree + include ForestAdminAgent::Routes::QueryHandler def setup_routes add_route( @@ -23,18 +24,7 @@ def handle_request(args = {}) context = build(args) context.permissions.can?(:browse, context.child_collection) context.permissions.can?(:export, context.child_collection) - condition_tree = ForestAdminAgent::Utils::QueryStringParser.parse_condition_tree( - context.child_collection, args - ) - context.permissions.assert_can_read_query_fields( - context.child_collection, condition_tree: condition_tree - ) - - filter = ForestAdminDatasourceToolkit::Components::Query::Filter.new( - condition_tree: ConditionTreeFactory.intersect( - [context.permissions.get_scope(context.child_collection), condition_tree] - ) - ) + filter = build_context_filter(context.child_collection, args, context.permissions, context.caller) requested = ForestAdminAgent::Utils::QueryStringParser.parse_requested_projection( context.child_collection, args ) diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/dissociate_related.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/dissociate_related.rb index fe003eaff..2d4b25b69 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/dissociate_related.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/dissociate_related.rb @@ -8,6 +8,7 @@ class DissociateRelated < AbstractRelatedRoute include ForestAdminAgent::Builder include ForestAdminDatasourceToolkit::Utils include ForestAdminDatasourceToolkit::Components::Query + include ForestAdminAgent::Routes::QueryHandler def setup_routes add_route( @@ -126,14 +127,9 @@ def get_base_foreign_filter(args, context) raise ForestAdminDatasourceToolkit::Exceptions::ForestException, 'Expected no empty id list' end - Filter.new( - condition_tree: ConditionTree::ConditionTreeFactory.intersect( - [ - context.permissions.get_scope(context.child_collection), - Utils::QueryStringParser.parse_condition_tree(context.child_collection, args), - selected_ids - ] - ) + filter = build_context_filter(context.child_collection, args, context.permissions, context.caller) + filter.override( + condition_tree: ConditionTree::ConditionTreeFactory.intersect([filter.condition_tree, selected_ids]) ) end end diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/list_related.rb b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/list_related.rb index 5b906a308..0c3dfc5c8 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/list_related.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/routes/resources/related/list_related.rb @@ -8,6 +8,7 @@ class ListRelated < AbstractRelatedRoute include ForestAdminAgent::Builder include ForestAdminDatasourceToolkit::Utils include ForestAdminDatasourceToolkit::Components::Query::ConditionTree + include ForestAdminAgent::Routes::QueryHandler def setup_routes add_route( @@ -23,21 +24,10 @@ def setup_routes def handle_request(args = {}) context = build(args) context.permissions.can?(:browse, context.child_collection) - condition_tree = ForestAdminAgent::Utils::QueryStringParser.parse_condition_tree( - context.child_collection, args - ) sort = ForestAdminAgent::Utils::QueryStringParser.parse_sort(context.child_collection, args) - context.permissions.assert_can_read_query_fields( - context.child_collection, condition_tree: condition_tree, sort: sort - ) - - filter = ForestAdminDatasourceToolkit::Components::Query::Filter.new( - condition_tree: ConditionTreeFactory.intersect( - [context.permissions.get_scope(context.child_collection), condition_tree] - ), - page: ForestAdminAgent::Utils::QueryStringParser.parse_pagination(args), - sort: sort - ) + filter = build_context_filter(context.child_collection, args, context.permissions, context.caller, + sort: sort) + .override(page: ForestAdminAgent::Utils::QueryStringParser.parse_pagination(args)) projection = redacted_projection_with_pks(context, context.child_collection, args) primary_key_values = Utils::Id.unpack_id(context.collection, args[:params]['id'], with_key: true) records = Collection.list_relation( diff --git a/packages/forest_admin_agent/lib/forest_admin_agent/utils/query_string_parser.rb b/packages/forest_admin_agent/lib/forest_admin_agent/utils/query_string_parser.rb index d5f2c4d90..18a562bc7 100644 --- a/packages/forest_admin_agent/lib/forest_admin_agent/utils/query_string_parser.rb +++ b/packages/forest_admin_agent/lib/forest_admin_agent/utils/query_string_parser.rb @@ -318,8 +318,7 @@ def self.parse_chart_parameters(args) end def self.parse_segment(collection, args) - segment = args.dig(:params, :data, :attributes, :all_records_subset_query, - :segment) || args.dig(:params, :segment) + segment = subset_or_query(args, :segment) || args.dig(:params, :segment) return unless segment diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/query_handler_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/query_handler_spec.rb index 3ee91fd43..31938aaf3 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/query_handler_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/query_handler_spec.rb @@ -83,6 +83,23 @@ module Routes expect(condition_tree.to_h).to eq({ field: 'id', operator: Operators::IN, value: [1, 2] }) end + it 'reads the segment query a select-all sends in its body' do + args = { + params: { + data: { + attributes: { + all_records_subset_query: { segmentQuery: 'SELECT id from user', connectionName: 'primary' } + } + } + } + } + + condition_tree = dummy_class.parse_query_segment(collection, args, permission, caller) + expect(condition_tree.to_h).to eq({ field: 'id', operator: Operators::IN, value: [1, 2] }) + expect(permission).to have_received(:can_execute_query_segment?) + .with(collection, 'SELECT id from user', 'primary') + end + it 'work with inject context variable' do args = { params: { diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/action/actions_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/action/actions_spec.rb index 41f26e822..e651fdff7 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/action/actions_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/action/actions_spec.rb @@ -43,9 +43,11 @@ module Action 'id' => ColumnSchema.new( column_type: 'Uuid', is_primary_key: true, - filter_operators: [Operators::EQUAL, Operators::IN] - ) - } + filter_operators: [Operators::EQUAL, Operators::IN, Operators::NOT_EQUAL] + ), + 'title' => ColumnSchema.new(column_type: 'String') + }, + segments: ['recent'] } ) @@ -62,6 +64,7 @@ module Action allow(permissions).to receive_messages( can_smart_action?: true, get_scope: nil, + assert_can_read_query_fields: nil, get_user_data: { id: 1, firstName: 'John', @@ -170,6 +173,49 @@ def declare_actions(actions) end end + context 'with a bulk action run on a searched, segmented select-all' do + before do + @action_collection.add_action( + 'foo', + ForestAdminDatasourceCustomizer::Decorators::Action::BaseAction.new( + scope: ForestAdminDatasourceCustomizer::Decorators::Action::Types::ActionScope::BULK + ) do |_context, result_builder| + result_builder.success + end + ) + args[:params][:data][:attributes].merge!( + ids: [], + all_records: true, + all_records_ids_excluded: ['123e4567-e89b-12d3-a456-426614174000'], + all_records_subset_query: { search: 'tolkien', segment: 'recent' } + ) + allow(@action_collection).to receive(:get_form).and_return([]) + end + + it 'hands the action the search and the segment of the list, minus the excluded ids' do + action.handle_hook_request(args) + + expect(@action_collection).to have_received(:get_form) do |caller, action_name, data, filter, _meta| + expect(caller).to be_instance_of(Components::Caller) + expect(action_name).to eq('foo') + expect(data).to be_nil + expect(filter).to have_attributes( + search: 'tolkien', search_extended: false, segment: 'recent', sort: nil, page: nil + ) + expect(filter.condition_tree.to_h).to eq( + { field: 'id', operator: Operators::NOT_EQUAL, value: '123e4567-e89b-12d3-a456-426614174000' } + ) + end + end + + it 'checks the caller can read the fields the search reaches' do + action.handle_hook_request(args) + + expect(permissions).to have_received(:assert_can_read_query_fields) + .with(@datasource.get_collection('book'), condition_tree: nil, search: 'tolkien', search_extended: false) + end + end + context 'with a single action used from list-view, detail-view & summary' do before do @action_collection.add_action( diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/delete_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/delete_spec.rb index 96254ed6b..3328bfcd0 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/delete_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/delete_spec.rb @@ -22,7 +22,7 @@ module Resources before do allow(ForestAdminAgent::Services::Permissions).to receive(:new).and_return(permissions) - allow(permissions).to receive_messages(can?: true, get_scope: nil) + allow(permissions).to receive_messages(can?: true, get_scope: nil, assert_can_read_query_fields: nil) end it 'adds the route forest_store' do @@ -56,7 +56,8 @@ def respond_to?(arg) ), 'first_name' => ColumnSchema.new(column_type: 'String'), 'last_name' => ColumnSchema.new(column_type: 'String') - } + }, + segments: ['active'] }, delete: true ) @@ -148,6 +149,40 @@ def respond_to?(arg) expect(filter.condition_tree.to_h).to eq(field: 'id', operator: Operators::NOT_IN, value: [1, 2, 3]) end end + + context 'when every record of a searched, segmented list is selected' do + before do + args[:params][:data][:attributes][:all_records] = true + args[:params][:data][:attributes][:all_records_ids_excluded] = %w[3] + args[:params][:data][:attributes][:all_records_subset_query].merge!( + search: 'jo', searchExtended: '1', segment: 'active' + ) + end + + it 'deletes only the records the search and the segment match, minus the excluded ones' do + delete.handle_request_bulk(args) + + expect(@datasource.get_collection('user')).to have_received(:delete) do |caller, filter| + expect(caller).to be_instance_of(Components::Caller) + expect(filter).to have_attributes( + search: 'jo', + search_extended: true, + segment: 'active', + page: nil, + sort: nil + ) + expect(filter.condition_tree.to_h).to eq(field: 'id', operator: Operators::NOT_EQUAL, value: 3) + end + end + + it 'rejects a segment the collection does not have, deleting nothing' do + args[:params][:data][:attributes][:all_records_subset_query][:segment] = 'unknown' + + expect { delete.handle_request_bulk(args) } + .to raise_error(ForestAdminAgent::Http::Exceptions::BadRequestError, 'Invalid segment: unknown') + expect(@datasource.get_collection('user')).not_to have_received(:delete) + end + end end context 'with polymorphic relations' do @@ -224,6 +259,7 @@ def respond_to?(arg) @datasource = ForestAdminAgent::Facades::Container.datasource allow(@datasource.get_collection('user')).to receive(:delete) + allow(@datasource.get_collection('user')).to receive(:list).and_return([{ 'id' => 42 }]) allow(@datasource.get_collection('address')).to receive(:update) end @@ -255,6 +291,72 @@ def respond_to?(arg) end end end + + describe 'polymorphic relation cleanup on a searched select-all' do + let(:params) do + { + 'collection_name' => 'user', + 'timezone' => 'Europe/Paris', + data: { + attributes: { + ids: [], + all_records: true, + all_records_ids_excluded: ['7'], + all_records_subset_query: { search: 'jo' } + }, + type: 'action-requests' + } + } + end + + it 'nullifies the references of the records the search matched, not of the excluded ids' do + delete.handle_request_bulk(args) + + expect(@datasource.get_collection('user')).to have_received(:list) do |caller, filter, projection| + expect(caller).to be_instance_of(Components::Caller) + expect(filter).to have_attributes( + search: 'jo', search_extended: false, segment: nil, sort: nil, page: nil + ) + expect(filter.condition_tree.to_h).to eq(field: 'id', operator: Operators::NOT_EQUAL, value: 7) + expect(projection).to eq(['id']) + end + expect(@datasource.get_collection('address')).to have_received(:update) do |caller, filter, patch| + expect(caller).to be_instance_of(Components::Caller) + expect(filter).to have_attributes(search: nil, search_extended: nil, segment: nil, sort: nil, page: nil) + expect(filter.condition_tree.to_h).to eq( + aggregator: 'And', + conditions: [ + { field: 'addressable_id', operator: Operators::IN, value: [42] }, + { field: 'addressable_type', operator: Operators::EQUAL, value: 'user' } + ] + ) + expect(patch).to eq({ 'addressable_id' => nil, 'addressable_type' => nil }) + end + end + + it 'nullifies nothing when the search matches no record' do + allow(@datasource.get_collection('user')).to receive(:list).and_return([]) + + delete.handle_request_bulk(args) + + expect(@datasource.get_collection('address')).not_to have_received(:update) + end + + it 'keeps the references attached when the delete itself fails' do + allow(@datasource.get_collection('user')).to receive(:delete).and_raise(StandardError, 'hook refused') + + expect { delete.handle_request_bulk(args) }.to raise_error(StandardError, 'hook refused') + expect(@datasource.get_collection('address')).not_to have_received(:update) + end + + it 'nullifies nothing when the select-all filter is rejected' do + args[:params][:data][:attributes][:all_records_subset_query][:segment] = 'unknown' + + expect { delete.handle_request_bulk(args) } + .to raise_error(ForestAdminAgent::Http::Exceptions::BadRequestError, 'Invalid segment: unknown') + expect(@datasource.get_collection('address')).not_to have_received(:update) + end + end end context 'with composite primary keys and polymorphic relations' do @@ -337,6 +439,9 @@ def respond_to?(arg) @datasource = ForestAdminAgent::Facades::Container.datasource allow(@datasource.get_collection('composite_model')).to receive(:delete) + allow(@datasource.get_collection('composite_model')).to receive(:list).and_return( + [{ 'key1' => 'abc', 'key2' => 1 }, { 'key1' => 'def', 'key2' => 2 }, { 'key1' => 'ghi', 'key2' => 3 }] + ) allow(@datasource.get_collection('composite_address')).to receive(:update) end diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/count_related_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/count_related_spec.rb index 742b87049..dea574f8e 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/count_related_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/count_related_spec.rb @@ -53,8 +53,9 @@ module Related schema: { fields: { 'id' => ColumnSchema.new(column_type: 'Number', is_primary_key: true), - 'label' => ColumnSchema.new(column_type: 'String') - } + 'label' => ColumnSchema.new(column_type: 'String', filter_operators: [Operators::I_CONTAINS]) + }, + segments: ['popular'] } ) allow(ForestAdminAgent::Builder::AgentFactory.instance).to receive(:send_schema).and_return(nil) @@ -64,7 +65,7 @@ module Related ForestAdminAgent::Builder::AgentFactory.instance.build allow(ForestAdminAgent::Services::Permissions).to receive(:new).and_return(permissions) - allow(permissions).to receive_messages(can?: true, get_scope: Nodes::ConditionTreeBranch.new('Or', [])) + allow(permissions).to receive_messages(can?: true, get_scope: Nodes::ConditionTreeBranch.new('Or', []), assert_can_read_query_fields: nil) end it 'adds the route forest_related_count' do @@ -92,7 +93,7 @@ module Related condition_tree: have_attributes(aggregator: 'Or', conditions: []), page: nil, search: nil, - search_extended: nil, + search_extended: false, segment: nil, sort: nil ) @@ -102,6 +103,34 @@ module Related end end + context 'when the related list is searched and segmented' do + it 'counts only the related records the search and the segment match' do + args[:params].merge!('relation_name' => 'category', 'id' => 1, search: 'bar', segment: 'popular') + ForestAdminAgent::Facades::Container.datasource.get_collection('category').enable_count + allow(ForestAdminDatasourceToolkit::Utils::Collection).to receive(:aggregate_relation) + .and_return([{ value: 1 }]) + + count.handle_request(args) + + expect(ForestAdminDatasourceToolkit::Utils::Collection).to have_received(:aggregate_relation) do + |collection, id, relation_name, caller, foreign_filter, aggregation| + expect(caller).to be_instance_of(Components::Caller) + expect(collection.name).to eq('user') + expect(id).to eq({ 'id' => 1 }) + expect(relation_name).to eq('category') + expect(foreign_filter).to have_attributes( + condition_tree: have_attributes(aggregator: 'Or', conditions: []), + page: nil, + search: 'bar', + search_extended: false, + segment: 'popular', + sort: nil + ) + expect(aggregation).to have_attributes(operation: 'Count') + end + end + end + context 'when checking permissions and scope' do it 'authorizes and scopes on the related collection, not the parent' do args[:params]['relation_name'] = 'category' diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/csv_related_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/csv_related_spec.rb index 5e67841ed..bb85995dc 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/csv_related_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/csv_related_spec.rb @@ -57,8 +57,9 @@ module Related schema: { fields: { 'id' => ColumnSchema.new(column_type: 'Number', is_primary_key: true), - 'label' => ColumnSchema.new(column_type: 'String') - } + 'label' => ColumnSchema.new(column_type: 'String', filter_operators: [Operators::I_CONTAINS]) + }, + segments: ['popular'] }, list: [Category.new(1, 'bar'), Category.new(2, 'baz'), Category.new(3, 'qux')] ) @@ -87,7 +88,7 @@ module Related csv.handle_request(args) - expect(read_guard_calls[:query_fields]).to eq([{ collection: 'category', applies: %i[filter] }]) + expect(read_guard_calls[:query_fields]).to eq([{ collection: 'category', applies: %i[filter search search_extended] }]) expect(read_guard_calls[:projections]).to eq([{ collection: 'category', named_by_caller: false }]) end @@ -150,6 +151,24 @@ module Related ) end + it 'exports only the related records the search and the segment match' do + args[:params].merge!('relation_name' => 'category', 'id' => 1, search: 'bar', segment: 'popular') + allow(csv_generator_stream).to receive(:stream).and_return(["id,label\n"].to_enum) + + csv.handle_request(args) + + expect(csv_generator_stream).to have_received(:stream) do |_header, filter, _projection, _list_records, _limit| + expect(filter).to have_attributes( + condition_tree: have_attributes(aggregator: 'Or', conditions: []), + page: nil, + search: 'bar', + search_extended: false, + segment: 'popular', + sort: nil + ) + end + end + it 'passes a lambda to stream that calls list_relation with correct parameters' do args[:params]['relation_name'] = 'category' args[:params]['id'] = 1 diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/dissociate_related_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/dissociate_related_spec.rb index bcf48f52b..664aae886 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/dissociate_related_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/dissociate_related_spec.rb @@ -90,6 +90,7 @@ module Related ) } ) + collection_address.add_segments(['billing']) allow(ForestAdminAgent::Builder::AgentFactory.instance).to receive(:send_schema).and_return(nil) datasource.add_collection(collection_user) @@ -101,7 +102,7 @@ module Related @datasource = ForestAdminAgent::Facades::Container.datasource allow(ForestAdminAgent::Services::Permissions).to receive(:new).and_return(permissions) - allow(permissions).to receive_messages(can?: true, get_scope: nil) + allow(permissions).to receive_messages(can?: true, get_scope: nil, assert_can_read_query_fields: nil) end it 'adds the route forest_related_dissociate' do @@ -153,7 +154,7 @@ module Related ), page: nil, search: nil, - search_extended: nil, + search_extended: false, segment: nil, sort: nil ) @@ -187,7 +188,7 @@ module Related ), page: nil, search: nil, - search_extended: nil, + search_extended: false, segment: nil, sort: nil ) @@ -226,7 +227,7 @@ module Related ), page: nil, search: nil, - search_extended: nil, + search_extended: false, segment: nil, sort: nil ) @@ -287,7 +288,7 @@ module Related ), page: nil, search: nil, - search_extended: nil, + search_extended: false, segment: nil, sort: nil ) @@ -321,7 +322,7 @@ module Related ), page: nil, search: nil, - search_extended: nil, + search_extended: false, segment: nil, sort: nil ) @@ -339,7 +340,7 @@ module Related ), page: nil, search: nil, - search_extended: nil, + search_extended: false, segment: nil, sort: nil ) @@ -374,7 +375,7 @@ module Related ), page: nil, search: nil, - search_extended: nil, + search_extended: false, segment: nil, sort: nil ) @@ -409,7 +410,7 @@ module Related ), page: nil, search: nil, - search_extended: nil, + search_extended: false, segment: nil, sort: nil ) @@ -417,6 +418,43 @@ module Related expect(result).to eq({ content: nil, status: 204 }) end + + it 'deletes only the related records a select-all search and segment match' do + allow(@datasource.get_collection('address')).to receive(:delete).and_return(true) + + args[:params][:delete] = true + args[:params]['relation_name'] = 'address_poly' + args[:params]['id'] = 1 + args[:params][:data] = { + attributes: { + ids: [], + all_records: true, + all_records_ids_excluded: ['2'], + all_records_subset_query: { search: 'paris', segment: 'billing' } + } + } + + dissociate.handle_request(args) + + expect(@datasource.get_collection('address')).to have_received(:delete) do |caller, filter| + expect(caller).to be_instance_of(Components::Caller) + expect(filter).to have_attributes( + condition_tree: have_attributes( + aggregator: 'And', + conditions: [ + have_attributes(field: 'id', operator: Operators::NOT_EQUAL, value: 2), + have_attributes(field: 'addressable_id', operator: Operators::EQUAL, value: 1), + have_attributes(field: 'addressable_type', operator: Operators::EQUAL, value: 'user') + ] + ), + page: nil, + search: 'paris', + search_extended: false, + segment: 'billing', + sort: nil + ) + end + end end end end diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/list_related_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/list_related_spec.rb index 30288e707..0cf86ace1 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/list_related_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/routes/resources/related/list_related_spec.rb @@ -59,7 +59,8 @@ module Related is_primary_key: true ), 'label' => ColumnSchema.new(column_type: 'String') - } + }, + segments: ['popular'] } ) allow(ForestAdminAgent::Builder::AgentFactory.instance).to receive(:send_schema).and_return(nil) @@ -80,8 +81,8 @@ module Related expect(list.routes.length).to eq 1 end - # The only routes resolving against the foreign collection rather than their own, and a - # related listing applies no search — so neither is checked against the parent. + # The only routes resolving against the foreign collection rather than their own, so none of + # their components is checked against the parent. it 'checks the components it applies against the foreign collection' do args[:params]['relation_name'] = 'category' args[:params]['id'] = 1 @@ -90,11 +91,37 @@ module Related list.handle_request(args) expect(read_guard_calls[:query_fields]).to eq( - [{ collection: 'category', applies: %i[filter sort] }] + [{ collection: 'category', applies: %i[filter sort search search_extended] }] ) expect(read_guard_calls[:projections]).to eq([{ collection: 'category', named_by_caller: false }]) end + context 'when the related list is searched and segmented' do + it 'lists only the related records the search and the segment match' do + args[:params].merge!('relation_name' => 'category', 'id' => 1, search: 'bar', segment: 'popular') + allow(ForestAdminDatasourceToolkit::Utils::Collection).to receive(:list_relation).and_return([]) + + list.handle_request(args) + + expect(ForestAdminDatasourceToolkit::Utils::Collection).to have_received(:list_relation) do + |collection, id, relation_name, caller, foreign_filter, projection| + expect(caller).to be_instance_of(Components::Caller) + expect(collection.name).to eq('user') + expect(id).to eq({ 'id' => 1 }) + expect(relation_name).to eq('category') + expect(foreign_filter).to have_attributes( + condition_tree: have_attributes(aggregator: 'Or', conditions: []), + page: be_instance_of(ForestAdminDatasourceToolkit::Components::Query::Page), + search: 'bar', + search_extended: false, + segment: 'popular', + sort: [{ ascending: true, field: 'id' }] + ) + expect(projection).to eq(%w[id label]) + end + end + end + context 'when call without filters' do it 'call list_relation with expected args' do args[:params]['relation_name'] = 'category' @@ -112,7 +139,7 @@ module Related condition_tree: have_attributes(aggregator: 'Or', conditions: []), page: be_instance_of(ForestAdminDatasourceToolkit::Components::Query::Page), search: nil, - search_extended: nil, + search_extended: false, segment: nil, sort: [{ ascending: true, field: 'id' }] ) @@ -196,7 +223,7 @@ module Related ), page: be_instance_of(ForestAdminDatasourceToolkit::Components::Query::Page), search: nil, - search_extended: nil, + search_extended: false, segment: nil, sort: [{ ascending: true, field: 'id' }] ) diff --git a/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/query_string_parser_spec.rb b/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/query_string_parser_spec.rb index 30325b65d..04d82c6e7 100644 --- a/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/query_string_parser_spec.rb +++ b/packages/forest_admin_agent/spec/lib/forest_admin_agent/utils/query_string_parser_spec.rb @@ -1381,6 +1381,22 @@ module Utils end end + describe 'when parse_segment' do + let(:segmented) { build_collection(schema: { segments: ['active'] }) } + + it 'falls back to the query string segment when data is an array of picked ids' do + args = { params: { data: [{ 'id' => 1 }], segment: 'active' } } + + expect(described_class.parse_segment(segmented, args)).to eq('active') + end + + it 'falls back to the query string segment when the select-all body sends false' do + args = { params: { segment: 'active', data: { attributes: { all_records_subset_query: { segment: false } } } } } + + expect(described_class.parse_segment(segmented, args)).to eq('active') + end + end + describe 'when parse_search_extended' do it 'returns the query searchExtended parameter' do args = { params: { searchExtended: true } } diff --git a/packages/forest_admin_datasource_active_record/lib/forest_admin_datasource_active_record/utils/query.rb b/packages/forest_admin_datasource_active_record/lib/forest_admin_datasource_active_record/utils/query.rb index 12a800f48..81228bfba 100644 --- a/packages/forest_admin_datasource_active_record/lib/forest_admin_datasource_active_record/utils/query.rb +++ b/packages/forest_admin_datasource_active_record/lib/forest_admin_datasource_active_record/utils/query.rb @@ -57,92 +57,84 @@ def apply_pagination(page) @query end - def apply_condition_tree(condition_tree, aggregator = nil) - if condition_tree.is_a? Nodes::ConditionTreeBranch - aggregator = condition_tree.aggregator.downcase.to_sym - condition_tree.conditions.each do |condition| - @query = apply_condition_tree(condition, aggregator) - @query = @query.send(aggregator, @query) - end + def apply_condition_tree(condition_tree) + query_aggregator(:and, condition_relation(condition_tree)) + end - @query - else - @query = compute_main_operator(condition_tree, aggregator || :and) - end + # Each branch is built as its own relation so ActiveRecord groups it: chaining every leaf onto + # @query compiled And[a, Or[x, y]] as `a OR x OR y`. + def condition_relation(condition_tree) + return leaf_relation(condition_tree) unless condition_tree.is_a?(Nodes::ConditionTreeBranch) + + aggregator = condition_tree.aggregator.downcase.to_sym + relations = condition_tree.conditions.map { |condition| condition_relation(condition) } + return @collection.model.where(Arel.sql(aggregator == :or ? '1 = 0' : '1 = 1')) if relations.empty? + + relations.reduce { |combined, relation| combined.send(aggregator, relation) } end - def compute_main_operator(condition_tree, aggregator) + def leaf_relation(condition_tree) resolved = resolve_field(condition_tree.field) field = resolved[:formatted] arel_attr = resolved[:arel_attr] value = condition_tree.value - aggregator = aggregator.to_sym case condition_tree.operator when Operators::PRESENT field_schema = ForestAdminDatasourceToolkit::Utils::Collection.get_field_schema(@collection, condition_tree.field) - @query = if field_schema.column_type == 'String' - query_aggregator(aggregator, @collection.model.where.not({ field => [nil, ''] })) - else - query_aggregator(aggregator, @collection.model.where.not({ field => nil })) - end + if field_schema.column_type == 'String' + @collection.model.where.not({ field => [nil, ''] }) + else + @collection.model.where.not({ field => nil }) + end when Operators::BLANK field_schema = ForestAdminDatasourceToolkit::Utils::Collection.get_field_schema(@collection, condition_tree.field) - @query = if field_schema.column_type == 'String' - query_aggregator(aggregator, @collection.model.where({ field => [nil, ''] })) - else - query_aggregator(aggregator, @collection.model.where({ field => nil })) - end + if field_schema.column_type == 'String' + @collection.model.where({ field => [nil, ''] }) + else + @collection.model.where({ field => nil }) + end when Operators::MISSING - @query = query_aggregator(aggregator, @collection.model.where({ field => nil })) + @collection.model.where({ field => nil }) when Operators::EQUAL, Operators::IN - @query = query_aggregator(aggregator, @collection.model.where({ field => value })) + @collection.model.where({ field => value }) when Operators::NOT_EQUAL, Operators::NOT_IN - @query = query_aggregator(aggregator, @collection.model.where.not({ field => value })) + @collection.model.where.not({ field => value }) when Operators::GREATER_THAN - @query = query_aggregator(aggregator, build_comparison_query(condition_tree.field, arel_attr, value, :gt)) + build_comparison_query(condition_tree.field, arel_attr, value, :gt) when Operators::GREATER_THAN_OR_EQUAL - @query = query_aggregator(aggregator, build_comparison_query(condition_tree.field, arel_attr, value, :gteq)) + build_comparison_query(condition_tree.field, arel_attr, value, :gteq) when Operators::LESS_THAN - @query = query_aggregator(aggregator, build_comparison_query(condition_tree.field, arel_attr, value, :lt)) + build_comparison_query(condition_tree.field, arel_attr, value, :lt) when Operators::LESS_THAN_OR_EQUAL - @query = query_aggregator(aggregator, build_comparison_query(condition_tree.field, arel_attr, value, :lteq)) + build_comparison_query(condition_tree.field, arel_attr, value, :lteq) when Operators::CONTAINS - @query = query_aggregator(aggregator, - @collection.model.where(arel_attr.matches("%#{value}%"))) + @collection.model.where(arel_attr.matches("%#{value}%")) when Operators::I_CONTAINS lower_field = Arel::Nodes::NamedFunction.new('LOWER', [arel_attr]) - @query = query_aggregator(aggregator, - @collection.model.where(lower_field.matches("%#{value.to_s.downcase}%"))) + @collection.model.where(lower_field.matches("%#{value.to_s.downcase}%")) when Operators::NOT_CONTAINS - @query = query_aggregator(aggregator, - @collection.model.where.not(arel_attr.matches("%#{value}%"))) + @collection.model.where.not(arel_attr.matches("%#{value}%")) when Operators::NOT_I_CONTAINS lower_field = Arel::Nodes::NamedFunction.new('LOWER', [arel_attr]) - @query = query_aggregator(aggregator, - @collection.model.where.not(lower_field.matches("%#{value.to_s.downcase}%"))) + @collection.model.where.not(lower_field.matches("%#{value.to_s.downcase}%")) when Operators::STARTS_WITH - @query = query_aggregator(aggregator, - @collection.model.where(arel_attr.matches("#{value}%"))) + @collection.model.where(arel_attr.matches("#{value}%")) when Operators::I_STARTS_WITH lower_field = Arel::Nodes::NamedFunction.new('LOWER', [arel_attr]) - @query = query_aggregator(aggregator, - @collection.model.where(lower_field.matches("#{value.to_s.downcase}%"))) + @collection.model.where(lower_field.matches("#{value.to_s.downcase}%")) when Operators::ENDS_WITH - @query = query_aggregator(aggregator, - @collection.model.where(arel_attr.matches("%#{value}"))) + @collection.model.where(arel_attr.matches("%#{value}")) when Operators::I_ENDS_WITH lower_field = Arel::Nodes::NamedFunction.new('LOWER', [arel_attr]) - @query = query_aggregator(aggregator, - @collection.model.where(lower_field.matches("%#{value.to_s.downcase}"))) + @collection.model.where(lower_field.matches("%#{value.to_s.downcase}")) when Operators::LIKE - @query = query_aggregator(aggregator, @collection.model.where(arel_attr.matches(value))) + @collection.model.where(arel_attr.matches(value)) when Operators::I_LIKE lower_field = Arel::Nodes::NamedFunction.new('LOWER', [arel_attr]) - @query = query_aggregator(aggregator, - @collection.model.where(lower_field.matches(value.to_s.downcase))) + @collection.model.where(lower_field.matches(value.to_s.downcase)) when Operators::MATCH # Match operator supports: # - Regexp objects from pattern transformations: Regexp.new("pattern") @@ -167,18 +159,16 @@ def compute_main_operator(condition_tree, aggregator) raise ArgumentError, "Match operator is not supported for database adapter '#{adapter_name}'" end - @query = query_aggregator(aggregator, @collection.model.where(regex_clause, pattern)) + @collection.model.where(regex_clause, pattern) when Operators::INCLUDES_ALL - @query = query_aggregator(aggregator, @collection.model.where(arel_attr.matches_all(value))) + @collection.model.where(arel_attr.matches_all(value)) when Operators::SHORTER_THAN length_func = Arel::Nodes::NamedFunction.new('LENGTH', [arel_attr]) - @query = query_aggregator(aggregator, @collection.model.where(length_func.lt(value))) + @collection.model.where(length_func.lt(value)) when Operators::LONGER_THAN length_func = Arel::Nodes::NamedFunction.new('LENGTH', [arel_attr]) - @query = query_aggregator(aggregator, @collection.model.where(length_func.gt(value))) + @collection.model.where(length_func.gt(value)) end - - @query end def build_select(collection, projection) diff --git a/packages/forest_admin_datasource_active_record/spec/lib/forest_admin_datasource_active_record/utils/query_spec.rb b/packages/forest_admin_datasource_active_record/spec/lib/forest_admin_datasource_active_record/utils/query_spec.rb index 1de5322d6..6559669eb 100644 --- a/packages/forest_admin_datasource_active_record/spec/lib/forest_admin_datasource_active_record/utils/query_spec.rb +++ b/packages/forest_admin_datasource_active_record/spec/lib/forest_admin_datasource_active_record/utils/query_spec.rb @@ -375,6 +375,66 @@ module Utils end end + describe 'nested condition branches' do + let(:nodes) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Nodes } + let(:operators) { ForestAdminDatasourceToolkit::Components::Query::ConditionTree::Operators } + + def sql_for(condition_tree) + query_builder = described_class.new(collection, nil, Filter.new(condition_tree: condition_tree)) + query_builder.build + query_builder.query.to_sql + end + + # Car's default_scope (id > 10) is still applied to filtered queries, as on main. + it 'groups an Or nested in an And, so a search cannot widen the other conditions' do + sql = sql_for(nodes::ConditionTreeBranch.new('And', [ + nodes::ConditionTreeLeaf.new('id', operators::NOT_EQUAL, 3), + nodes::ConditionTreeBranch.new('Or', [ + nodes::ConditionTreeLeaf.new('brand', operators::CONTAINS, 'a'), + nodes::ConditionTreeLeaf.new('model', operators::CONTAINS, 'b') + ]) + ])) + + expect(sql).to end_with( + 'WHERE (id > 10) AND "cars"."id" != 3 AND ("cars"."brand" LIKE \'%a%\' OR "cars"."model" LIKE \'%b%\')' + ) + end + + it 'groups an And nested in an Or' do + sql = sql_for(nodes::ConditionTreeBranch.new('Or', [ + nodes::ConditionTreeLeaf.new('id', operators::EQUAL, 1), + nodes::ConditionTreeBranch.new('And', [ + nodes::ConditionTreeLeaf.new('brand', operators::EQUAL, 'a'), + nodes::ConditionTreeLeaf.new('model', operators::EQUAL, 'b') + ]) + ])) + + expect(sql).to end_with( + 'WHERE (id > 10) AND ("cars"."id" = 1 OR "cars"."brand" = \'a\' AND "cars"."model" = \'b\')' + ) + end + + it 'keeps the join of a related field used inside a nested Or' do + sql = sql_for(nodes::ConditionTreeBranch.new('And', [ + nodes::ConditionTreeLeaf.new('id', operators::NOT_EQUAL, 3), + nodes::ConditionTreeBranch.new('Or', [ + nodes::ConditionTreeLeaf.new('brand', operators::CONTAINS, 'a'), + nodes::ConditionTreeLeaf.new('category:label', operators::CONTAINS, 'b') + ]) + ])) + + expect(sql).to include('LEFT OUTER JOIN "categories"') + expect(sql).to end_with( + 'WHERE (id > 10) AND "cars"."id" != 3 AND ("cars"."brand" LIKE \'%a%\' OR "categories"."label" LIKE \'%b%\')' + ) + end + + it 'matches nothing on an empty Or and everything on an empty And' do + expect(sql_for(nodes::ConditionTreeBranch.new('Or', []))).to end_with('WHERE (id > 10) AND (1 = 0)') + expect(sql_for(nodes::ConditionTreeBranch.new('And', []))).to end_with('WHERE (id > 10) AND (1 = 1)') + end + end + describe 'sorting on related fields' do it 'uses the related table name for sorting' do sort = [{ field: 'category:label', ascending: true }]