Merge pull request #3697 from evazion/fix-3696

Fix #3696: API: handle boolean params consistently

app/logical/tag_alias_request.rb

@@ -75,10 +75,6 @@ class TagAliasRequest
   end
 
   def skip_secondary_validations=(v)
-    if v == "1" or v == true or v =~ /t/
-      @skip_secondary_validations = true
-    else
-      @skip_secondary_validations = false
-    end
+    @skip_secondary_validations = v.to_s.truthy?
   end
 end

app/logical/tag_implication_request.rb

@@ -75,10 +75,6 @@ class TagImplicationRequest
   end
 
   def skip_secondary_validations=(v)
-    if v == "1" or v == true or v =~ /t/
-      @skip_secondary_validations = true
-    else
-      @skip_secondary_validations = false
-    end
+    @skip_secondary_validations = v.to_s.truthy?
   end
 end

app/models/application_record.rb

@@ -5,10 +5,34 @@ class ApplicationRecord < ActiveRecord::Base
 
   concerning :SearchMethods do
     class_methods do
-      # range: "5", ">5", "<5", ">=5", "<=5", "5..10", "5,6,7"
-      def attribute_matches(attribute, range)
-        return all if range.blank?
+      def attribute_matches(attribute, value)
+        return all if value.nil?
 
+        column = column_for_attribute(attribute)
+        case column.sql_type_metadata.type
+        when :boolean
+          boolean_attribute_matches(attribute, value)
+        when :integer, :datetime
+          numeric_attribute_matches(attribute, value)
+        else
+          raise ArgumentError, "unhandled attribute type"
+        end
+      end
+
+      def boolean_attribute_matches(attribute, value)
+        if value.to_s.truthy?
+          value = true
+        elsif value.to_s.falsy?
+          value = false
+        else
+          raise ArgumentError, "value must be truthy or falsy"
+        end
+
+        where(attribute => value)
+      end
+
+      # range: "5", ">5", "<5", ">=5", "<=5", "5..10", "5,6,7"
+      def numeric_attribute_matches(attribute, range)
         column = column_for_attribute(attribute)
         qualified_column = "#{table_name}.#{column.name}"
         parsed_range = Tag.parse_helper(range, column.type)

app/models/artist.rb

@@ -566,17 +566,8 @@ class Artist < ApplicationRecord
         q = q.url_matches(params[:url_matches])
       end
 
-      if params[:is_active] == "true"
-        q = q.active
-      elsif params[:is_active] == "false"
-        q = q.deleted
-      end
-
-      if params[:is_banned] == "true"
-        q = q.banned
-      elsif params[:is_banned] == "false"
-        q = q.unbanned
-      end
+      q = q.attribute_matches(:is_active, params[:is_active])
+      q = q.attribute_matches(:is_banned, params[:is_banned])
 
       if params[:creator_name].present?
         q = q.where("artists.creator_id = (select _.id from users _ where lower(_.name) = ?)", params[:creator_name].tr(" ", "_").mb_chars.downcase)
@@ -587,13 +578,13 @@ class Artist < ApplicationRecord
       end
 
       # XXX deprecated, remove at some point.
-      if params[:empty_only] == "true"
+      if params[:empty_only].to_s.truthy?
         params[:has_tag] = "false"
       end
 
-      if params[:has_tag] == "true"
+      if params[:has_tag].to_s.truthy?
         q = q.joins(:tag).where("tags.post_count > 0")
-      elsif params[:has_tag] == "false"
+      elsif params[:has_tag].to_s.falsy?
         q = q.includes(:tag).where("tags.name IS NULL OR tags.post_count <= 0").references(:tags)
       end
 

app/models/artist_commentary.rb

@@ -41,15 +41,15 @@ class ArtistCommentary < ApplicationRecord
         q = q.where(post_id: params[:post_id].split(",").map(&:to_i))
       end
 
-      if params[:original_present] == "yes"
+      if params[:original_present].to_s.truthy?
         q = q.where("(original_title != '') or (original_description != '')")
-      elsif params[:original_present] == "no"
+      elsif params[:original_present].to_s.falsy?
         q = q.where("(original_title = '') and (original_description = '')")
       end
 
-      if params[:translated_present] == "yes"
+      if params[:translated_present].to_s.truthy?
         q = q.where("(translated_title != '') or (translated_description != '')")
-      elsif params[:translated_present] == "no"
+      elsif params[:translated_present].to_s.falsy?
         q = q.where("(translated_title = '') and (translated_description = '')")
       end
 
@@ -84,37 +84,17 @@ class ArtistCommentary < ApplicationRecord
   end
 
   def tag_post
-    if remove_commentary_tag == "1"
-      post.remove_tag("commentary")
-    end
+    post.remove_tag("commentary") if remove_commentary_tag.to_s.truthy?
+    post.add_tag("commentary") if add_commentary_tag.to_s.truthy?
 
-    if add_commentary_tag == "1"
-      post.add_tag("commentary")
-    end
+    post.remove_tag("commentary_request") if remove_commentary_request_tag.to_s.truthy?
+    post.add_tag("commentary_request") if add_commentary_request_tag.to_s.truthy?
 
-    if remove_commentary_request_tag == "1"
-      post.remove_tag("commentary_request")
-    end
+    post.remove_tag("check_commentary") if remove_commentary_check_tag.to_s.truthy?
+    post.add_tag("check_commentary") if add_commentary_check_tag.to_s.truthy?
 
-    if add_commentary_request_tag == "1"
-      post.add_tag("commentary_request")
-    end
-
-    if remove_commentary_check_tag == "1"
-      post.remove_tag("check_commentary")
-    end
-
-    if add_commentary_check_tag == "1"
-      post.add_tag("check_commentary")
-    end
-
-    if remove_partial_commentary_tag == "1"
-      post.remove_tag("partial_commentary")
-    end
-
-    if add_partial_commentary_tag == "1"
-      post.add_tag("partial_commentary")
-    end
+    post.remove_tag("partial_commentary") if remove_partial_commentary_tag.to_s.truthy?
+    post.add_tag("partial_commentary") if add_partial_commentary_tag.to_s.truthy?
 
     post.save if post.tag_string_changed?
   end

app/models/artist_version.rb

@@ -31,17 +31,8 @@ class ArtistVersion < ApplicationRecord
         q = q.where(artist_id: params[:artist_id].split(",").map(&:to_i))
       end
 
-      if params[:is_active] == "true"
-        q = q.where("is_active = true")
-      elsif params[:is_active] == "false"
-        q = q.where("is_active = false")
-      end
-
-      if params[:is_banned] == "true"
-        q = q.where("is_banned = true")
-      elsif params[:is_banned] == "false"
-        q = q.where("is_banned = false")
-      end
+      q = q.attribute_matches(:is_active, params[:is_active])
+      q = q.attribute_matches(:is_banned, params[:is_banned])
 
       params[:order] ||= params.delete(:sort)
       if params[:order] == "name"

app/models/ban.rb

@@ -47,10 +47,8 @@ class Ban < ApplicationRecord
       q = q.reason_matches(params[:reason_matches])
     end
 
-    case params[:expired]
-    when "true"  then q = q.expired
-    when "false" then q = q.unexpired
-    end
+    q = q.expired if params[:expired].to_s.truthy?
+    q = q.unexpired if params[:expired].to_s.falsy?
 
     case params[:order]
     when "expires_at_desc"

app/models/bulk_update_request.rb

@@ -205,10 +205,6 @@ class BulkUpdateRequest < ApplicationRecord
   end
 
   def skip_secondary_validations=(v)
-    if v == "1" or v == true
-      @skip_secondary_validations = true
-    else
-      @skip_secondary_validations = false
-    end
+    @skip_secondary_validations = v.to_s.truthy?
   end
 end

app/models/comment.rb

@@ -59,22 +59,6 @@ class Comment < ApplicationRecord
       where("comments.is_deleted = false")
     end
 
-    def sticky
-      where("comments.is_sticky = true")
-    end
-
-    def unsticky
-      where("comments.is_sticky = false")
-    end
-
-    def bumping
-      where("comments.do_not_bump_post = false")
-    end
-
-    def nonbumping
-      where("comments.do_not_bump_post = true")
-    end
-
     def post_tags_match(query)
       PostQueryBuilder.new(query).build(self.joins(:post)).reorder("")
     end
@@ -110,14 +94,9 @@ class Comment < ApplicationRecord
         q = q.for_creator(params[:creator_id].to_i)
       end
 
-      q = q.deleted if params[:is_deleted] == "true"
-      q = q.undeleted if params[:is_deleted] == "false"
-
-      q = q.sticky if params[:is_sticky] == "true"
-      q = q.unsticky if params[:is_sticky] == "false"
-
-      q = q.nonbumping if params[:do_not_bump_post] == "true"
-      q = q.bumping if params[:do_not_bump_post] == "false"
+      q = q.attribute_matches(:is_deleted, params[:is_deleted])
+      q = q.attribute_matches(:is_sticky, params[:is_sticky])
+      q = q.attribute_matches(:do_not_bump_post, params[:do_not_bump_post])
 
       case params[:order]
       when "post_id", "post_id_desc"

app/models/dmail.rb

@@ -171,6 +171,10 @@ class Dmail < ApplicationRecord
       end
     end
 
+    def read
+      where(is_read: true)
+    end
+
     def unread
       where("is_read = false and is_deleted = false")
     end
@@ -214,17 +218,13 @@ class Dmail < ApplicationRecord
         q = q.where("from_id = ?", params[:from_id].to_i)
       end
 
-      if params[:is_spam].present?
-        q = q.where("is_spam = ?", true)
-      else
-        q = q.where("is_spam = ?", false)
-      end
+      params[:is_spam] = false unless params[:is_spam].present?
+      q = q.attribute_matches(:is_spam, params[:is_spam])
+      q = q.attribute_matches(:is_read, params[:is_read])
+      q = q.attribute_matches(:is_deleted, params[:is_deleted])
 
-      if params[:read] == "true"
-        q = q.where("is_read = true")
-      elsif params[:read] == "false"
-        q = q.unread
-      end
+      q = q.read if params[:read].to_s.truthy?
+      q = q.unread if params[:read].to_s.falsy?
 
       q.apply_default_order(params)
     end

app/models/favorite_group.rb

@@ -65,6 +65,8 @@ class FavoriteGroup < ApplicationRecord
         q = q.name_matches(params[:name_matches])
       end
 
+      q = q.attribute_matches(:is_public, params[:is_public])
+
       q.apply_default_order(params)
     end
   end

app/models/forum_post.rb

@@ -92,6 +92,8 @@ class ForumPost < ApplicationRecord
         q = q.joins(:topic).where("forum_topics.category_id = ?", params[:topic_category_id].to_i)
       end
 
+      q = q.attribute_matches(:is_deleted, params[:is_deleted])
+
       q.apply_default_order(params)
     end
   end

app/models/forum_topic.rb

@@ -95,6 +95,10 @@ class ForumTopic < ApplicationRecord
         q = q.where("title = ?", params[:title])
       end
 
+      q = q.attribute_matches(:is_sticky, params[:is_sticky])
+      q = q.attribute_matches(:is_locked, params[:is_locked])
+      q = q.attribute_matches(:is_deleted, params[:is_deleted])
+
       case params[:order]
       when "sticky"
         q = q.sticky_first

app/models/note.rb

@@ -47,11 +47,7 @@ class Note < ApplicationRecord
         q = q.body_matches(params[:body_matches])
       end
 
-      if params[:is_active] == "true"
-        q = q.active
-      elsif params[:is_active] == "false"
-        q = q.where("is_active = false")
-      end
+      q = q.attribute_matches(:is_active, params[:is_active])
 
       if params[:post_id].present?
         q = q.where(post_id: params[:post_id].split(",").map(&:to_i))

app/models/note_version.rb

@@ -17,6 +17,8 @@ class NoteVersion < ApplicationRecord
       q = q.where(note_id: params[:note_id].split(",").map(&:to_i))
     end
 
+    q = q.attribute_matches(:is_active, params[:is_active])
+
     q.apply_default_order(params)
   end
 

app/models/pool.rb

@@ -70,23 +70,14 @@ class Pool < ApplicationRecord
         q = q.where(creator_id: params[:creator_id].split(",").map(&:to_i))
       end
 
-      if params[:is_active] == "true"
-        q = q.where("pools.is_active = true")
-      elsif params[:is_active] == "false"
-        q = q.where("pools.is_active = false")
-      end
-
       if params[:category] == "series"
         q = q.series
       elsif params[:category] == "collection"
         q = q.collection
       end
 
-      if params[:is_deleted] == "true"
-        q = q.deleted
-      else
-        q = q.undeleted
-      end
+      q = q.attribute_matches(:is_active, params[:is_active])
+      q = q.attribute_matches(:is_deleted, params[:is_deleted])
 
       params[:order] ||= params.delete(:sort)
       case params[:order]

app/models/post.rb

@@ -1829,28 +1829,20 @@ class Post < ApplicationRecord
   end
 
   def mark_as_translated(params)
-    tags = self.tag_array.dup
+    add_tag("check_translation") if params["check_translation"].to_s.truthy?
+    remove_tag("check_translation") if params["check_translation"].to_s.falsy?
 
-    if params["check_translation"] == "1"
-      tags << "check_translation"
-    elsif params["check_translation"] == "0"
-      tags -= ["check_translation"]
-    end
-    if params["partially_translated"] == "1"
-      tags << "partially_translated"
-    elsif params["partially_translated"] == "0"
-      tags -= ["partially_translated"]
-    end
+    add_tag("partially_translated") if params["partially_translated"].to_s.truthy?
+    remove_tag("partially_translated") if params["partially_translated"].to_s.falsy?
 
-    if params["check_translation"] == "1" || params["partially_translated"] == "1"
-      tags << "translation_request"
-      tags -= ["translated"]
+    if has_tag?("check_translation") || has_tag?("partially_translated")
+      add_tag("translation_request")
+      remove_tag("translated")
     else
-      tags << "translated"
-      tags -= ["translation_request"]
+      add_tag("translated")
+      remove_tag("translation_request")
     end
 
-    self.tag_string = tags.join(" ")
     save
   end
 

app/models/post_appeal.rb

@@ -65,11 +65,8 @@ class PostAppeal < ApplicationRecord
         q = q.post_tags_match(params[:post_tags_match])
       end
 
-      if params[:is_resolved] == "true"
-        q = q.resolved
-      elsif params[:is_resolved] == "false"
-        q = q.unresolved
-      end
+      q = q.resolved if params[:is_resolved].to_s.truthy?
+      q = q.unresolved if params[:is_resolved].to_s.falsy?
 
       q.apply_default_order(params)
     end

app/models/post_flag.rb

@@ -98,11 +98,7 @@ class PostFlag < ApplicationRecord
         q = q.post_tags_match(params[:post_tags_match])
       end
 
-      if params[:is_resolved] == "true"
-        q = q.resolved
-      elsif params[:is_resolved] == "false"
-        q = q.unresolved
-      end
+      q = q.attribute_matches(:is_resolved, params[:is_resolved])
 
       case params[:category]
       when "normal"

app/models/saved_search.rb

@@ -130,10 +130,6 @@ class SavedSearch < ApplicationRecord
   end
 
   def disable_labels=(value)
-    CurrentUser.update(disable_categorized_saved_searches: true) if value == "1"
-  end
-
-  def disable_labels=(value)
-    CurrentUser.update(disable_categorized_saved_searches: true) if value == "1"
+    CurrentUser.update(disable_categorized_saved_searches: true) if value.to_s.truthy?
   end
 end

app/models/tag.rb

@@ -887,22 +887,24 @@ class Tag < ApplicationRecord
         q = q.where("category = ?", params[:category])
       end
 
-      if params[:hide_empty].blank? || params[:hide_empty] != "no"
+      if params[:hide_empty].blank? || params[:hide_empty].to_s.truthy?
         q = q.where("post_count > 0")
       end
 
-      if params[:has_wiki] == "yes"
+      if params[:has_wiki].to_s.truthy?
         q = q.joins(:wiki_page).where("wiki_pages.is_deleted = false")
-      elsif params[:has_wiki] == "no"
+      elsif params[:has_wiki].to_s.falsy?
         q = q.joins("LEFT JOIN wiki_pages ON tags.name = wiki_pages.title").where("wiki_pages.title IS NULL OR wiki_pages.is_deleted = true")
       end
 
-      if params[:has_artist] == "yes"
+      if params[:has_artist].to_s.truthy?
         q = q.joins("INNER JOIN artists ON tags.name = artists.name").where("artists.is_active = true")
-      elsif params[:has_artist] == "no"
+      elsif params[:has_artist].to_s.falsy?
         q = q.joins("LEFT JOIN artists ON tags.name = artists.name").where("artists.name IS NULL OR artists.is_active = false")
       end
 
+      q = q.attribute_matches(:is_locked, params[:is_locked])
+
       params[:order] ||= params.delete(:sort)
       case params[:order]
       when "name"

app/models/upload.rb

@@ -426,9 +426,9 @@ class Upload < ApplicationRecord
         q = q.attribute_matches(:post_id, params[:post_id])
       end
 
-      if params[:has_post] == "yes"
+      if params[:has_post].to_s.truthy?
         q = q.where.not(post_id: nil)
-      elsif params[:has_post] == "no"
+      elsif params[:has_post].to_s.falsy?
         q = q.where(post_id: nil)
       end
 
@@ -490,10 +490,10 @@ class Upload < ApplicationRecord
   end
 
   def upload_as_pending?
-    as_pending == "1"
+    as_pending.to_s.truthy?
   end
 
   def include_artist_commentary?
-    include_artist_commentary == "1"
+    include_artist_commentary.to_s.truthy?
   end
 end

app/models/user.rb

@@ -843,10 +843,10 @@ class User < ApplicationRecord
       [:can_approve_posts, :can_upload_free, :is_super_voter].each do |x|
         if params[x].present?
           attr_idx = BOOLEAN_ATTRIBUTES.index(x.to_s)
-          if params[x] == "true"
+          if params[x].to_s.truthy?
             bitprefs_include ||= "0"*bitprefs_length
             bitprefs_include[attr_idx] = '1'
-          elsif params[x] == "false"
+          elsif params[x].to_s.falsy?
             bitprefs_exclude ||= "0"*bitprefs_length
             bitprefs_exclude[attr_idx] = '1'
           end
@@ -865,7 +865,7 @@ class User < ApplicationRecord
                     {:len => bitprefs_length, :bits => bitprefs_exclude})
       end
 
-      if params[:current_user_first] == "true" && !CurrentUser.is_anonymous?
+      if params[:current_user_first].to_s.truthy? && !CurrentUser.is_anonymous?
         q = q.order("id = #{CurrentUser.user.id.to_i} desc")
       end
       

app/models/wiki_page.rb

@@ -82,16 +82,19 @@ class WikiPage < ApplicationRecord
         q = q.where("creator_id = (select _.id from users _ where lower(_.name) = ?)", params[:creator_name].tr(" ", "_").mb_chars.downcase)
       end
 
-      if params[:hide_deleted] =~ /y/i
+      if params[:hide_deleted].to_s.truthy?
         q = q.where("is_deleted = false")
       end
 
-      if params[:other_names_present] == "yes"
+      if params[:other_names_present].to_s.truthy?
         q = q.where("other_names is not null and other_names != ''")
-      elsif params[:other_names_present] == "no"
+      elsif params[:other_names_present].to_s.falsy?
         q = q.where("other_names is null or other_names = ''")
       end
 
+      q = q.attribute_matches(:is_locked, params[:is_locked])
+      q = q.attribute_matches(:is_deleted, params[:is_deleted])
+
       params[:order] ||= params.delete(:sort)
       case params[:order]
       when "title"
@@ -165,7 +168,7 @@ class WikiPage < ApplicationRecord
   end
 
   def skip_secondary_validations=(value)
-    @skip_secondary_validations = (value == true || value == "1")
+    @skip_secondary_validations = value.to_s.truthy?
   end
 
   def category_name

app/models/wiki_page_version.rb

@@ -20,6 +20,9 @@ class WikiPageVersion < ApplicationRecord
         q = q.where("wiki_page_id = ?", params[:wiki_page_id].to_i)
       end
 
+      q = q.attribute_matches(:is_locked, params[:is_locked])
+      q = q.attribute_matches(:is_deleted, params[:is_deleted])
+
       q.apply_default_order(params)
     end
   end

config/initializers/core_extensions.rb

@@ -23,6 +23,14 @@ module Danbooru
       def to_escaped_for_tsquery
         "'#{gsub(/\0/, '').gsub(/'/, '\0\0').gsub(/\\/, '\0\0\0\0')}'"
       end
+
+      def truthy?
+        self.match?(/\A(true|t|yes|y|on|1)\z/i)
+      end
+
+      def falsy?
+        self.match?(/\A(false|f|no|n|off|0)\z/i)
+      end
     end
   end
 end