From 33103f6dc44e8ea20b7f3d512e977f23c63ff29c Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 15 Jan 2022 20:16:06 -0600 Subject: [PATCH] pools: add ability to search for pools linking to given tag. Add ability to search for pools linking to a given tag in the pool description. Example: https://danbooru.donmai.us/pools?search[linked_to]=touhou (This isn't actually exposed in the UI to avoid cluttering the pool search form with rarely used options.) Pools with broken links can be found here: https://danbooru.donmai.us/dtext_links?search[has_linked_tag]=No&search[has_linked_wiki]=No&search[model_type]=Pool Lays the groundwork for fixing #4629. --- app/logical/concerns/has_dtext_links.rb | 36 +++++++++++++++++++ app/models/application_record.rb | 1 + app/models/dtext_link.rb | 5 +-- app/models/forum_post.rb | 11 +----- app/models/pool.rb | 11 +++++- app/models/wiki_page.rb | 19 +--------- app/views/dtext_links/index.html.erb | 6 ++-- script/fixes/090_create_pool_dtext_links.rb | 9 +++++ .../functional/dtext_links_controller_test.rb | 6 ++-- test/functional/pools_controller_test.rb | 10 +++--- 10 files changed, 72 insertions(+), 42 deletions(-) create mode 100644 app/logical/concerns/has_dtext_links.rb create mode 100755 script/fixes/090_create_pool_dtext_links.rb diff --git a/app/logical/concerns/has_dtext_links.rb b/app/logical/concerns/has_dtext_links.rb new file mode 100644 index 000000000..4573717c3 --- /dev/null +++ b/app/logical/concerns/has_dtext_links.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module HasDtextLinks + extend ActiveSupport::Concern + + class_methods do + # Declare a field that has DText links. Any wiki links or external links + # contained in this field will be saved in the dtext_links table whenever + # the field is updated. This allows finding wiki pages, forum posts, and + # pool descriptions linking to a given tag. + # + # @param attribute [Symbol] the name of the DText field. + def has_dtext_links(attribute) + has_many :dtext_links, as: :model, dependent: :destroy + before_save :update_dtext_links, if: :dtext_links_changed? + + define_method(:dtext_links_changed?) do + attribute_changed?(attribute) && DText.dtext_links_differ?(self[attribute], attribute_was(attribute)) + end + + define_method(:update_dtext_links) do + self.dtext_links = DtextLink.new_from_dtext(self[attribute]) + end + end + + # Return pages (e.g. wikis, forum posts, pool descriptions) that link to the given wiki page. + def linked_to(title) + where(dtext_links: DtextLink.where(model_type: name).wiki_link.where(link_target: WikiPage.normalize_title(title))) + end + + # Return pages that don't link to the given wiki page. + def not_linked_to(title) + where.not(dtext_links: DtextLink.where(model_type: name).wiki_link.where(link_target: WikiPage.normalize_title(title))) + end + end +end diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 1c8b94d6d..97b653f6a 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -7,6 +7,7 @@ class ApplicationRecord < ActiveRecord::Base include Mentionable include Normalizable include ArrayAttribute + include HasDtextLinks extend HasBitFlags extend Searchable diff --git a/app/models/dtext_link.rb b/app/models/dtext_link.rb index 87af5dcf7..d353291d5 100644 --- a/app/models/dtext_link.rb +++ b/app/models/dtext_link.rb @@ -12,15 +12,16 @@ class DtextLink < ApplicationRecord scope :wiki_page, -> { where(model_type: "WikiPage") } scope :forum_post, -> { where(model_type: "ForumPost") } + scope :pool, -> { where(model_type: "Pool") } def self.visible(user) # XXX the double negation is to prevent postgres from choosing a bad query # plan (it doesn't know that most forum posts aren't mod-only posts). - wiki_page.or(forum_post.where.not(model_id: ForumPost.not_visible(user))) + wiki_page.or(forum_post.where.not(model_id: ForumPost.not_visible(user))).or(pool) end def self.model_types - %w[WikiPage ForumPost] + %w[WikiPage ForumPost Pool] end def self.new_from_dtext(dtext) diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb index 50ca429ad..d9b4ce309 100644 --- a/app/models/forum_post.rb +++ b/app/models/forum_post.rb @@ -7,7 +7,6 @@ class ForumPost < ApplicationRecord belongs_to_updater belongs_to :topic, class_name: "ForumTopic", inverse_of: :forum_posts - has_many :dtext_links, as: :model, dependent: :destroy has_many :moderation_reports, as: :model has_many :votes, class_name: "ForumPostVote" has_one :tag_alias @@ -16,7 +15,6 @@ class ForumPost < ApplicationRecord validates :body, presence: true, length: { maximum: 200_000 }, if: :body_changed? - before_save :update_dtext_links, if: :dtext_links_changed? before_create :autoreport_spam after_create :update_topic_updated_at_on_create after_update :update_topic_updated_at_on_update_for_original_posts @@ -31,6 +29,7 @@ class ForumPost < ApplicationRecord after_create_commit :async_send_discord_notification deletable + has_dtext_links :body mentionable( message_field: :body, title: ->(_user_name) {%{#{creator.name} mentioned you in topic ##{topic_id} (#{topic.title})}}, @@ -112,14 +111,6 @@ class ForumPost < ApplicationRecord update_topic_updated_at_on_undelete end - def dtext_links_changed? - body_changed? && DText.dtext_links_differ?(body, body_was) - end - - def update_dtext_links - self.dtext_links = DtextLink.new_from_dtext(body) - end - def update_topic_updated_at_on_delete max = ForumPost.where(:topic_id => topic.id, :is_deleted => false).order("updated_at desc").first if max diff --git a/app/models/pool.rb b/app/models/pool.rb index dc5e61d94..cee883fe9 100644 --- a/app/models/pool.rb +++ b/app/models/pool.rb @@ -15,6 +15,7 @@ class Pool < ApplicationRecord after_save :create_version deletable + has_dtext_links :description scope :series, -> { where(category: "series") } scope :collection, -> { where(category: "collection") } @@ -37,7 +38,7 @@ class Pool < ApplicationRecord end def search(params) - q = search_attributes(params, :id, :created_at, :updated_at, :is_deleted, :name, :description, :post_ids) + 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] @@ -48,6 +49,14 @@ class Pool < ApplicationRecord q = q.name_matches(params[:name_matches]) end + if params[:linked_to].present? + q = q.linked_to(params[:linked_to]) + end + + if params[:not_linked_to].present? + q = q.not_linked_to(params[:not_linked_to]) + end + case params[:category] when "series" q = q.series diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index d8e7fe620..bd77d3416 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -5,7 +5,6 @@ class WikiPage < ApplicationRecord META_WIKIS = ["list_of_", "tag_group:", "pool_group:", "howto:", "about:", "help:", "template:"] - before_save :update_dtext_links, if: :dtext_links_changed? after_save :create_version normalize :title, :normalize_title @@ -22,9 +21,9 @@ class WikiPage < ApplicationRecord has_one :tag, :foreign_key => "name", :primary_key => "title" has_one :artist, -> { active }, foreign_key: "name", primary_key: "title" has_many :versions, -> {order("wiki_page_versions.id ASC")}, :class_name => "WikiPageVersion", :dependent => :destroy - has_many :dtext_links, as: :model, dependent: :destroy deletable + has_dtext_links :body module SearchMethods def find_by_id_or_title(id) @@ -58,14 +57,6 @@ class WikiPage < ApplicationRecord end end - def linked_to(title) - where(dtext_links: DtextLink.wiki_page.wiki_link.where(link_target: normalize_title(title))) - end - - def not_linked_to(title) - where.not(dtext_links: DtextLink.wiki_page.wiki_link.where(link_target: normalize_title(title))) - end - def default_order order(updated_at: :desc) end @@ -213,14 +204,6 @@ class WikiPage < ApplicationRecord end end - def dtext_links_changed? - body_changed? && DText.dtext_links_differ?(body, body_was) - end - - def update_dtext_links - self.dtext_links = DtextLink.new_from_dtext(body) - end - def tags titles = DText.parse_wiki_titles(body).uniq tags = Tag.nonempty.where(name: titles).pluck(:name) diff --git a/app/views/dtext_links/index.html.erb b/app/views/dtext_links/index.html.erb index 8ec40e974..266d8820a 100644 --- a/app/views/dtext_links/index.html.erb +++ b/app/views/dtext_links/index.html.erb @@ -2,7 +2,7 @@
<%= search_form_for(dtext_links_path) do |f| %> <%= f.input :link_target_ilike, label: "Link", hint: "Use * for wildcard", input_html: { value: params[:search][:link_target_ilike], data: { autocomplete: "wiki-page" } } %> - <%= f.input :model_type, label: "Page Type", collection: [["Wiki Page", "WikiPage"], ["Forum Post", "ForumPost"]], include_blank: true, selected: params[:search][:model_type] %> + <%= f.input :model_type, label: "Page Type", collection: DtextLink.model_types.map { [_1.titleize, _1] }, include_blank: true, selected: params[:search][:model_type] %> <%= f.input :link_type, label: "Link Type", collection: [["Wiki", "0"], ["External", "1"]], include_blank: true, selected: params[:search][:link_type] %> <%= f.input :has_linked_wiki, label: "Wiki Exists?", collection: ["Yes", "No"], include_blank: true, selected: params[:search][:has_linked_wiki] %> <%= f.input :has_linked_tag, label: "Tag Exists?", collection: ["Yes", "No"], include_blank: true, selected: params[:search][:has_linked_tag] %> @@ -16,8 +16,8 @@ <%= link_to(dtext_link.model.title, dtext_link.model) %> <%= link_to("ยป", dtext_links_path(search: { model_type: "WikiPage", model: { title: dtext_link.model.title }})) %> - <% elsif dtext_link.model_type == "ForumPost" %> - <%= link_to("forum ##{dtext_link.model_id}", dtext_link.model) %> + <% else %> + <%= link_to(dtext_link.model.dtext_shortlink, dtext_link.model) %> <% end %> <% end %> diff --git a/script/fixes/090_create_pool_dtext_links.rb b/script/fixes/090_create_pool_dtext_links.rb new file mode 100755 index 000000000..9395b095d --- /dev/null +++ b/script/fixes/090_create_pool_dtext_links.rb @@ -0,0 +1,9 @@ +#!/usr/bin/env ruby + +require_relative "base" + +with_confirmation do + Pool.find_each do |pool| + pool.update_dtext_links + end +end diff --git a/test/functional/dtext_links_controller_test.rb b/test/functional/dtext_links_controller_test.rb index 366a1b510..d3a137f39 100644 --- a/test/functional/dtext_links_controller_test.rb +++ b/test/functional/dtext_links_controller_test.rb @@ -6,6 +6,7 @@ class DtextLinksControllerTest < ActionDispatch::IntegrationTest as(@user) do @wiki = create(:wiki_page, title: "case", body: "[[test]]") @forum = create(:forum_post, topic: build(:forum_topic, title: "blah"), body: "[[case]]") + @pool = create(:pool, description: "[[case]]") create(:tag, name: "test") end end @@ -16,13 +17,14 @@ class DtextLinksControllerTest < ActionDispatch::IntegrationTest assert_response :success end - should respond_to_search({}).with { @forum.dtext_links + @wiki.dtext_links } + should respond_to_search({}).with { @pool.dtext_links + @forum.dtext_links + @wiki.dtext_links } context "using includes" do should respond_to_search(model_type: "WikiPage").with { @wiki.dtext_links } should respond_to_search(model_type: "ForumPost").with { @forum.dtext_links } + should respond_to_search(model_type: "Pool").with { @pool.dtext_links } should respond_to_search(has_linked_tag: "true").with { @wiki.dtext_links } - should respond_to_search(has_linked_wiki: "true").with { @forum.dtext_links } + should respond_to_search(has_linked_wiki: "true").with { @pool.dtext_links + @forum.dtext_links } should respond_to_search(ForumPost: {topic: {title_matches: "blah"}}).with { @forum.dtext_links } should respond_to_search(ForumPost: {topic: {title_matches: "nah"}}).with { [] } end diff --git a/test/functional/pools_controller_test.rb b/test/functional/pools_controller_test.rb index 35b9e3fff..aee1a9d9a 100644 --- a/test/functional/pools_controller_test.rb +++ b/test/functional/pools_controller_test.rb @@ -9,7 +9,7 @@ class PoolsControllerTest < ActionDispatch::IntegrationTest end as(@user) do @post = create(:post) - @pool = create(:pool) + @pool = create(:pool, name: "pool", description: "[[touhou]]") end end @@ -19,17 +19,15 @@ class PoolsControllerTest < ActionDispatch::IntegrationTest assert_response :success end - should "list all pools (with search)" do - get pools_path, params: {:search => {:name_matches => @pool.name}} - assert_response :success - end - should "render for a sitemap" do get pools_path(format: :sitemap) assert_response :success assert_equal(Pool.count, response.parsed_body.css("urlset url loc").size) end + should respond_to_search(name_matches: "pool").with { @pool } + should respond_to_search(linked_to: "touhou").with { @pool } + should respond_to_search(not_linked_to: "touhou").with { [] } end context "show action" do