From 364042f12722a68199aeb310189652058d61e3b4 Mon Sep 17 00:00:00 2001 From: Mateusz Luterek Date: Thu, 10 Jul 2025 16:04:37 +0100 Subject: [PATCH] fix: include through associations referenced in filters in ResourcesGetter --- app/services/forest_liana/resources_getter.rb | 23 +++++ spec/requests/resources_spec.rb | 98 +++++++++++++++++++ .../forest_liana/resources_getter_spec.rb | 52 ++++++++++ 3 files changed, 173 insertions(+) diff --git a/app/services/forest_liana/resources_getter.rb b/app/services/forest_liana/resources_getter.rb index d39183de..a9e721fe 100644 --- a/app/services/forest_liana/resources_getter.rb +++ b/app/services/forest_liana/resources_getter.rb @@ -347,9 +347,32 @@ def compute_select_fields end end + associations_from_filters.each do |association| + select << "#{@resource.table_name}.#{association.foreign_key}" + end + select.uniq end + def associations_from_filters + return [] unless filters_fields + + filters_fields + .select { |field| field.include?(':') } + .map { |field| field.split(':').first } + .map { |field| get_one_association(field) } + .compact + end + + def filters_fields + return unless @params[:filters] + + parsed_filters = JSON.parse(@params[:filters]) + # Different structure when multiple filters are applied + parsed_filters = parsed_filters['conditions'] if parsed_filters['conditions'] + Array.wrap(parsed_filters).map { |filter| filter['field'] }.compact.uniq + end + def get_one_association(name) ForestLiana::QueryHelper.get_one_associations(@resource) .select { |association| association.name == name.to_sym } diff --git a/spec/requests/resources_spec.rb b/spec/requests/resources_spec.rb index f5b850e7..a808eeae 100644 --- a/spec/requests/resources_spec.rb +++ b/spec/requests/resources_spec.rb @@ -177,6 +177,104 @@ }) end end + + describe 'with a single filter on a through association that is not a displayed column when also including an unrelated association' do + params = { + fields: { 'Tree' => 'id,name,owner' }, + filters: JSON.generate({ + field: 'location:coordinates', + operator: 'equal', + value: '1,2' + }), + page: { 'number' => '1', 'size' => '10' }, + searchExtended: '0', + sort: '-id', + timezone: 'Europe/Paris' + } + + it 'should respond 200' do + get '/forest/Tree', params: params, headers: headers + expect(response.status).to eq(200) + end + + it 'should respond with the tree data' do + get '/forest/Tree', params: params, headers: headers + expect(JSON.parse(response.body)).to match({ + "data" => [{ + "type" => "Tree", + "id" => "1", + "attributes" => { + "id" => 1, + "name" => "Lemon Tree" + }, + "links" => { + "self" => "/forest/tree/1" + }, + "relationships" => { + "owner" => { + "data" => { "id" => "1", "type" => "User" }, + "links" => { "related" => {} } + } + } + }], + "included" => be_a(Array) + }) + end + end + + describe 'with multiple filters on a through association that is not a displayed column when also including an unrelated association' do + params = { + fields: { 'Tree' => 'id,name,owner' }, + filters: JSON.generate({ + aggregator: "or", + conditions: [ + { + field: 'location:coordinates', + operator: 'equal', + value: '1,2' + }, + { + field: 'location:coordinates', + operator: 'equal', + value: '2,3' + } + ] + }), + page: { 'number' => '1', 'size' => '10' }, + searchExtended: '0', + sort: '-id', + timezone: 'Europe/Paris' + } + + it 'should respond 200' do + get '/forest/Tree', params: params, headers: headers + expect(response.status).to eq(200) + end + + it 'should respond with the tree data' do + get '/forest/Tree', params: params, headers: headers + expect(JSON.parse(response.body)).to match({ + "data" => [{ + "type" => "Tree", + "id" => "1", + "attributes" => { + "id" => 1, + "name" => "Lemon Tree" + }, + "links" => { + "self" => "/forest/tree/1" + }, + "relationships" => { + "owner" => { + "data" => { "id" => "1", "type" => "User" }, + "links" => { "related" => {} } + } + } + }], + "included" => be_a(Array) + }) + end + end end end diff --git a/spec/services/forest_liana/resources_getter_spec.rb b/spec/services/forest_liana/resources_getter_spec.rb index 78d2a696..a17ae4d3 100644 --- a/spec/services/forest_liana/resources_getter_spec.rb +++ b/spec/services/forest_liana/resources_getter_spec.rb @@ -306,6 +306,58 @@ def association_connection.current_database end end + describe 'when filtering on a field of a through association without including the field in the fields param and when also including an unrelated association' do + let(:resource) { Tree } + let(:fields) { { 'Tree' => 'id,name,owner' } } + let(:filters) { { + field: 'location:coordinates', + operator: 'equal', + value: '12345' + }.to_json } + + it 'should get only the expected records' do + getter.perform + records = getter.records + count = getter.count + + expect(records.count).to eq 2 + expect(count).to eq 2 + expect(records.map(&:id)).to eq [1, 2] + expect(records.map(&:name)).to eq ['Lemon Tree', 'Ginger Tree'] + end + end + + describe 'when filtering with multiple conditions on a field of a through association without including the field in the fields param and when also including an unrelated association' do + let(:resource) { Tree } + let(:fields) { { 'Tree' => 'id,name,owner' } } + let(:filters) { { + aggregator: "or", + conditions: [ + { + field: 'location:coordinates', + operator: 'equal', + value: '12345' + }, + { + field: 'location:coordinates', + operator: 'equal', + value: '54321' + } + ] + }.to_json } + + it 'should get only the expected records' do + getter.perform + records = getter.records + count = getter.count + + expect(records.count).to eq 3 + expect(count).to eq 3 + expect(records.map(&:id)).to eq [1, 2, 3] + expect(records.map(&:name)).to eq ['Lemon Tree', 'Ginger Tree', 'Apple Tree'] + end + end + describe 'when filtering on an ambiguous field' do let(:resource) { Tree } let(:pageSize) { 5 }