Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add test to cover double join errors #571

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions app/services/forest_liana/filters_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ def apply_filters
@joins.each do |join|
current_resource = @resource.reflect_on_association(join.name).klass
current_resource.include(ArelHelpers::Aliases)
current_resource.aliased_as(join.name) do |aliased_resource|
@resource = @resource.joins(ArelHelpers.join_association(@resource, join.name, Arel::Nodes::OuterJoin, aliases: [aliased_resource]))
current_resource.aliased_as("#{join.name}_#{@resource.table_name}") do |aliased_resource|
@resource = @resource.joins(ArelHelpers.join_association(@resource, join.name, Arel::Nodes::OuterJoin, { aliases: [aliased_resource] }))
end
end

Expand Down Expand Up @@ -174,14 +174,16 @@ def parse_field_name(field)
current_resource = @resource.reflect_on_association(field.split(':').first.to_sym)&.klass
raise ForestLiana::Errors::HTTP422Error.new("Field '#{field}' not found") unless current_resource

association = get_association_name_for_condition(field)
quoted_table_name = ActiveRecord::Base.connection.quote_column_name(association)
association = get_association_for_condition(field)

quoted_table_name = ActiveRecord::Base.connection.quote_table_name("#{association.name}_#{@resource.table_name}")
field_name = field.split(':')[1]
else
quoted_table_name = @resource.quoted_table_name
current_resource = @resource
field_name = field
end

quoted_field_name = ActiveRecord::Base.connection.quote_column_name(field_name)

column_found = current_resource.columns.find { |column| column.name == field.split(':').last }
Expand All @@ -196,15 +198,15 @@ def is_belongs_to(field)
field.include?(':')
end

def get_association_name_for_condition(field)
def get_association_for_condition(field)
field, subfield = field.split(':')

association = @resource.reflect_on_association(field.to_sym)
return nil if association.blank?

@joins << association unless @joins.include? association

association.name
association
end

# NOTICE: Look for a previous interval condition matching the following:
Expand Down
1 change: 1 addition & 0 deletions app/services/forest_liana/resources_getter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ def compute_includes
associations_has_one = ForestLiana::QueryHelper.get_one_associations(@resource)

includes = associations_has_one.map(&:name)

includes_for_smart_search = []

if @collection && @collection.search_fields
Expand Down
7 changes: 2 additions & 5 deletions app/services/forest_liana/search_query_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,10 @@ def detect_reference(param)
ref, field = param.split('.')

if ref && field
association = @resource.reflect_on_all_associations
.find {|a| a.name == ref.to_sym }

referenced_table = association ? association_table_name(association.name) : ref
association = @resource.reflect_on_all_associations().find {|a| a.name == ref.to_sym }

ForestLiana::AdapterHelper
.format_column_name(referenced_table, field)
.format_column_name("#{association.name}_#{@resource.table_name}", field)
else
param
end
Expand Down
34 changes: 34 additions & 0 deletions spec/services/forest_liana/resources_getter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ def init_scopes
let(:sort) { 'owner.name' }

it 'should get only the expected records' do
sql_query = getter.perform.to_sql
p sql_query

getter.perform
records = getter.records
count = getter.count
Expand All @@ -153,6 +156,37 @@ def init_scopes
expect(count).to eq 5
expect(records.map(&:id)).to eq [1, 4, 5, 3, 2]
end

it 'should include associated table only once' do
sql_query = getter.perform.to_sql
location_includes_count = sql_query.scan('LEFT OUTER JOIN "locations"').count
expect(location_includes_count).to eq(1)
end

context 'when filters is given' do
let(:filters) { {
field: 'location:id',
operator: 'equal',
value: 1,
}.to_json }

it 'should get only the expected records' do
getter.perform
records = getter.records
count = getter.count

expect(records.count).to eq 1
expect(count).to eq 1
expect(records.map(&:id)).to eq [1]
end

it 'should include associated table only once' do
sql_query = getter.perform.to_sql
p sql_query
location_includes_count = sql_query.scan('LEFT OUTER JOIN "locations"').count
expect(location_includes_count).to eq(1)
end
end
end

describe 'when getting instance dependent associations' do
Expand Down