From 3114ef3daf65010a3a228d58460cb2e64f5318a7 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 21 Sep 2022 19:32:25 -0500 Subject: [PATCH] searchable: standardize the `_matches` operator for text fields. Standardize it so that all fields of type `text` are searchable with `search[_matches]`. Before, the `_matches` param was handled manually and some fields were left out or handled inconsistently. Now it applies to all columns of type `text`. This does a full-text search on the field, so for example, searching `/artist_commentaries?search[translated_description_matches]=smiling` will match translated commentaries containing either the word "smiling", "smiles", "smiled", or "smile". Note that this only applies to columns defined as type `text`, not to columns defined as `character varying`. The difference is that `text` is used for fields containing free-form natural language, such as comments, notes, forum posts, wiki pages, pool descriptions, etc, while `character varying` is used for short strings not containing free-form language, such as tag names, wiki page titles, urls, status fields, etc. API changes: * Add the `search[original_title_matches]`, `search[original_description_matches]`, `search[translated_title_matches]`, `search[translated_description_matches]` params to /artist_commentaries and /artist_commentary_versions. * Remove the `search[name_matches]` and `search[group_name_matches]` params from /artist_versions. * Remove the `search[title_matches]` param from /wiki_page_versions. * Change the `search[name_matches]` param on /pools, /favorite_groups, and /pool_versions to do a full-text search instead of a substring match. --- app/logical/concerns/searchable.rb | 18 +++++++++++++++--- app/models/artist_version.rb | 2 -- app/models/ban.rb | 1 - app/models/bulk_update_request.rb | 1 - app/models/comment.rb | 1 - app/models/dmail.rb | 4 +--- app/models/forum_post.rb | 1 - app/models/forum_topic.rb | 1 - app/models/ip_ban.rb | 1 - app/models/mod_action.rb | 1 - app/models/moderation_report.rb | 1 - app/models/note.rb | 1 - app/models/note_version.rb | 1 - app/models/pool.rb | 1 - app/models/post_appeal.rb | 1 - app/models/post_disapproval.rb | 1 - app/models/post_flag.rb | 1 - app/models/user_feedback.rb | 1 - app/models/wiki_page.rb | 3 +-- app/models/wiki_page_version.rb | 2 -- .../wiki_page_versions_controller_test.rb | 2 +- 21 files changed, 18 insertions(+), 28 deletions(-) diff --git a/app/logical/concerns/searchable.rb b/app/logical/concerns/searchable.rb index 032043e64..d9cb41404 100644 --- a/app/logical/concerns/searchable.rb +++ b/app/logical/concerns/searchable.rb @@ -188,7 +188,7 @@ module Searchable end end - def text_attribute_matches(columns, query) + def where_text_matches(columns, query) columns = Array.wrap(columns) if query.nil? @@ -264,7 +264,9 @@ module Searchable end case type - when :string, :text + when :string # :string is for columns of type `character varying` in the database + search_string_attribute(name) + when :text # :text is for columns of type `text` in the database search_text_attribute(name) when :uuid search_uuid_attribute(name) @@ -323,7 +325,7 @@ module Searchable relation end - def search_text_attribute(attr) + def search_string_attribute(attr) relation = self.relation if params[attr].present? @@ -397,6 +399,16 @@ module Searchable relation end + def search_text_attribute(attr) + relation = search_string_attribute(attr) + + if params[:"#{attr}_matches"].present? + relation = visible(relation, attr).where_text_matches(attr, params[:"#{attr}_matches"]) + end + + relation + end + def search_uuid_attribute(attr) relation = self.relation diff --git a/app/models/artist_version.rb b/app/models/artist_version.rb index 70fe17395..9165d5aca 100644 --- a/app/models/artist_version.rb +++ b/app/models/artist_version.rb @@ -18,8 +18,6 @@ class ArtistVersion < ApplicationRecord module SearchMethods def search(params) q = search_attributes(params, :id, :created_at, :updated_at, :is_deleted, :is_banned, :name, :group_name, :urls, :other_names, :updater, :artist) - q = q.text_attribute_matches(:name, params[:name_matches]) - q = q.text_attribute_matches(:group_name, params[:group_name_matches]) if params[:order] == "name" q = q.order("artist_versions.name").default_order diff --git a/app/models/ban.rb b/app/models/ban.rb index d4548ac18..6f3d8d304 100644 --- a/app/models/ban.rb +++ b/app/models/ban.rb @@ -20,7 +20,6 @@ class Ban < ApplicationRecord def self.search(params) q = search_attributes(params, :id, :created_at, :updated_at, :duration, :reason, :user, :banner) - q = q.text_attribute_matches(:reason, params[:reason_matches]) q = q.expired if params[:expired].to_s.truthy? q = q.unexpired if params[:expired].to_s.falsy? diff --git a/app/models/bulk_update_request.rb b/app/models/bulk_update_request.rb index 35caa9063..c40511ada 100644 --- a/app/models/bulk_update_request.rb +++ b/app/models/bulk_update_request.rb @@ -31,7 +31,6 @@ class BulkUpdateRequest < ApplicationRecord module SearchMethods def search(params = {}) q = search_attributes(params, :id, :created_at, :updated_at, :script, :tags, :user, :forum_topic, :forum_post, :approver) - q = q.text_attribute_matches(:script, params[:script_matches]) if params[:status].present? q = q.where(status: params[:status].split(",")) diff --git a/app/models/comment.rb b/app/models/comment.rb index 9ea14b68d..616a43c19 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -34,7 +34,6 @@ class Comment < ApplicationRecord module SearchMethods def search(params) q = search_attributes(params, :id, :created_at, :updated_at, :is_deleted, :is_sticky, :do_not_bump_post, :body, :score, :post, :creator, :updater) - q = q.text_attribute_matches(:body, params[:body_matches]) case params[:order] when "post_id", "post_id_desc" diff --git a/app/models/dmail.rb b/app/models/dmail.rb index df60e3f4c..ad46e0a22 100644 --- a/app/models/dmail.rb +++ b/app/models/dmail.rb @@ -106,9 +106,7 @@ class Dmail < ApplicationRecord def search(params) q = search_attributes(params, :id, :created_at, :updated_at, :is_read, :is_deleted, :title, :body, :to, :from) - q = q.text_attribute_matches(:title, params[:title_matches]) - q = q.text_attribute_matches(:body, params[:body_matches]) - q = q.text_attribute_matches([:title, :body], params[:message_matches]) + q = q.where_text_matches([:title, :body], params[:message_matches]) q = q.folder_matches(params[:folder]) diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb index 8b9ac688f..3555ec55c 100644 --- a/app/models/forum_post.rb +++ b/app/models/forum_post.rb @@ -58,7 +58,6 @@ class ForumPost < ApplicationRecord def search(params) q = search_attributes(params, :id, :created_at, :updated_at, :is_deleted, :body, :creator, :updater, :topic, :dtext_links, :votes, :tag_alias, :tag_implication, :bulk_update_request) - q = q.text_attribute_matches(:body, params[:body_matches]) if params[:linked_to].present? q = q.wiki_link_matches(params[:linked_to]) diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb index bdf6cea95..6d317ff35 100644 --- a/app/models/forum_topic.rb +++ b/app/models/forum_topic.rb @@ -89,7 +89,6 @@ class ForumTopic < ApplicationRecord def search(params) q = search_attributes(params, :id, :created_at, :updated_at, :is_sticky, :is_locked, :is_deleted, :category_id, :title, :response_count, :creator, :updater, :forum_posts, :bulk_update_requests, :tag_aliases, :tag_implications) - q = q.text_attribute_matches(:title, params[:title_matches]) if params[:is_private].to_s.truthy? q = q.private_only diff --git a/app/models/ip_ban.rb b/app/models/ip_ban.rb index 57adbe046..9e70a70b6 100644 --- a/app/models/ip_ban.rb +++ b/app/models/ip_ban.rb @@ -36,7 +36,6 @@ class IpBan < ApplicationRecord def self.search(params) q = search_attributes(params, :id, :created_at, :updated_at, :ip_addr, :reason, :is_deleted, :category, :hit_count, :last_hit_at, :creator) - q = q.text_attribute_matches(:reason, params[:reason_matches]) case params[:order] when /\A(created_at|updated_at|last_hit_at)(?:_(asc|desc))?\z/i diff --git a/app/models/mod_action.rb b/app/models/mod_action.rb index 841aa80d2..4d70f452e 100644 --- a/app/models/mod_action.rb +++ b/app/models/mod_action.rb @@ -80,7 +80,6 @@ class ModAction < ApplicationRecord def self.search(params) q = search_attributes(params, :id, :created_at, :updated_at, :category, :description, :creator) - q = q.text_attribute_matches(:description, params[:description_matches]) case params[:order] when "created_at_asc" diff --git a/app/models/moderation_report.rb b/app/models/moderation_report.rb index 66301d6b5..1e565eaea 100644 --- a/app/models/moderation_report.rb +++ b/app/models/moderation_report.rb @@ -84,7 +84,6 @@ class ModerationReport < ApplicationRecord def self.search(params) q = search_attributes(params, :id, :created_at, :updated_at, :reason, :creator, :model, :status) - q = q.text_attribute_matches(:reason, params[:reason_matches]) if params[:recipient_id].present? q = q.received_by(User.search(id: params[:recipient_id])) diff --git a/app/models/note.rb b/app/models/note.rb index ceb8a9b13..1d39c82e1 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -21,7 +21,6 @@ class Note < ApplicationRecord module SearchMethods def search(params) q = search_attributes(params, :id, :created_at, :updated_at, :is_active, :x, :y, :width, :height, :body, :version, :post) - q = q.text_attribute_matches(:body, params[:body_matches]) q.apply_default_order(params) end diff --git a/app/models/note_version.rb b/app/models/note_version.rb index 841deae04..a27bc6820 100644 --- a/app/models/note_version.rb +++ b/app/models/note_version.rb @@ -7,7 +7,6 @@ class NoteVersion < ApplicationRecord def self.search(params) q = search_attributes(params, :id, :created_at, :updated_at, :is_active, :x, :y, :width, :height, :body, :version, :updater, :note, :post) - q = q.text_attribute_matches(:body, params[:body_matches]) q.apply_default_order(params) end diff --git a/app/models/pool.rb b/app/models/pool.rb index da44b48de..43b885b78 100644 --- a/app/models/pool.rb +++ b/app/models/pool.rb @@ -39,7 +39,6 @@ class Pool < ApplicationRecord def search(params) q = search_attributes(params, :id, :created_at, :updated_at, :is_deleted, :name, :description, :post_ids, :dtext_links) - q = q.text_attribute_matches(:description, params[:description_matches]) if params[:post_tags_match] q = q.post_tags_match(params[:post_tags_match]) diff --git a/app/models/post_appeal.rb b/app/models/post_appeal.rb index 495e6b87e..ec663eab4 100644 --- a/app/models/post_appeal.rb +++ b/app/models/post_appeal.rb @@ -25,7 +25,6 @@ class PostAppeal < ApplicationRecord module SearchMethods def search(params) q = search_attributes(params, :id, :created_at, :updated_at, :reason, :status, :creator, :post) - q = q.text_attribute_matches(:reason, params[:reason_matches]) q.apply_default_order(params) end diff --git a/app/models/post_disapproval.rb b/app/models/post_disapproval.rb index 757718895..1df0d5476 100644 --- a/app/models/post_disapproval.rb +++ b/app/models/post_disapproval.rb @@ -36,7 +36,6 @@ class PostDisapproval < ApplicationRecord def search(params) q = search_attributes(params, :id, :created_at, :updated_at, :message, :reason, :post) - q = q.text_attribute_matches(:message, params[:message_matches]) q = q.with_message if params[:has_message].to_s.truthy? q = q.without_message if params[:has_message].to_s.falsy? diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb index dbb6029ff..135328a60 100644 --- a/app/models/post_flag.rb +++ b/app/models/post_flag.rb @@ -60,7 +60,6 @@ class PostFlag < ApplicationRecord def search(params) q = search_attributes(params, :id, :created_at, :updated_at, :reason, :status, :post, :creator) - q = q.text_attribute_matches(:reason, params[:reason_matches]) if params[:category] q = q.category_matches(params[:category]) diff --git a/app/models/user_feedback.rb b/app/models/user_feedback.rb index 8ea571b92..ea415a136 100644 --- a/app/models/user_feedback.rb +++ b/app/models/user_feedback.rb @@ -34,7 +34,6 @@ class UserFeedback < ApplicationRecord def search(params) q = search_attributes(params, :id, :created_at, :updated_at, :category, :body, :is_deleted, :creator, :user) - q = q.text_attribute_matches(:body, params[:body_matches]) q.apply_default_order(params) end diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 25d51f55f..4468332a0 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -63,8 +63,7 @@ class WikiPage < ApplicationRecord def search(params = {}) q = search_attributes(params, :id, :created_at, :updated_at, :is_locked, :is_deleted, :body, :title, :other_names, :tag, :artist, :dtext_links) - q = q.text_attribute_matches(:body, params[:body_matches]) - q = q.text_attribute_matches([:title, :body], params[:title_or_body_matches]) + q = q.where_text_matches([:title, :body], params[:title_or_body_matches]) if params[:title_normalize].present? q = q.where_like(:title, normalize_title(params[:title_normalize])) diff --git a/app/models/wiki_page_version.rb b/app/models/wiki_page_version.rb index 541b0fe5b..bcf6fbee5 100644 --- a/app/models/wiki_page_version.rb +++ b/app/models/wiki_page_version.rb @@ -9,8 +9,6 @@ class WikiPageVersion < ApplicationRecord module SearchMethods def search(params) q = search_attributes(params, :id, :created_at, :updated_at, :title, :body, :other_names, :is_locked, :is_deleted, :updater, :wiki_page, :tag) - q = q.text_attribute_matches(:title, params[:title_matches]) - q = q.text_attribute_matches(:body, params[:body_matches]) q.apply_default_order(params) end diff --git a/test/functional/wiki_page_versions_controller_test.rb b/test/functional/wiki_page_versions_controller_test.rb index ad897b10c..b654a8ccd 100644 --- a/test/functional/wiki_page_versions_controller_test.rb +++ b/test/functional/wiki_page_versions_controller_test.rb @@ -21,7 +21,7 @@ class WikiPageVersionsControllerTest < ActionDispatch::IntegrationTest end should respond_to_search({}).with { @versions.reverse } - should respond_to_search(title_matches: "supreme").with { [@versions[2], @versions[1]] } + should respond_to_search(title_like: "supreme").with { [@versions[2], @versions[1]] } should respond_to_search(body_matches: "blah").with { [@versions[2], @versions[1]] } should respond_to_search(other_names_include_any: "not_this").with { [@versions[2], @versions[1]] }