Merge branch 'develop' into art/1738/activerecord-caching

Author: Oaphi

app/helpers/search_helper.rb

@@ -1,4 +1,6 @@
 module SearchHelper
+  include SearchQualifierHelper
+
   ##
   # Search & sort a default posts list based on parameters in the current request.
   #
@@ -129,86 +131,46 @@ def parse_search(raw_search)
     { qualifiers: qualifiers, search: search }
   end
 
-  # rubocop:disable Metrics/CyclomaticComplexity
-
   ##
   # Parses a full qualifier string into an array of qualifier objects.
   # @param qualifiers [String] A qualifier string as returned by {#parse_search}.
   # @return [Array<Hash{Symbol => Object}>]
   def parse_qualifier_strings(qualifiers)
-    valid_value = {
-      date: /^[<>=]{0,2}[\d.]+(?:s|m|h|d|w|mo|y)?$/,
-      status: /any|open|closed/,
-      numeric: /^[<>=]{0,2}[\d.]+$/
-    }
-
-    qualifiers.map do |qualifier| # rubocop:disable Metrics/BlockLength
+    qualifiers.map do |qualifier|
       splat = qualifier.split ':'
       parameter = splat[0]
       value = splat[1]
 
-      case parameter
-      when 'score'
-        next unless value.match?(valid_value[:numeric])
-
-        operator, val = numeric_value_sql value
-        { param: :score, operator: operator.presence || '=', value: val.to_f }
-      when 'created'
-        next unless value.match?(valid_value[:date])
-
-        operator, val, timeframe = date_value_sql value
-        { param: :created, operator: operator.presence || '=', timeframe: timeframe, value: val.to_i }
-      when 'user'
-        operator, val = if value.match?(valid_value[:numeric])
-                          numeric_value_sql value
-                        elsif value == 'me'
-                          ['=', current_user&.id&.to_i]
-                        else
-                          next
-                        end
-
-        { param: :user, operator: operator.presence || '=', user_id: val }
-      when 'upvotes'
-        next unless value.match?(valid_value[:numeric])
-
-        operator, val = numeric_value_sql value
-        { param: :upvotes, operator: operator.presence || '=', value: val.to_i }
-      when 'downvotes'
-        next unless value.match?(valid_value[:numeric])
-
-        operator, val = numeric_value_sql value
-        { param: :downvotes, operator: operator.presence || '=', value: val.to_i }
-      when 'votes'
-        next unless value.match?(valid_value[:numeric])
-
-        operator, val = numeric_value_sql value
-        { param: :net_votes, operator: operator.presence || '=', value: val.to_i }
-      when 'tag'
-        { param: :include_tag, tag_id: Tag.where(name: value).select(:id) }
-      when '-tag'
-        { param: :exclude_tag, tag_id: Tag.where(name: value).select(:id) }
-      when 'category'
-        next unless value.match?(valid_value[:numeric])
-
-        operator, val = numeric_value_sql value
-        { param: :category, operator: operator.presence || '=', category_id: val.to_i }
-      when 'post_type'
-        next unless value.match?(valid_value[:numeric])
-
-        operator, val = numeric_value_sql value
-        { param: :post_type, operator: operator.presence || '=', post_type_id: val.to_i }
-      when 'answers'
-        next unless value.match?(valid_value[:numeric])
-
-        operator, val = numeric_value_sql value
-        { param: :answers, operator: operator.presence || '=', value: val.to_i }
-      when 'status'
-        next unless value.match?(valid_value[:status])
-
-        { param: :status, value: value }
-      end
+      parsed = case parameter
+               when 'score'
+                 parse_score_qualifier(value)
+               when 'created'
+                 parse_created_qualifier(value)
+               when 'user'
+                 parse_user_qualifier(value)
+               when 'upvotes'
+                 parse_upvotes_qualifier(value)
+               when 'downvotes'
+                 parse_downvotes_qualifier(value)
+               when 'votes'
+                 parse_votes_qualifier(value)
+               when 'tag'
+                 parse_include_tag_qualifier(value)
+               when '-tag'
+                 parse_exclude_tag_qualifier(value)
+               when 'category'
+                 parse_category_qualifier(value)
+               when 'post_type'
+                 parse_post_type_qualifier(value)
+               when 'answers'
+                 parse_answers_qualifier(value)
+               when 'status'
+                 parse_status_qualifier(value)
+               end
+
+      parsed
     end.compact
-    # Consider partitioning and telling the user which filters were invalid
+    # Consider telling the user which filters were invalid
   end
 
   ##
@@ -266,49 +228,4 @@ def qualifiers_to_sql(qualifiers, query, user)
 
     query
   end
-  # rubocop:enable Metrics/CyclomaticComplexity
-
-  ##
-  # Parses a qualifier value string, including operator, as a numeric value.
-  # @param value [String] The value part of the qualifier, i.e. +">=10"+
-  # @return [Array(String, String)] A 2-tuple containing operator and value.
-  # @api private
-  def numeric_value_sql(value)
-    operator = ''
-    while ['<', '>', '='].include? value[0]
-      operator += value[0]
-      value = value[1..-1]
-    end
-
-    # whatever's left after stripping operator is the number
-    # validated by regex in qualifiers_to_sql
-    [operator, value]
-  end
-
-  ##
-  # Parses a qualifier value string, including operator, as a date value.
-  # @param value [String] The value part of the qualifier, i.e. +">=10d"+
-  # @return [Array(String, String, String)] A 3-tuple containing operator, value, and timeframe.
-  # @api private
-  def date_value_sql(value)
-    operator = ''
-
-    while ['<', '>', '='].include? value[0]
-      operator += value[0]
-      value = value[1..-1]
-    end
-
-    # working with dates: <1y ('less than one year ago') is SQL: > 1y ago
-    operator = { '<' => '>', '>' => '<', '<=' => '>=', '>=' => '<=' }[operator] || ''
-
-    val = ''
-    while value[0] =~ /[[:digit:]]/
-      val += value[0]
-      value = value[1..-1]
-    end
-
-    timeframe = { s: 'SECOND', m: 'MINUTE', h: 'HOUR', d: 'DAY', w: 'WEEK', mo: 'MONTH', y: 'YEAR' }[value.to_sym]
-
-    [operator, val, timeframe || 'MONTH']
-  end
 end

app/helpers/search_qualifier_helper.rb

@@ -0,0 +1,157 @@
+module SearchQualifierHelper
+  def matches_date?(value)
+    value.match?(/^[<>=]{0,2}[\d.]+(?:s|m|h|d|w|mo|y)?$/)
+  end
+
+  def matches_id?(value)
+    value.match?(/^[<>=]{0,2}\d+$/)
+  end
+
+  def matches_int?(value)
+    value.match?(/^[<>=]{0,2}-?\d+$/)
+  end
+
+  def matches_numeric?(value)
+    value.match?(/^[<>=]{0,2}[\d.]+$/)
+  end
+
+  def matches_non_negative_int?(value)
+    value.match?(/^[<>=]{0,2}\d+$/)
+  end
+
+  def matches_status?(value)
+    value.match?(/any|open|closed/)
+  end
+
+  def parse_answers_qualifier(value)
+    return unless matches_non_negative_int?(value)
+
+    operator, val = numeric_value_sql(value)
+
+    { param: :answers, operator: operator.presence || '=', value: val.to_i }
+  end
+
+  def parse_category_qualifier(value)
+    return unless matches_id?(value)
+
+    operator, val = numeric_value_sql(value)
+
+    { param: :category, operator: operator.presence || '=', category_id: val.to_i }
+  end
+
+  def parse_created_qualifier(value)
+    return unless matches_date?(value)
+
+    operator, val, timeframe = date_value_sql(value)
+
+    { param: :created, operator: operator.presence || '=', timeframe: timeframe, value: val.to_i }
+  end
+
+  def parse_downvotes_qualifier(value)
+    return unless matches_non_negative_int?(value)
+
+    operator, val = numeric_value_sql(value)
+
+    { param: :downvotes, operator: operator.presence || '=', value: val.to_i }
+  end
+
+  def parse_exclude_tag_qualifier(value)
+    { param: :exclude_tag, tag_id: Tag.where(name: value).select(:id) }
+  end
+
+  def parse_post_type_qualifier(value)
+    return unless matches_id?(value)
+
+    operator, val = numeric_value_sql(value)
+
+    { param: :post_type, operator: operator.presence || '=', post_type_id: val.to_i }
+  end
+
+  def parse_score_qualifier(value)
+    return unless matches_numeric?(value)
+
+    operator, val = numeric_value_sql(value)
+
+    { param: :score, operator: operator.presence || '=', value: val.to_f }
+  end
+
+  def parse_status_qualifier(value)
+    return unless matches_status?(value)
+
+    { param: :status, value: value }
+  end
+
+  def parse_include_tag_qualifier(value)
+    { param: :include_tag, tag_id: Tag.where(name: value).select(:id) }
+  end
+
+  def parse_upvotes_qualifier(value)
+    return unless matches_non_negative_int?(value)
+
+    operator, val = numeric_value_sql(value)
+
+    { param: :upvotes, operator: operator.presence || '=', value: val.to_i }
+  end
+
+  def parse_user_qualifier(value)
+    return unless matches_int?(value) || value == 'me'
+
+    operator, val = if value == 'me'
+                      ['=', current_user&.id]
+                    else
+                      numeric_value_sql(value)
+                    end
+
+    { param: :user, operator: operator.presence || '=', user_id: val.to_i }
+  end
+
+  def parse_votes_qualifier(value)
+    return unless matches_int?(value)
+
+    operator, val = numeric_value_sql(value)
+
+    { param: :net_votes, operator: operator.presence || '=', value: val.to_i }
+  end
+
+  # Parses a qualifier value string, including operator, as a numeric value.
+  # @param value [String] The value part of the qualifier, i.e. +">=10"+
+  # @return [Array(String, String)] A 2-tuple containing operator and value.
+  # @api private
+  def numeric_value_sql(value)
+    operator = ''
+    while ['<', '>', '='].include? value[0]
+      operator += value[0]
+      value = value[1..-1]
+    end
+
+    # whatever's left after stripping operator is the number
+    # validated by regex in qualifiers_to_sql
+    [operator, value]
+  end
+
+  # Parses a qualifier value string, including operator, as a date value.
+  # @param value [String] The value part of the qualifier, i.e. +">=10d"+
+  # @return [Array(String, String, String)] A 3-tuple containing operator, value, and timeframe.
+  # @api private
+  def date_value_sql(value)
+    operator = ''
+
+    while ['<', '>', '='].include? value[0]
+      operator += value[0]
+      value = value[1..-1]
+    end
+
+    # working with dates: <1y ('less than one year ago') is SQL: > 1y ago
+    operator = { '<' => '>', '>' => '<', '<=' => '>=', '>=' => '<=' }[operator] || ''
+
+    val = ''
+    while value[0] =~ /[[:digit:]]/
+      val += value[0]
+      value = value[1..-1]
+    end
+
+    timeframe = { s: 'SECOND', m: 'MINUTE', h: 'HOUR', d: 'DAY', w: 'WEEK', mo: 'MONTH', y: 'YEAR' }[value.to_sym]
+
+    [operator, val, timeframe || 'MONTH']
+  end
+end

test/controllers/comments/create_test.rb

@@ -97,7 +97,7 @@ class CommentsControllerTest < ActionController::TestCase
     end
   end
 
-  test 'should correclty create comments in threads' do
+  test 'should correctly create comments in threads' do
     sign_in users(:editor)
     before_author_notifs = users(:standard_user).notifications.count
     before_follow_notifs = users(:deleter).notifications.count

test/helpers/search_helper_test.rb

@@ -13,29 +13,6 @@ class SearchHelperTest < ActionView::TestCase
     end
   end
 
-  test 'numeric_value_sql should return correct operator and value' do
-    expected = {
-      '12345' => ['', '12345'],
-      '<12345' => ['<', '12345'],
-      '>=12345' => ['>=', '12345']
-    }
-    expected.each do |input, expect|
-      assert_equal expect, numeric_value_sql(input)
-    end
-  end
-
-  test 'date_value_sql should return correct operator, value, and timeframe' do
-    expected = {
-      '1' => ['', '1', 'MONTH'],
-      '1y' => ['', '1', 'YEAR'],
-      '<1y' => ['>', '1', 'YEAR'],
-      '>=2w' => ['<=', '2', 'WEEK']
-    }
-    expected.each do |input, expect|
-      assert_equal expect, date_value_sql(input)
-    end
-  end
-
   test 'qualifiers_to_sql should correctly narrow by :category qualifier' do
     main = categories(:main)
     admin_only = categories(:admin_only)