[PostVersions] Reimplement Query as subclass of ElasticQueryBuilder (#772)

app/controllers/post_versions_controller.rb

@@ -6,7 +6,7 @@ class PostVersionsController < ApplicationController
   respond_to :js, only: [:undo]
 
   def index
-    @post_versions = PostVersion.document_store.search(PostVersion.build_query(search_params)).paginate(params[:page], limit: params[:limit], max_count: 10_000, search_count: params[:search], includes: [:updater, post: [:versions]])
+    @post_versions = PostVersion.search(search_params).paginate(params[:page], limit: params[:limit], max_count: 10_000, search_count: params[:search], includes: [:updater, post: [:versions]])
     respond_with(@post_versions)
   end
 

app/logical/elastic_post_version_query_builder.rb

@@ -0,0 +1,65 @@
+# frozen_string_literal: true
+
+class ElasticPostVersionQueryBuilder < ElasticQueryBuilder
+  def model_class
+    PostVersion
+  end
+
+  def build
+    add_range_relation(:id, :id)
+
+    if q[:updater_name].present?
+      user_id = User.name_to_id(q[:updater_name])
+      must.push({ term: { updater_id: user_id } }) if user_id
+    end
+
+    add_range_relation(:updater_id, :updater_id)
+    add_range_relation(:post_id, :post_id)
+
+    if q[:rating].present?
+      must.concat(q[:rating].split(",").map { |x| { term: { rating: x.to_s.downcase[0] } } })
+    end
+
+    if q[:rating_changed].present?
+      if q[:rating_changed] != "any"
+        must.push({ term: { rating: q[:rating_changed] } })
+      end
+      must.push({ term: { rating_changed: true } })
+    end
+
+    add_range_relation(:parent_id, :parent_id)
+
+    if q[:parent_id_changed].present?
+      if q[:parent_id_changed].is_a?(Integer)
+        must.push({ term: { parent_id: q[:parent_id_changed] } })
+      else
+        must.push({ exists: { field: :parent_id } })
+      end
+      must.push({ term: { parent_id_changed: true } })
+    end
+
+    %i[tags tags_removed tags_added locked_tags locked_tags_removed locked_tags_added].each do |tag_field|
+      tags = q[tag_field]
+      if tags
+        must.concat(TagQuery.scan(tags.downcase).map { |tag| { term: { tag_field => tag } } })
+      end
+    end
+
+    add_range_relation(:updated_at, :updated_at, type: :date)
+    add_text_match(:reason, :reason)
+    add_text_match(:description, :description)
+
+    %i[description_changed source_changed].each do |flag|
+      add_boolean_match(flag, flag)
+    end
+
+    if q[:uploads].present?
+      case q[:uploads].downcase
+      when "excluded"
+        must_not.push({ term: { version: 1 } })
+      when "only"
+        must.push({ term: { version: 1 } })
+      end
+    end
+  end
+end

app/logical/elastic_query_builder.rb

@@ -25,12 +25,14 @@ class ElasticQueryBuilder
         should: should,
       },
     }
+
     query[:bool][:minimum_should_match] = 1 if should.any?
 
     if @function_score.present?
       @function_score[:query] = query
       query = { function_score: @function_score }
     end
+
     search_body = {
       query: query,
       sort: order,
@@ -116,4 +118,42 @@ class ElasticQueryBuilder
       should.push(match_none({ exists: { field: index_field } }))
     end
   end
+
+  def add_boolean_match(key, index_field)
+    return if q[key].blank?
+    if q[key].to_s.truthy?
+      must.push({ term: { index_field => true } })
+    elsif q[key].to_s.falsy?
+      must.push({ term: { index_field => false } })
+    else
+      raise ArgumentError, "value must be truthy or falsy"
+    end
+  end
+
+  def add_range_relation(key, index_field, type: :integer)
+    if q[key].present?
+      must.push(range_relation(ParseValue.range(q[key], type), index_field))
+    end
+  end
+
+  def add_text_match(key, index_field)
+    value = q[key]
+    return if value.blank?
+    must.push(match: { index_field => value })
+  end
+
+  def apply_basic_order(key: :order)
+    case q[key]
+    when "id_asc"
+      order.push({ id: { order: "asc" } })
+    when "id_desc"
+      order.push({ id: { order: "desc" } })
+    else
+      apply_default_order
+    end
+  end
+
+  def apply_default_order
+    order.push({ id: { order: "desc" } })
+  end
 end

app/models/post_version.rb

@@ -18,143 +18,8 @@ class PostVersion < ApplicationRecord
       end
     end
 
-    def should(*args)
+    def search(params)
-      { bool: { should: args } }
+      ElasticPostVersionQueryBuilder.new(params).search
-    end
-
-    def split_to_terms(field, input)
-      input.split(",").map(&:to_i).map { |x| { term: { field => x } } }
-    end
-
-    def tag_list(field, input, target)
-      if input.present?
-        target += TagQuery.scan(input.downcase).map { |x| { term: { field => x } } }
-      end
-      target
-    end
-
-    def to_rating(input)
-      input.to_s.downcase[0]
-    end
-
-    def build_query(params)
-      must = []
-      must_not = []
-
-      if params[:updater_name].present?
-        user_id = User.name_to_id(params[:updater_name])
-        must << { term: { updater_id: user_id } } if user_id
-      end
-
-      if params[:updater_id].present?
-        must << should(*split_to_terms(:updater_id, params[:updater_id]))
-      end
-
-      if params[:post_id].present?
-        must << should(*split_to_terms(:post_id, params[:post_id]))
-      end
-
-      if params[:start_id].present?
-        must << { range: { id: {gte: params[:start_id].to_i } } }
-      end
-
-      if params[:rating].present?
-        must << { term: { rating: to_rating(params[:rating]) } }
-      end
-
-      if params[:rating_changed].present?
-        if params[:rating_changed] != "any"
-          must << { term: { rating: to_rating(params[:rating_changed]) } }
-        end
-        must << { term: { rating_changed: true } }
-      end
-
-      if params[:parent_id].present?
-        must << { term: { parent_id: params[:parent_id].to_i } }
-      end
-
-      if params[:parent_id_changed].present?
-        must << { term: { parent_id: params[:parent_id_changed].to_i } }
-        must << { term: { parent_id_changed: true } }
-      end
-
-      must = tag_list(:tags, params[:tags], must)
-      must = tag_list(:tags_removed, params[:tags_removed], must)
-      must = tag_list(:tags_added, params[:tags_added], must)
-      must = tag_list(:locked_tags, params[:locked_tags], must)
-      must = tag_list(:locked_tags_removed, params[:locked_tags_removed], must)
-      must = tag_list(:locked_tags_added, params[:locked_tags_added], must)
-
-      if params[:updated_at].present?
-        updated = range_relation(ParseValue.range(params[:updated_at], :date), :updated_at)
-        must << updated if updated.present?
-      end
-
-      if params[:reason].present?
-        must << { match: { reason: params[:reason] } }
-      end
-
-      if params[:description].present?
-        must << { match: { description: params[:description] } }
-      end
-
-      must = boolean_match(:description_changed, params[:description_changed], must)
-      must = boolean_match(:source_changed, params[:source_changed], must)
-
-      if params[:uploads].present?
-        if params[:uploads].downcase == "excluded"
-          must_not << { term: { version: 1 } }
-        elsif params[:uploads].downcase == "only"
-          must << { term: { version: 1 } }
-        end
-      end
-
-      if must.empty?
-        must.push({ match_all: {} })
-      end
-
-      {
-        query: { bool: { must: must, must_not: must_not } },
-        sort: { id: :desc },
-        _source: false,
-        timeout: "#{CurrentUser.user.try(:statement_timeout) || 3_000}ms"
-      }
-    end
-
-    def range_relation(arr, field)
-      return if arr.nil?
-      return if arr.size < 2
-      return if arr[1].nil?
-
-      case arr[0]
-      when :eq
-        if arr[1].is_a?(Time)
-          { range: { field => { gte: arr[1].beginning_of_day, lte: arr[1].end_of_day } } }
-        else
-          { term: { field => arr[1] } }
-        end
-      when :gt
-        { range: { field => { gt: arr[1] } } }
-      when :gte
-        { range: { field => { gte: arr[1] } } }
-      when :lt
-        { range: { field => { lt: arr[1] } } }
-      when :lte
-        { range: { field => { lte: arr[1] } } }
-      when :in
-        { terms: { field => arr[1] } }
-      when :between
-        { range: { field => { gte: arr[1], lte: arr[2] } } }
-      end
-    end
-
-    def boolean_match(attribute, value, must)
-      if value&.truthy?
-        must << { term: { attribute => true } }
-      elsif value&.falsy?
-        must << { term: { attribute => false } }
-      end
-      must
     end
   end