diff --git a/CHANGELOG.md b/CHANGELOG.md index 5905edc1..22d09a2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### 2.14.0 + * Making flat_query an includable module for implementing filter classes + * Adds a new method for filter classes with `FlatQueryTools` included. `can_use_flat_query?` which uses smart defaults and can be overridden + * branched the standard `get_query` method to go to `get_flat_query` or the sub-select `get_complex_query` based on whether module is included and `can_use_flat_query?` is set to true ### 2.13.8 * Each flat-query condition now loops to more deeply support nested attributes. For each nest a new join param will be added to the query * simplified some of the variable usage and params being passed around to remove things unnecessary. diff --git a/Gemfile.lock b/Gemfile.lock index 9ba2de0f..77d9d939 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - refine-rails (2.13.8) + refine-rails (2.14.0) rails (>= 6.0) GEM @@ -105,7 +105,7 @@ GEM mocha (2.7.1) ruby2_keywords (>= 0.0.5) mysql2 (0.5.6) - net-imap (0.5.6) + net-imap (0.5.8) date net-protocol net-pop (0.1.2) diff --git a/app/models/refine/filter.rb b/app/models/refine/filter.rb index d46b039f..4850b489 100644 --- a/app/models/refine/filter.rb +++ b/app/models/refine/filter.rb @@ -6,7 +6,6 @@ class Filter include Stabilize include Internationalized include Inspector - include FlatQueryTools # This validation structure sents `initial_query` as the method to validate against define_model_callbacks :initialize, only: [:after] after_initialize :valid? @@ -92,10 +91,10 @@ def valid_query? def get_query raise "Initial query must exist" if initial_query.nil? - if blueprint.present? - @relation.where(group(make_sub_query(blueprint))) + if self.class.included_modules.include?(Refine::FlatQueryTools) && can_use_flat_query? + get_flat_query else - @relation + get_complex_query end end @@ -105,6 +104,14 @@ def get_query! result end + def get_complex_query + if blueprint.present? + @relation.where(group(make_sub_query(blueprint))) + else + @relation + end + end + def add_nodes_to_query(subquery:, nodes:, query_method:) # Apply existing nodes to existing subquery if subquery.present? && nodes.present? @@ -213,6 +220,7 @@ def group(nodes) def apply_condition(criterion) begin + condition = get_condition_for_criterion(criterion) get_condition_for_criterion(criterion)&.apply(criterion[:input], table, initial_query, false, nil) rescue Refine::Conditions::Errors::ConditionClauseError => e e.errors.each do |error| diff --git a/app/models/refine/flat_query_tools.rb b/app/models/refine/flat_query_tools.rb index 009b07bf..7386a634 100644 --- a/app/models/refine/flat_query_tools.rb +++ b/app/models/refine/flat_query_tools.rb @@ -47,7 +47,16 @@ def construct_flat_query else unless condition_already_applied?(criteria_or_conjunction) node = apply_flat_condition(criteria_or_conjunction) - @relation = @relation.where(Arel.sql(node.to_sql)) + if node.is_a?(Arel::Nodes::Grouping) + @relation = @relation.where(node) + elsif node.is_a?(Array) + error = node.filter{ |n| n.is_a?(ActiveModel::Error) }.first + if error + filter.errors.add(:base, error.full_message) + end + else + @relation = @relation.where(Arel::Nodes::Grouping.new(node)) + end track_condition_applied(criteria_or_conjunction) end end @@ -84,7 +93,6 @@ def apply_pending_joins end join_count += 1 end - end end @@ -100,5 +108,10 @@ def condition_already_applied?(criterion) applied_conditions[criterion[:condition_id]] && applied_conditions[criterion[:condition_id]].include?(criterion[:input]) end + + # Meant to be overridden by Filter classes to add or replace with custom logic + def can_use_flat_query? + !uses_or? && !uses_negative_clause? && !has_duplicate_conditions? + end end end \ No newline at end of file diff --git a/lib/refine/rails/version.rb b/lib/refine/rails/version.rb index 0e4e99ef..24d32e4d 100644 --- a/lib/refine/rails/version.rb +++ b/lib/refine/rails/version.rb @@ -1,5 +1,5 @@ module Refine module Rails - VERSION = "2.13.8" + VERSION = "2.14.0" end end diff --git a/package.json b/package.json index 6d59e8a8..60ee732a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@clickfunnels/refine-stimulus", - "version": "2.13.8", + "version": "2.14.0", "description": "Refine is a flexible query builder for your apps. It lets your users filter down to exactly what they're looking for. Completely configured on the backend.", "browserslist": [ "defaults", 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..74545338 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 @@ -91,7 +91,7 @@ def create_filter(blueprint=nil) tag_options = [{id: "1", display: "tag1"}, {id: "2", display: "tag2"}, {id: "3", display: "tag3"}, {id: "4", display: "tag4"}] event_source_options = [{id: "1", display: "source1"}, {id: "2", display: "source2"}, {id: "5", display: "source5"}] - BlankTestFilter.new(blueprint, + FlatQueryTestFilter.new(blueprint, Contact.all, [ Refine::Conditions::TextCondition.new("text_field_value"), diff --git a/test/refine/models/filters/flat_query_tools/flat_query_forms_submissions_db_test.rb b/test/refine/models/filters/flat_query_tools/flat_query_forms_submissions_db_test.rb index c65ee25d..fff8f1b4 100644 --- a/test/refine/models/filters/flat_query_tools/flat_query_forms_submissions_db_test.rb +++ b/test/refine/models/filters/flat_query_tools/flat_query_forms_submissions_db_test.rb @@ -28,7 +28,7 @@ SELECT DISTINCT `contacts`.* FROM `contacts` INNER JOIN `forms_submissions` ON `forms_submissions`.`id` = `contacts`.`custom_attributes_id` INNER JOIN `forms_submissions_answers` ON `forms_submissions_answers`.`submission_id` = `forms_submissions`.`id` - WHERE ((`forms_submissions_answers`.`entry` = 'test')) + WHERE (`forms_submissions_answers`.`entry` = 'test') SQL assert_equal expected_sql, filter.get_flat_query.to_sql end @@ -41,7 +41,7 @@ SELECT DISTINCT `contacts`.* FROM `contacts` INNER JOIN `forms_submissions` ON `forms_submissions`.`id` = `contacts`.`custom_attributes_id` INNER JOIN `forms_submissions_answers` ON `forms_submissions_answers`.`submission_id` = `forms_submissions`.`id` - WHERE ((`forms_submissions_answers`.`fields_option_id` IN (2, 3))) + WHERE (`forms_submissions_answers`.`fields_option_id` IN (2, 3)) SQL assert_equal expected_sql, filter.get_flat_query.to_sql end @@ -55,7 +55,7 @@ INNER JOIN `forms_submissions` ON `forms_submissions`.`id` = `contacts`.`custom_attributes_id` INNER JOIN `forms_submissions_answers` ON `forms_submissions_answers`.`submission_id` = `forms_submissions`.`id` INNER JOIN `forms_submissions_answers_selected_options` ON `forms_submissions_answers_selected_options`.`answer_id` = `forms_submissions_answers`.`id` - WHERE ((`forms_submissions_answers_selected_options`.`fields_option_id` IN (1, 2))) + WHERE (`forms_submissions_answers_selected_options`.`fields_option_id` IN (1, 2)) SQL assert_equal expected_sql, filter.get_flat_query.to_sql end @@ -69,7 +69,7 @@ INNER JOIN `forms_submissions` ON `forms_submissions`.`id` = `contacts`.`custom_attributes_id` INNER JOIN `forms_submissions_answers` ON `forms_submissions_answers`.`submission_id` = `forms_submissions`.`id` INNER JOIN `contacts_applied_tags` ON `contacts_applied_tags`.`contact_id` = `contacts`.`id` - WHERE ((`forms_submissions_answers`.`fields_option_id` IN (2, 3))) AND ((`contacts_applied_tags`.`tag_id` IN (1, 2))) + WHERE (`forms_submissions_answers`.`fields_option_id` IN (2, 3)) AND (`contacts_applied_tags`.`tag_id` IN (1, 2)) SQL assert_equal expected_sql, filter.get_flat_query.to_sql end @@ -78,7 +78,7 @@ def create_filter(blueprint=nil) field_options = [{id: "1", display: "field1"}, {id: "2", display: "field2"}, {id: "3", display: "field3"}, {id: "4", display: "field4"}] selected_options = [{id: "1", display: "selected1"}, {id: "2", display: "selected2"}, {id: "3", display: "selected3"}, {id: "4", display: "selected4"}] - BlankTestFilter.new(blueprint, + FlatQueryTestFilter.new(blueprint, Contact.all, [ Refine::Conditions::TextCondition.new("custom_attributes.answers.entry"), 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..c0dcb820 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 @@ -44,7 +44,7 @@ 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, + FlatQueryTestFilter.new(blueprint, Contact.all, [ Refine::Conditions::TextCondition.new("text_field_value"), 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..607bf0fa 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 @@ -26,7 +26,7 @@ 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)) + WHERE (`contacts_applied_tags`.`tag_id` IN (1, 2)) AND (`contacts_applied_tags`.`tag_id` = 4) SQL assert_equal expected_sql, filter.get_flat_query.to_sql end @@ -35,7 +35,7 @@ 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, + FlatQueryTestFilter.new(blueprint, Contact.all, [ Refine::Conditions::TextCondition.new("text_field_value"), diff --git a/test/refine/models/filters/flat_query_tools_test.rb b/test/refine/models/filters/flat_query_tools_test.rb index 2ee88a2e..ee5a9580 100644 --- a/test/refine/models/filters/flat_query_tools_test.rb +++ b/test/refine/models/filters/flat_query_tools_test.rb @@ -2,6 +2,7 @@ require "support/refine/test_double_filter" require "support/refine/contact_complex_relationships" require "refine/invalid_filter_error" +require 'mocha/minitest' describe Refine::Filter do include FilterTestHelper @@ -43,7 +44,7 @@ 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` = 1)) + WHERE (`contacts_applied_tags`.`tag_id` = 1) SQL assert_equal expected_sql, filter.get_flat_query.to_sql end @@ -55,80 +56,24 @@ 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` = 1)) + WHERE (`contacts_applied_tags`.`tag_id` = 1) SQL assert_equal expected_sql, filter.get_flat_query.to_sql end end end - def grouped_blueprint - Refine::Blueprints::Blueprint.new - .criterion("text_field_value", - clause: Refine::Conditions::TextCondition::CLAUSE_EQUALS, - value: "one",) - .and - .group { - criterion("text_field_value", - clause: Refine::Conditions::TextCondition::CLAUSE_EQUALS, - value: "two",) - .and - .criterion("text_field_value", - clause: Refine::Conditions::TextCondition::CLAUSE_EQUALS, - value: "three",) - } - end - - def nested_group_blueprint - Refine::Blueprints::Blueprint.new - .criterion("text_field_value", - clause: Refine::Conditions::TextCondition::CLAUSE_EQUALS, - value: "one",) - .and - .group { - group { - criterion("text_field_value", - clause: Refine::Conditions::TextCondition::CLAUSE_EQUALS, - value: "two",) - .and - .criterion("text_field_value", - clause: Refine::Conditions::TextCondition::CLAUSE_EQUALS, - value: "three",) - } - .and - .criterion("text_field_value", - clause: Refine::Conditions::TextCondition::CLAUSE_EQUALS, - value: "four",) - } - .and - .criterion("text_field_value", - clause: Refine::Conditions::TextCondition::CLAUSE_EQUALS, - value: "five") - end - def create_filter(blueprint=nil) - BlankTestFilter.new(blueprint, + FlatQueryTestFilter.new(blueprint, Contact.all, [ Refine::Conditions::TextCondition.new("text_field_value"), - Refine::Conditions::OptionCondition.new("tags.id").with_options(proc { [{id: "1", display: "tag1"}] }) + Refine::Conditions::OptionCondition.new("tags.id").with_options(proc { [{id: "1", display: "tag1"}, {id: "2", display: "tag2"}, {id: "3", display: "tag3"}, {id: "4", display: "tag4"}] }) ], Contact.arel_table) end - def bad_id - [{ - depth: 0, - type: "criterion", - condition_id: "fake", - input: { - clause: "eq", - value: "aaron" - } - }] - end - def single_tag_blueprint [{ depth: 0, @@ -153,42 +98,6 @@ def single_condition_blueprint }] end - def invalid_condition_blueprint - [{ - depth: 0, - type: "criterion", - condition_id: "invalid_condition", - input: { - clause: "eq", - value: "invalid" - } - }] - end - - def and_condition_blueprint - [{ # criterion aaron and aa - depth: 0, - type: "criterion", - condition_id: "text_field_value", - input: { - clause: "eq", - value: "aaron" - } - }, { # conjunction - depth: 0, - type: "conjunction", - word: "and" - }, { # criterion - depth: 0, - type: "criterion", - condition_id: "text_field_value", - input: { - clause: "eq", - value: "aa" - } - }] - end - def or_condition_blueprint [{ # criterion aaron OR aa depth: 0, @@ -213,57 +122,58 @@ def or_condition_blueprint }] end - def grouped_or_blueprint - [{ - type: "criterion", - condition_id: "user_name", - depth: 1, - input: { - clause: "cont", - value: "Aaron" - } - }, - { - type: "conjunction", - word: "and", - depth: 1 - }, - { - type: "criterion", - condition_id: "user_name", - depth: 1, - input: { - clause: "cont", - value: "Francis" - } - }, - { - type: "conjunction", - word: "or", - depth: 0 - }, - { - type: "criterion", - condition_id: "user_name", - depth: 1, - input: { - clause: "cont", - value: "Sean" + describe "flat query fallback and selection" do + it "falls back to complex query if OR is present" do + # Setup a blueprint with an OR conjunction + blueprint = [ + { + depth: 0, + type: "criterion", + condition_id: "tags.id", + input: { + clause: "in", + selected: ["1", "2"] + } + }, + { + depth: 0, + type: "conjunction", + word: "or" + }, + { + depth: 0, + type: "criterion", + condition_id: "tags.id", + input: { + clause: "eq", + selected: ["4"] + } } - }, - { - type: "conjunction", - word: "and", - depth: 1 - }, - { - type: "criterion", - condition_id: "user_name", - depth: 1, - input: { - clause: "cont", - value: "Fioritto" + ] + filter = create_filter(blueprint) + # Force can_use_get_query to return false for this test + filter.stubs(:can_use_get_query?).returns(false) + # Should use the complex query logic, not flat + assert_equal filter.get_complex_query.to_sql, filter.get_query.to_sql + end + + it "uses flat query for simple AND tags filter" do + blueprint = [ + { + depth: 0, + type: "criterion", + condition_id: "tags.id", + input: { + clause: "in", + selected: ["1", "2"] + } } - }] + ] + filter = create_filter(blueprint) + # Force can_use_get_query to return true for this test + filter.stubs(:can_use_get_query?).returns(true) + # Should use the flat query logic + assert_equal filter.get_flat_query.to_sql, filter.get_query.to_sql + end end end diff --git a/test/support/refine/filter_test_helper.rb b/test/support/refine/filter_test_helper.rb index bfb7590a..cac0ca93 100644 --- a/test/support/refine/filter_test_helper.rb +++ b/test/support/refine/filter_test_helper.rb @@ -1,5 +1,6 @@ require "support/refine/blank_test_filter" require "support/refine/cf2_blank_test_filter" +require "support/refine/flat_query_test_filter" module FilterTestHelper class TestDouble < ActiveRecord::Base diff --git a/test/support/refine/flat_query_test_filter.rb b/test/support/refine/flat_query_test_filter.rb new file mode 100644 index 00000000..26f07281 --- /dev/null +++ b/test/support/refine/flat_query_test_filter.rb @@ -0,0 +1,18 @@ +require "support/refine/filter_test_helper" + +class FlatQueryTestFilter < Refine::Filter + include Refine::FlatQueryTools + attr_accessor :conditions + + def t(key, options = {}) + I18n.t("#{key}", options) + end + + def initialize(blueprint = nil, initial_query = FilterTestHelper::TestDouble.all, conditions = nil, table = FilterTestHelper::TestDouble.arel_table) + @table = table + @conditions = conditions + super(blueprint, initial_query) + end + + attr_reader :table +end