diff --git a/app/models/refine/conditions/condition.rb b/app/models/refine/conditions/condition.rb index dfa6929d..7d85d232 100644 --- a/app/models/refine/conditions/condition.rb +++ b/app/models/refine/conditions/condition.rb @@ -148,12 +148,13 @@ def apply(input, table, initial_query, inverse_clause=false, through_attribute=n input.delete(:filter_refinement) end + # No longer a relationship attribute, apply condition normally + self.attribute = through_attribute.to_s if through_attribute if is_relationship_attribute? apply_relationship_attribute(input: input, query: initial_query) return end - # No longer a relationship attribute, apply condition normally - self.attribute = through_attribute if through_attribute + nodes = apply_condition(input, table, inverse_clause) if !is_refinement && has_any_refinements? refined_node = apply_refinements(input) diff --git a/app/models/refine/conditions/supports_flat_queries.rb b/app/models/refine/conditions/supports_flat_queries.rb index 69f63830..606431fc 100644 --- a/app/models/refine/conditions/supports_flat_queries.rb +++ b/app/models/refine/conditions/supports_flat_queries.rb @@ -14,9 +14,10 @@ module SupportsFlatQueries # @param [Arel::Table] table The arel_table the query is built on # @param [ActiveRecord::Relation] initial_query The base query the query is built on # @param [Bool] inverse_clause Whether to invert the clause + # @param [Bool] apply_condition_on_join Whether to apply the condition on the join instead of the root query # @return [Arel::Node] # This is mostly a copy of the `apply` method from the Condition module, but avoids recursion and the pending_subquery system - def apply_flat(input, table, initial_query, inverse_clause=false) + def apply_flat(input, table, initial_query, inverse_clause=false, apply_condition_on_join=false) table ||= filter.table @is_flat_query = true # Ensurance validations are checking the developer configured correctly @@ -40,7 +41,7 @@ def apply_flat(input, table, initial_query, inverse_clause=false) end if is_relationship_attribute? - return handle_flat_relational_condition(input: input, table: table, query: initial_query, inverse_clause: inverse_clause) + return handle_flat_relational_condition(input: input, table: table, query: initial_query, inverse_clause: inverse_clause, apply_condition_on_join: apply_condition_on_join) end # Not a relationship attribute, apply condition normally nodes = apply_condition(input, table, inverse_clause) @@ -52,10 +53,12 @@ def apply_flat(input, table, initial_query, inverse_clause=false) nodes end - def handle_flat_relational_condition(input:, table:, query:, inverse_clause:) + def handle_flat_relational_condition(input:, table:, query:, inverse_clause:, apply_condition_on_join: false) model_class = query.model condition_joins = [] + condition_nodes = nil while @attribute.include?(".") + puts "Attribute: #{@attribute}" forced_id = false # Split on first . decompose_attribute = @attribute.split(".", 2) @@ -64,12 +67,13 @@ def handle_flat_relational_condition(input:, table:, query:, inverse_clause:) # Relation to be handled relation = decompose_attribute[0] + puts "RELATION: #{relation}" + puts "Reflecting #{model_class} on #{relation}" instance = model_class.reflect_on_association(relation.to_sym) if @attribute == "id" # We're referencing a primary key ID, so we dont need the final join table and # can just reference the foreign key of the previous step in the relation chain - model_class = instance.class_name.safe_constantize through_reflection = get_through_reflection(instance: instance, relation: relation) unless condition_uses_different_database?(through_reflection.klass, query.model) forced_id = true @@ -87,14 +91,52 @@ def handle_flat_relational_condition(input:, table:, query:, inverse_clause:) end end model_class = instance.class_name.safe_constantize + puts "MODEL CLASS: #{model_class}" end # End of while loop + if condition_joins.any? - add_pending_joins_if_needed(input: input, joins_array: condition_joins) - end + # Copied from apply_flat_relational_condition - need to refactor + if through_reflection + # If through reflection is passed in (due to an association only referencing the id) + # use that to get the table and class + relation_table_being_queried = through_reflection.klass.arel_table + relation_class = through_reflection.klass + else + # Otherwise, use the instance to get the table and class. + relation_table_being_queried = instance.class_name.safe_constantize&.arel_table + relation_class = instance.class_name.safe_constantize + end + + instance = get_reflection_object(query, relation) if forced_id + + key_1 = key_1(instance) + key_2 = key_2(instance) - apply_flat_relational_condition(instance: instance, relation: relation, through_reflection: through_reflection, input: input, query: query, inverse_clause: inverse_clause, forced_id: forced_id) + # Ensure we're aliasing the table for the WHERE clause if the condition is used more than once. + if apply_condition_on_join + pending_join_value = filter.pending_joins[relation_table_being_queried.table_name] || {} + current_join_count = pending_join_value[:count] || 0 + alias_name = "#{relation_table_being_queried.table_name}_#{current_join_count + 1}" + relation_table_being_queried = relation_table_being_queried.alias(alias_name) + end + + if forced_id + condition_nodes = apply(input, relation_table_being_queried, relation_class, inverse_clause, key_2) + else + condition_nodes = apply(input, relation_table_being_queried, relation_class, inverse_clause) + end + # end of copied code + root_level_condition_to_add = add_pending_joins_if_needed(input: input, joins_array: condition_joins, through_reflection: through_reflection, on_condition: condition_nodes) + # If the condition is used just once, we need to apply the condition normally + unless apply_condition_on_join + return apply_flat_relational_condition(instance: instance, relation: relation, through_reflection: through_reflection, input: input, query: query, inverse_clause: inverse_clause, forced_id: forced_id) + end + root_level_condition_to_add + else + apply_flat_relational_condition(instance: instance, relation: relation, through_reflection: through_reflection, input: input, query: query, inverse_clause: inverse_clause, forced_id: forced_id) + end end # apply_flat_relational_condition @@ -176,28 +218,73 @@ def get_foreign_key_from_relation(instance:, reflection:) child_foreign_key end - def add_pending_join(joins_array, join_type=:inner) - relation = joins_array.first - joins_block = joins_array.reverse.inject({}) { |a, n| { n.to_sym => a } } - # If we already are tracking the relation with a left joins, don't overwrite it - unless join_type == :inner && filter.pending_joins[relation] && filter.pending_joins[relation][:type] == :left - filter.needs_distinct = true - filter.pending_joins[relation] = { type: join_type, joins_block: joins_block}.compact + def add_pending_join(joins_array:, join_type: :inner, on_condition:, through_reflection: nil) + current_model = filter.model + left_table = current_model.arel_table + filter.needs_distinct = true + + joins_array.each_with_index do |relation, idx| + puts "Applying Join chain: #{relation} - #{idx} - #{current_model}" + reflection = current_model.reflect_on_association(relation.to_sym) + raise "Association #{relation} not found on #{current_model}" unless reflection + + base_table = reflection.klass.table_name + filter.pending_joins[base_table] ||= { count: 0, nodes: [], aliased_nodes: [] joins_block: nil } + filter.pending_joins[base_table][:count] += 1 + join_count = filter.pending_joins[base_table][:count] + alias_name = "#{base_table}_#{join_count}" + right_table = Arel::Table.new(base_table).alias(alias_name) + + # Determine join keys + parent_key = reflection.active_record_primary_key + foreign_key = reflection.foreign_key + + # The child table (right_table) always has the foreign key + bridge_condition = right_table[foreign_key].eq(left_table[parent_key]) + + # Only apply on_condition to the last join + full_condition = if idx == joins_array.length - 1 && on_condition + bridge_condition.and(on_condition) + else + bridge_condition + end + + join_class = join_type == :left ? Arel::Nodes::OuterJoin : Arel::Nodes::InnerJoin + if left_table.is_a?(Arel::Nodes::TableAlias) + join_node = Arel::Nodes::InnerJoin.new( + left_table, + Arel::Nodes::On.new( + left_table[parent_key].eq(right_table[foreign_key]) + ).on(full_condition) + .join_sources + ) + else + join_node = left_table.join(right_table, join_class) + .on(full_condition) + .join_sources + end + + filter.pending_joins[base_table][:nodes] << join_node + + # Prepare for next iteration + left_table = right_table + current_model = reflection.klass end + + # Should return nil because we've already added the condition to the query and do not need to add it later + nil end - def add_pending_joins_if_needed(input:, joins_array:) + def add_pending_joins_if_needed(input:, joins_array:, on_condition: nil, through_reflection: nil) # Determine if we need to do left-joins due to the clause needing to include null values - if(input && LEFT_JOIN_CLAUSES.include?(input[:clause])) - add_pending_join(joins_array, :left) - else - add_pending_join(joins_array, :inner) - end + join_type = (input && LEFT_JOIN_CLAUSES.include?(input[:clause])) ? :left : :inner + add_pending_join(joins_array: joins_array, join_type: join_type, on_condition: on_condition, through_reflection: through_reflection) end def condition_uses_different_database?(current_model, parent_model) # Are the queries on different databases? parent_model.connection_db_config.configuration_hash != current_model.connection_db_config.configuration_hash end + end end \ No newline at end of file diff --git a/app/models/refine/flat_query_tools.rb b/app/models/refine/flat_query_tools.rb index 009b07bf..04e84723 100644 --- a/app/models/refine/flat_query_tools.rb +++ b/app/models/refine/flat_query_tools.rb @@ -3,14 +3,14 @@ # NOTE: This is more specialized query construction and it is up to the implementer to use the inspector tools to ensure this is only being used for supported queries module Refine module FlatQueryTools - attr_accessor :pending_joins, :applied_conditions, :needs_distinct + attr_accessor :pending_joins, :applied_conditions, :needs_distinct, :condition_counts def pending_joins @pending_joins ||= {} end - def applied_conditions - @applied_conditions ||= {} + def condition_counts + @condition_counts ||= {} end def needs_distinct? @@ -37,6 +37,7 @@ def get_flat_query! # It is meant to be idempotent hence it checks for already applied conditions def construct_flat_query groups = [] + build_condition_counts blueprint.each do |criteria_or_conjunction| if criteria_or_conjunction[:type] == "conjunction" if criteria_or_conjunction[:word] == "or" @@ -45,10 +46,9 @@ def construct_flat_query @applied_conditions = {} end else - unless condition_already_applied?(criteria_or_conjunction) - node = apply_flat_condition(criteria_or_conjunction) - @relation = @relation.where(Arel.sql(node.to_sql)) - track_condition_applied(criteria_or_conjunction) + node = apply_flat_condition(criteria_or_conjunction) + if node + @relation = @relation.where(node) end end end @@ -64,7 +64,8 @@ def construct_flat_query # Same as Filter.apply_condition but uses `supports_flat_queries` helpers instead of default path def apply_flat_condition(criterion) begin - get_condition_for_criterion(criterion)&.apply_flat(criterion[:input], table, initial_query, false) + condition = get_condition_for_criterion(criterion) + condition&.apply_flat(criterion[:input], table, initial_query, false, should_apply_condition_on_join?(condition)) rescue Refine::Conditions::Errors::ConditionClauseError => e e.errors.each do |error| errors.add(:base, error.full_message, criterion_uid: criterion[:uid]) @@ -73,32 +74,46 @@ def apply_flat_condition(criterion) end # Called at the end of the filter's construct_flat_query. Applies joins from pending_joins hash constructed by individual conditions + # If the same joins occurs twice, we need to apply the extra clauses to the joins AND use aliases def apply_pending_joins - if pending_joins.present? - join_count = 0 - pending_joins.each do |relation, join_data| - if join_data[:type] == :left - @relation = @relation.left_joins(join_data[:joins_block]).distinct + return if pending_joins.blank? + + pending_joins.each_value do |data| + if data[:count] > 1 + data[:nodes].each do |join_node_array| + join_node_array.each do |join_node| + puts "Applying join node: #{join_node} for dupe" + @relation = @relation.joins(join_node).distinct + end + end + else + if data[:joins_block].present? + puts "Applying joins block: #{data[:joins_block]} - single" + @relation = @relation.joins(data[:joins_block]).distinct else - @relation = @relation.joins(join_data[:joins_block]).distinct + puts "Applying join node: #{data[:nodes]} - single" + @relation = @relation.joins(data[:nodes]).distinct end - join_count += 1 end - end end - def track_condition_applied(criterion) - if applied_conditions[criterion[:condition_id]].nil? - applied_conditions[criterion[:condition_id]] = [criterion[:input]] - else - applied_conditions[criterion[:condition_id]] << criterion[:input] + def build_condition_counts + blueprint.each do |criterion_or_conjunction| + if criterion_or_conjunction[:type] == "criterion" + increment_condition_count(get_condition_for_criterion(criterion_or_conjunction)) + end end end - def condition_already_applied?(criterion) - applied_conditions[criterion[:condition_id]] && - applied_conditions[criterion[:condition_id]].include?(criterion[:input]) + def increment_condition_count(condition_object) + condition_counts[condition_object.attribute] ||= 0 + condition_counts[condition_object.attribute] += 1 + end + + def should_apply_condition_on_join?(condition_object) + return false if condition_counts[condition_object.attribute].blank? + condition_counts[condition_object.attribute] > 1 end end end \ No newline at end of file diff --git a/test/refine/models/filters/flat_query_tools/flat_query_complex_test.rb b/test/refine/models/filters/flat_query_tools/flat_query_complex_test.rb new file mode 100644 index 00000000..1828bf11 --- /dev/null +++ b/test/refine/models/filters/flat_query_tools/flat_query_complex_test.rb @@ -0,0 +1,264 @@ +require "test_helper" +require "support/refine/test_double_filter" +require "support/refine/contact_complex_relationships" +require "refine/invalid_filter_error" + +describe Refine::Filter do + include FilterTestHelper + + around do |test| + ApplicationRecord.connection.execute("CREATE TABLE contacts (id bigint primary key, text_field_value varchar(255));") + ApplicationRecord.connection.execute("CREATE TABLE contacts_applied_tags (id bigint primary key, contact_id bigint, tag_id bigint);") + ApplicationRecord.connection.execute("CREATE TABLE contacts_tags (id bigint primary key);") + ApplicationRecord.connection.execute("CREATE TABLE contacts_last_activities (id bigint primary key);") + ApplicationRecord.connection.execute("CREATE TABLE products (id bigint primary key);") + ApplicationRecord.connection.execute("CREATE TABLE orders (id bigint primary key, contact_id bigint, service_status varchar(255));") + ApplicationRecord.connection.execute("CREATE TABLE orders_line_items (id bigint primary key, order_id bigint, original_product_id bigint);") + ApplicationRecord.connection.execute("CREATE TABLE forms_submissions (id bigint primary key, contact_id bigint, form_id bigint, submitted_at datetime);") + test.call + ApplicationRecord.connection.execute("DROP TABLE contacts, contacts_applied_tags, contacts_tags, contacts_last_activities, products, orders, orders_line_items, forms_submissions;") + end + + describe "get_flat_query" do + it "handles complex query with multiple conditions and joins" do + initial_query = Contact.all + filter = create_filter(complex_criteria) + + expected_sql = <<-SQL.squish + SELECT DISTINCT `contacts`.* FROM `contacts` + INNER JOIN `contacts_applied_tags` `contacts_applied_tags_1` ON `contacts_applied_tags_1`.`contact_id` = `contacts`.`id` AND `contacts_applied_tags_1`.`tag_id` IN (1, 2) + INNER JOIN `contacts_applied_tags` `contacts_applied_tags_2` ON `contacts_applied_tags_2`.`contact_id` = `contacts`.`id` AND `contacts_applied_tags_2`.`tag_id` = 4 + INNER JOIN `orders` ON `orders`.`contact_id` = `contacts`.`id` + INNER JOIN `orders_line_items` ON `orders_line_items`.`order_id` = `orders`.`id` + WHERE (`contacts`.`text_field_value` = 'aaron') + AND (`orders`.`service_status` = 'active') + AND (`orders_line_items`.`original_product_id` = 123) + SQL + assert_equal expected_sql, filter.get_flat_query.to_sql + end + + it "handles forms submissions with date range" do + initial_query = Contact.all + filter = create_filter(custom_attributes_criteria) + + expected_sql = <<-SQL.squish + SELECT DISTINCT `contacts`.* FROM `contacts` + INNER JOIN `forms_submissions` ON `forms_submissions`.`contact_id` = `contacts`.`id` + WHERE (`forms_submissions`.`form_id` = 1) + AND (`forms_submissions`.`submitted_at` >= '2023-01-01 00:00:00') + AND (`forms_submissions`.`submitted_at` <= '2023-12-31 23:59:59') + SQL + assert_equal expected_sql, filter.get_flat_query.to_sql + end + + it "handles multiple forms submissions with different conditions" do + initial_query = Contact.all + filter = create_filter(multiple_custom_attributes_criteria) + + expected_sql = <<-SQL.squish + SELECT DISTINCT `contacts`.* FROM `contacts` + INNER JOIN `forms_submissions` `forms_submissions_1` + ON `forms_submissions_1`.`contact_id` = `contacts`.`id` + AND `forms_submissions_1`.`form_id` = 1 + AND `forms_submissions_1`.`submitted_at` >= '2023-01-01 00:00:00' + INNER JOIN `forms_submissions` `forms_submissions_2` + ON `forms_submissions_2`.`contact_id` = `contacts`.`id` + AND `forms_submissions_2`.`form_id` = 2 + AND `forms_submissions_2`.`submitted_at` <= '2023-12-31 23:59:59' + SQL + assert_equal expected_sql, filter.get_flat_query.to_sql + end + end + + def create_filter(blueprint=nil) + tag_options = [{id: "1", display: "tag1"}, {id: "2", display: "tag2"}, {id: "3", display: "tag3"}, {id: "4", display: "tag4"}] + BlankTestFilter.new(blueprint, + Contact.all, + [ + Refine::Conditions::TextCondition.new("text_field_value"), + Refine::Conditions::OptionCondition.new("tags.id").with_options(proc { tag_options }), + Refine::Conditions::TextCondition.new("orders.service_status"), + Refine::Conditions::TextCondition.new("line_items.original_product_id"), + Refine::Conditions::TextCondition.new("custom_attributes.form_id"), + Refine::Conditions::DateCondition.new("custom_attributes.submitted_at") + ], + Contact.arel_table) + end + + def complex_criteria + [ + # First tag condition (IN) + { + depth: 0, + type: "criterion", + condition_id: "tags.id", + input: { + clause: "in", + selected: ["1", "2"] + } + }, + # AND conjunction + { + depth: 0, + type: "conjunction", + word: "and" + }, + # Second tag condition (EQ) + { + depth: 0, + type: "criterion", + condition_id: "tags.id", + input: { + clause: "eq", + selected: ["4"] + } + }, + # AND conjunction + { + depth: 0, + type: "conjunction", + word: "and" + }, + # Text field condition + { + depth: 0, + type: "criterion", + condition_id: "text_field_value", + input: { + clause: "eq", + value: "aaron" + } + }, + # AND conjunction + { + depth: 0, + type: "conjunction", + word: "and" + }, + # Order service status condition + { + depth: 0, + type: "criterion", + condition_id: "orders.service_status", + input: { + clause: "eq", + value: "active" + } + }, + # AND conjunction + { + depth: 0, + type: "conjunction", + word: "and" + }, + # Order line item condition + { + depth: 0, + type: "criterion", + condition_id: "line_items.original_product_id", + input: { + clause: "eq", + value: "123" + } + } + ] + end + + def custom_attributes_criteria + [ + { + depth: 0, + type: "criterion", + condition_id: "custom_attributes.form_id", + input: { + clause: "eq", + value: "1" + } + }, + { + depth: 0, + type: "conjunction", + word: "and" + }, + { + depth: 0, + type: "criterion", + condition_id: "custom_attributes.submitted_at", + input: { + clause: "gte", + date1: "2023-01-01" + } + }, + { + depth: 0, + type: "conjunction", + word: "and" + }, + { + depth: 0, + type: "criterion", + condition_id: "custom_attributes.submitted_at", + input: { + clause: "lte", + date1: "2023-12-31" + } + } + ] + end + + def multiple_custom_attributes_criteria + [ + { + depth: 0, + type: "criterion", + condition_id: "custom_attributes.form_id", + input: { + clause: "eq", + value: "1" + } + }, + { + depth: 0, + type: "conjunction", + word: "and" + }, + { + depth: 0, + type: "criterion", + condition_id: "custom_attributes.submitted_at", + input: { + clause: "gte", + date1: "2023-01-01" + } + }, + { + depth: 0, + type: "conjunction", + word: "and" + }, + { + depth: 0, + type: "criterion", + condition_id: "custom_attributes.form_id", + input: { + clause: "eq", + value: "2" + } + }, + { + depth: 0, + type: "conjunction", + word: "and" + }, + { + depth: 0, + type: "criterion", + condition_id: "custom_attributes.submitted_at", + input: { + clause: "lte", + date1: "2023-12-31" + } + } + ] + end +end \ No newline at end of file diff --git a/test/refine/models/filters/flat_query_tools/flat_query_cross_db_test.rb b/test/refine/models/filters/flat_query_tools/flat_query_cross_db_test.rb index 749325b0..d03c387d 100644 --- a/test/refine/models/filters/flat_query_tools/flat_query_cross_db_test.rb +++ b/test/refine/models/filters/flat_query_tools/flat_query_cross_db_test.rb @@ -31,7 +31,7 @@ expected_event_sql = <<-SQL.squish SELECT `events`.`contact_id` FROM `events` - WHERE (`events`.`source_id` IN (1, 2)) + WHERE ()`events`.`source_id` IN (1, 2)) SQL subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |name, start, finish, id, payload| @@ -52,7 +52,7 @@ expected_sql = <<-SQL.squish SELECT `contacts`.* FROM `contacts` - WHERE (`contacts`.`id` = 50) + WHERE `contacts`.`id` = 50 SQL assert_equal expected_sql, filter.get_flat_query.to_sql end @@ -66,7 +66,7 @@ expected_sql = <<-SQL.squish SELECT `contacts`.* FROM `contacts` - WHERE (`contacts`.`id` IN (50, 51)) + WHERE `contacts`.`id` IN (50, 51) SQL assert_equal expected_sql, filter.get_flat_query.to_sql end @@ -81,7 +81,7 @@ expected_sql = <<-SQL.squish SELECT `contacts`.* FROM `contacts` - WHERE (`contacts`.`id` IN (50, 51)) AND (`contacts`.`id` = 50) + WHERE `contacts`.`id` IN (50, 51) AND `contacts`.`id` = 50 SQL assert_equal expected_sql, filter.get_flat_query.to_sql end diff --git a/test/refine/models/filters/flat_query_tools/flat_query_last_activity_test.rb b/test/refine/models/filters/flat_query_tools/flat_query_last_activity_test.rb index 2ec15c66..7f03379c 100644 --- a/test/refine/models/filters/flat_query_tools/flat_query_last_activity_test.rb +++ b/test/refine/models/filters/flat_query_tools/flat_query_last_activity_test.rb @@ -23,7 +23,7 @@ expected_sql = <<-SQL.squish SELECT DISTINCT `contacts`.* FROM `contacts` INNER JOIN `contacts_last_activities` ON `contacts_last_activities`.`contact_id` = `contacts`.`id` - WHERE ((`contacts_last_activities`.`last_activity_at` BETWEEN '2020-01-01 00:00:00' AND '2020-01-01 23:59:59')) + WHERE (`contacts_last_activities`.`last_activity_at` BETWEEN '2020-01-01 00:00:00' AND '2020-01-01 23:59:59') SQL assert_equal expected_sql, filter.get_flat_query.to_sql end @@ -36,7 +36,7 @@ SELECT DISTINCT `contacts`.* FROM `contacts` INNER JOIN `contacts_last_activities` ON `contacts_last_activities`.`contact_id` = `contacts`.`id` INNER JOIN `contacts_applied_tags` ON `contacts_applied_tags`.`contact_id` = `contacts`.`id` - WHERE ((`contacts_last_activities`.`last_activity_at` BETWEEN '2020-01-01 00:00:00' AND '2020-01-01 23:59:59')) AND ((`contacts_applied_tags`.`tag_id` = 4)) + WHERE (`contacts_last_activities`.`last_activity_at` BETWEEN '2020-01-01 00:00:00' AND '2020-01-01 23:59:59') AND (`contacts_applied_tags`.`tag_id` = 4) SQL assert_equal expected_sql, filter.get_flat_query.to_sql end diff --git a/test/refine/models/filters/flat_query_tools/flat_query_tags_test.rb b/test/refine/models/filters/flat_query_tools/flat_query_tags_test.rb index 7d2ed9a4..a5187213 100644 --- a/test/refine/models/filters/flat_query_tools/flat_query_tags_test.rb +++ b/test/refine/models/filters/flat_query_tools/flat_query_tags_test.rb @@ -19,18 +19,29 @@ end describe "get_flat_query" do - it "two separaete criteria referencing tags generates a proper query" do + it "two separate criteria referencing tags generates a proper query" do initial_query = Contact.all filter = create_filter(two_tag_criteria) expected_sql = <<-SQL.squish SELECT DISTINCT `contacts`.* FROM `contacts` - INNER JOIN `contacts_applied_tags` ON `contacts_applied_tags`.`contact_id` = `contacts`.`id` - WHERE ((`contacts_applied_tags`.`tag_id` IN (1, 2))) AND ((`contacts_applied_tags`.`tag_id` = 4)) + INNER JOIN `contacts_applied_tags` `contacts_applied_tags_1` ON `contacts_applied_tags_1`.`contact_id` = `contacts`.`id` AND (`contacts_applied_tags_1`.`tag_id` IN (1, 2)) + INNER JOIN `contacts_applied_tags` `contacts_applied_tags_2` ON `contacts_applied_tags_2`.`contact_id` = `contacts`.`id` AND (`contacts_applied_tags_2`.`tag_id` = 4) SQL assert_equal expected_sql, filter.get_flat_query.to_sql end + it "single criteria referencing tags does not add alias" do + initial_query = Contact.all + filter = create_filter(single_tag_criteria) + + expected_sql = <<-SQL.squish + SELECT DISTINCT `contacts`.* FROM `contacts` + INNER JOIN `contacts_applied_tags` ON `contacts_applied_tags`.`contact_id` = `contacts`.`id` + WHERE (`contacts_applied_tags`.`tag_id` IN (1, 2)) + SQL + assert_equal expected_sql, filter.get_flat_query.to_sql + end end def create_filter(blueprint=nil) @@ -44,6 +55,18 @@ def create_filter(blueprint=nil) Contact.arel_table) end + def single_tag_criteria + [{ + depth: 0, + type: "criterion", + condition_id: "tags.id", + input: { + clause: "in", + selected: ["1", "2"] + } + }] + end + def two_tag_criteria [{ depth: 0, diff --git a/test/support/refine/contact_complex_relationships.rb b/test/support/refine/contact_complex_relationships.rb index 10e77731..d5225935 100644 --- a/test/support/refine/contact_complex_relationships.rb +++ b/test/support/refine/contact_complex_relationships.rb @@ -4,7 +4,7 @@ class Contact < ActiveRecord::Base has_many :tags, through: :applied_tags has_many :orders - has_many :line_items, through: :orders + has_many :line_items, through: :orders, class_name: "Orders::LineItem" has_many :products, through: :line_items, source: :original_product has_many :churned_line_items, -> { where(orders: {service_status: %w[churned canceled]}) }, through: :orders, source: :line_items has_many :churned_products, through: :churned_line_items, source: :original_product