From 308a5021b49aa2f57d8ac0e14c747a9490a824d2 Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 13 Nov 2018 18:08:20 -0600 Subject: [PATCH] wiki pages: convert other_names to array (#3987). --- app/controllers/wiki_pages_controller.rb | 19 ++++--------- app/helpers/wiki_pages_helper.rb | 2 +- app/logical/sources/strategies/base.rb | 2 +- app/models/wiki_page.rb | 28 ++++++++----------- app/models/wiki_page_version.rb | 5 +--- app/views/wiki_pages/_form.html.erb | 2 +- ..._change_strings_to_arrays_on_wiki_pages.rb | 26 +++++++++++++++++ db/structure.sql | 19 ++++--------- test/unit/wiki_page_test.rb | 2 +- 9 files changed, 55 insertions(+), 50 deletions(-) create mode 100644 db/migrate/20181113174914_change_strings_to_arrays_on_wiki_pages.rb diff --git a/app/controllers/wiki_pages_controller.rb b/app/controllers/wiki_pages_controller.rb index 5c3eab980..41b7a2b59 100644 --- a/app/controllers/wiki_pages_controller.rb +++ b/app/controllers/wiki_pages_controller.rb @@ -5,7 +5,7 @@ class WikiPagesController < ApplicationController before_action :normalize_search_params, :only => [:index] def new - @wiki_page = WikiPage.new(wiki_page_create_params) + @wiki_page = WikiPage.new(wiki_page_params(:create)) respond_with(@wiki_page) end @@ -54,13 +54,13 @@ class WikiPagesController < ApplicationController end def create - @wiki_page = WikiPage.create(wiki_page_create_params) + @wiki_page = WikiPage.create(wiki_page_params(:create)) respond_with(@wiki_page) end def update @wiki_page = WikiPage.find(params[:id]) - @wiki_page.update(wiki_page_update_params) + @wiki_page.update(wiki_page_params(:update)) respond_with(@wiki_page) end @@ -98,18 +98,11 @@ class WikiPagesController < ApplicationController end end - def wiki_page_create_params - permitted_params = %i[title body other_names skip_secondary_validations] + def wiki_page_params(context) + permitted_params = %i[body other_names other_names_string skip_secondary_validations] permitted_params += %i[is_locked is_deleted] if CurrentUser.is_builder? + permitted_params += %i[title] if context == :create || CurrentUser.is_builder? params.fetch(:wiki_page, {}).permit(permitted_params) end - - def wiki_page_update_params - permitted_params = %i[body other_names skip_secondary_validations] - permitted_params += %i[title is_locked is_deleted] if CurrentUser.is_builder? - - params.fetch(:wiki_page, {}).permit(permitted_params) - end - end diff --git a/app/helpers/wiki_pages_helper.rb b/app/helpers/wiki_pages_helper.rb index b41b9aa66..daa3b2d40 100644 --- a/app/helpers/wiki_pages_helper.rb +++ b/app/helpers/wiki_pages_helper.rb @@ -18,7 +18,7 @@ module WikiPagesHelper end def wiki_page_other_names_list(wiki_page) - names_html = wiki_page.other_names_array.map{|name| link_to(name, "http://www.pixiv.net/search.php?s_mode=s_tag_full&word=#{u(name)}", :class => "wiki-other-name")} + names_html = wiki_page.other_names.map{|name| link_to(name, "http://www.pixiv.net/search.php?s_mode=s_tag_full&word=#{u(name)}", :class => "wiki-other-name")} names_html.join(" ").html_safe end end diff --git a/app/logical/sources/strategies/base.rb b/app/logical/sources/strategies/base.rb index 7e891aeed..01d883d74 100644 --- a/app/logical/sources/strategies/base.rb +++ b/app/logical/sources/strategies/base.rb @@ -187,7 +187,7 @@ module Sources def translate_tag(untranslated_tag) return [] if untranslated_tag.blank? - translated_tag_names = WikiPage.active.other_names_equal(untranslated_tag).uniq.pluck(:title) + translated_tag_names = WikiPage.active.other_names_include(untranslated_tag).uniq.pluck(:title) translated_tag_names = TagAlias.to_aliased(translated_tag_names) translated_tags = Tag.where(name: translated_tag_names) diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index e3d840fc4..6a9c9cd59 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -4,14 +4,16 @@ class WikiPage < ApplicationRecord before_save :normalize_title before_save :normalize_other_names after_save :create_version - belongs_to_creator - belongs_to_updater validates_uniqueness_of :title, :case_sensitive => false validates_presence_of :title validates_presence_of :body, :unless => -> { is_deleted? || other_names.present? } validate :validate_rename validate :validate_not_locked + attr_accessor :skip_secondary_validations + array_attribute :other_names + belongs_to_creator + belongs_to_updater has_one :tag, :foreign_key => "name", :primary_key => "title" has_one :artist, -> {where(:is_active => true)}, :foreign_key => "name", :primary_key => "title" has_many :versions, -> {order("wiki_page_versions.id ASC")}, :class_name => "WikiPageVersion", :dependent => :destroy @@ -29,17 +31,16 @@ class WikiPage < ApplicationRecord order("updated_at DESC").limit(25) end - def other_names_equal(name) - query_sql = name.unicode_normalize(:nfkc).to_escaped_for_tsquery - where("other_names_index @@ to_tsquery('danbooru', E?)", query_sql) + def other_names_include(name) + where("wiki_pages.other_names @> ARRAY[?]", name.unicode_normalize(:nfkc)) end def other_names_match(name) if name =~ /\*/ - subquery = WikiPage.from("unnest(string_to_array(other_names, ' ')) AS other_name").where("other_name ILIKE ?", name.to_escaped_for_sql_like) + subquery = WikiPage.from("unnest(other_names) AS other_name").where("other_name ILIKE ?", name.to_escaped_for_sql_like) where(id: subquery) else - other_names_equal(name) + other_names_include(name) end end @@ -73,9 +74,9 @@ class WikiPage < ApplicationRecord end if params[:other_names_present].to_s.truthy? - q = q.where("other_names is not null and other_names != ''") + q = q.where("other_names is not null and other_names != '{}'") elsif params[:other_names_present].to_s.falsy? - q = q.where("other_names is null or other_names = ''") + q = q.where("other_names is null or other_names = '{}'") end q = q.attribute_matches(:is_locked, params[:is_locked]) @@ -97,7 +98,7 @@ class WikiPage < ApplicationRecord module ApiMethods def hidden_attributes - super + [:body_index, :other_names_index] + super + [:body_index] end def method_attributes @@ -145,8 +146,7 @@ class WikiPage < ApplicationRecord end def normalize_other_names - normalized_other_names = other_names.to_s.unicode_normalize(:nfkc).scan(/[^[:space:]]+/) - self.other_names = normalized_other_names.uniq.join(" ") + self.other_names = other_names.map { |name| name.unicode_normalize(:nfkc) }.uniq end def skip_secondary_validations=(value) @@ -224,8 +224,4 @@ class WikiPage < ApplicationRecord def visible? artist.blank? || !artist.is_banned? || CurrentUser.is_builder? end - - def other_names_array - other_names.to_s.split(/[[:space:]]+/) - end end diff --git a/app/models/wiki_page_version.rb b/app/models/wiki_page_version.rb index 3db9ca94e..1f39a5467 100644 --- a/app/models/wiki_page_version.rb +++ b/app/models/wiki_page_version.rb @@ -1,4 +1,5 @@ class WikiPageVersion < ApplicationRecord + array_attribute :other_names belongs_to :wiki_page belongs_to_updater belongs_to :artist, optional: true @@ -45,8 +46,4 @@ class WikiPageVersion < ApplicationRecord def category_name Tag.category_for(title) end - - def other_names_array - other_names.to_s.split(/[[:space:]]+/) - end end diff --git a/app/views/wiki_pages/_form.html.erb b/app/views/wiki_pages/_form.html.erb index 737f3c9b7..48ebbd2db 100644 --- a/app/views/wiki_pages/_form.html.erb +++ b/app/views/wiki_pages/_form.html.erb @@ -10,7 +10,7 @@

<%= @wiki_page.pretty_title %>

<% end %> - <%= f.input :other_names, :as => :text, :label => "Other names (view help)".html_safe, :hint => "Names used for this tag on other sites such as Pixiv. Separate with spaces." %> + <%= f.input :other_names_string, as: :text, label: "Other names (#{link_to "help", wiki_pages_path(title: "help:translated_tags")})".html_safe, hint: "Names used for this tag on other sites such as Pixiv. Separate with spaces." %> <%= dtext_field "wiki_page", "body" %> diff --git a/db/migrate/20181113174914_change_strings_to_arrays_on_wiki_pages.rb b/db/migrate/20181113174914_change_strings_to_arrays_on_wiki_pages.rb new file mode 100644 index 000000000..9537993ca --- /dev/null +++ b/db/migrate/20181113174914_change_strings_to_arrays_on_wiki_pages.rb @@ -0,0 +1,26 @@ +class ChangeStringsToArraysOnWikiPages < ActiveRecord::Migration[5.2] + def up + WikiPage.without_timeout do + change_column :wiki_pages, :other_names, "text[]", using: "string_to_array(other_names, ' ')::text[]", default: "{}" + change_column :wiki_page_versions, :other_names, "text[]", using: "string_to_array(other_names, ' ')::text[]", default: "{}" + + remove_column :wiki_pages, :other_names_index + execute "DROP TRIGGER trigger_wiki_pages_on_update_for_other_names ON wiki_pages" + + add_index :wiki_pages, :other_names, using: :gin + end + end + + def down + WikiPage.without_timeout do + remove_index :wiki_pages, :other_names + + add_column :wiki_pages, :other_names_index, :tsvector + add_index :wiki_pages, :other_names_index, using: :gin + execute "CREATE TRIGGER trigger_wiki_pages_on_update_for_other_names BEFORE INSERT OR UPDATE ON wiki_pages FOR EACH ROW EXECUTE PROCEDURE tsvector_update_trigger('other_names_index', 'public.danbooru', 'other_names')" + + change_column :wiki_pages, :other_names, "text", using: "array_to_string(other_names, ' ')", default: nil + change_column :wiki_page_versions, :other_names, "text", using: "array_to_string(other_names, ' ')", default: nil + end + end +end diff --git a/db/structure.sql b/db/structure.sql index 7cdd77c0c..9227e739f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -3363,7 +3363,7 @@ CREATE TABLE public.wiki_page_versions ( is_locked boolean NOT NULL, created_at timestamp without time zone, updated_at timestamp without time zone, - other_names text, + other_names text[] DEFAULT '{}'::text[], is_deleted boolean DEFAULT false NOT NULL ); @@ -3402,8 +3402,7 @@ CREATE TABLE public.wiki_pages ( created_at timestamp without time zone, updated_at timestamp without time zone, updater_id integer, - other_names text, - other_names_index tsvector, + other_names text[] DEFAULT '{}'::text[], is_deleted boolean DEFAULT false NOT NULL ); @@ -7312,10 +7311,10 @@ CREATE INDEX index_wiki_pages_on_body_index_index ON public.wiki_pages USING gin -- --- Name: index_wiki_pages_on_other_names_index; Type: INDEX; Schema: public; Owner: - +-- Name: index_wiki_pages_on_other_names; Type: INDEX; Schema: public; Owner: - -- -CREATE INDEX index_wiki_pages_on_other_names_index ON public.wiki_pages USING gin (other_names_index); +CREATE INDEX index_wiki_pages_on_other_names ON public.wiki_pages USING gin (other_names); -- @@ -7402,13 +7401,6 @@ CREATE TRIGGER trigger_posts_on_tag_index_update BEFORE INSERT OR UPDATE ON publ CREATE TRIGGER trigger_wiki_pages_on_update BEFORE INSERT OR UPDATE ON public.wiki_pages FOR EACH ROW EXECUTE PROCEDURE tsvector_update_trigger('body_index', 'public.danbooru', 'body', 'title'); --- --- Name: wiki_pages trigger_wiki_pages_on_update_for_other_names; Type: TRIGGER; Schema: public; Owner: - --- - -CREATE TRIGGER trigger_wiki_pages_on_update_for_other_names BEFORE INSERT OR UPDATE ON public.wiki_pages FOR EACH ROW EXECUTE PROCEDURE tsvector_update_trigger('other_names_index', 'public.danbooru', 'other_names'); - - -- -- PostgreSQL database dump complete -- @@ -7579,6 +7571,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20180913184128'), ('20180916002448'), ('20181108162204'), -('20181108205842'); +('20181108205842'), +('20181113174914'); diff --git a/test/unit/wiki_page_test.rb b/test/unit/wiki_page_test.rb index e37dac810..bcf1e5575 100644 --- a/test/unit/wiki_page_test.rb +++ b/test/unit/wiki_page_test.rb @@ -63,7 +63,7 @@ class WikiPageTest < ActiveSupport::TestCase should "normalize its other names" do @wiki_page.update(:other_names => "foo*bar baz baz 加賀（艦これ）") - assert(%w[foo*bar baz 加賀(艦これ)], @wiki_page.other_names_array) + assert_equal(%w[foo*bar baz 加賀(艦これ)], @wiki_page.other_names) end should "search by title" do