diff --git a/app/controllers/artists_controller.rb b/app/controllers/artists_controller.rb
index 21deaee0d..b06226798 100644
--- a/app/controllers/artists_controller.rb
+++ b/app/controllers/artists_controller.rb
@@ -105,7 +105,7 @@ private
end
def artist_params
- permitted_params = %i[name other_names other_names_comma group_name url_string notes]
+ permitted_params = %i[name other_names other_names_string group_name url_string notes]
permitted_params << :is_active if CurrentUser.is_builder?
params.fetch(:artist, {}).permit(permitted_params)
diff --git a/app/javascript/src/styles/specific/artists.scss b/app/javascript/src/styles/specific/artists.scss
index 0d82f9466..1149ef5ce 100644
--- a/app/javascript/src/styles/specific/artists.scss
+++ b/app/javascript/src/styles/specific/artists.scss
@@ -14,7 +14,7 @@ div#c-artists, div#excerpt {
textarea {
height: 10em;
- &#artist_other_names_comma {
+ &#artist_other_names_string {
height: 3em;
}
}
diff --git a/app/models/artist.rb b/app/models/artist.rb
index 327049170..509d09c1b 100644
--- a/app/models/artist.rb
+++ b/app/models/artist.rb
@@ -3,8 +3,10 @@ class Artist < ApplicationRecord
class RevertError < Exception ; end
attr_accessor :url_string_changed
+ array_attribute :other_names
before_validation :normalize_name
+ before_validation :normalize_other_names
after_save :create_version
after_save :categorize_tag
after_save :update_wiki
@@ -239,16 +241,8 @@ class Artist < ApplicationRecord
name.tr("_", " ")
end
- def other_names_array
- other_names.try(:split, /[[:space:]]+/)
- end
-
- def other_names_comma
- other_names_array.try(:join, ", ")
- end
-
- def other_names_comma=(string)
- self.other_names = string.split(/,/).map {|x| Artist.normalize_name(x)}.join(" ")
+ def normalize_other_names
+ self.other_names = other_names.map { |x| Artist.normalize_name(x) }
end
end
@@ -308,7 +302,7 @@ class Artist < ApplicationRecord
self.name = version.name
self.url_string = version.urls.join("\n")
self.is_active = version.is_active
- self.other_names = version.other_names.join(" ")
+ self.other_names = version.other_names
self.group_name = version.group_name
save
end
@@ -466,13 +460,21 @@ class Artist < ApplicationRecord
where(name: normalize_name(name))
end
+ def any_other_name_matches(regex)
+ where(id: Artist.from("unnest(other_names) AS other_name").where("other_name ~ ?", regex))
+ end
+
+ def any_other_name_like(name)
+ where(id: Artist.from("unnest(other_names) AS other_name").where("other_name LIKE ?", name.to_escaped_for_sql_like))
+ end
+
def any_name_matches(query)
if query =~ %r!\A/(.*)/\z!
- where_regex(:name, $1).or(where_regex(:other_names, $1)).or(where_regex(:group_name, $1))
+ where_regex(:name, $1).or(any_other_name_matches($1)).or(where_regex(:group_name, $1))
else
normalized_name = normalize_name(query)
normalized_name = "*#{normalized_name}*" unless normalized_name.include?("*")
- where_like(:name, normalized_name).or(where_like(:other_names, normalized_name)).or(where_like(:group_name, normalized_name))
+ where_like(:name, normalized_name).or(any_other_name_like(normalized_name)).or(where_like(:group_name, normalized_name))
end
end
@@ -492,9 +494,12 @@ class Artist < ApplicationRecord
q = super
q = q.search_text_attribute(:name, params)
- q = q.search_text_attribute(:other_names, params)
q = q.search_text_attribute(:group_name, params)
+ if params[:any_other_name_like]
+ q = q.any_other_name_like(params[:any_other_name_like])
+ end
+
if params[:any_name_matches].present?
q = q.any_name_matches(params[:any_name_matches])
end
@@ -536,12 +541,6 @@ class Artist < ApplicationRecord
end
end
- module ApiMethods
- def hidden_attributes
- super + [:other_names_index]
- end
- end
-
include UrlMethods
include NameMethods
include GroupMethods
@@ -551,7 +550,6 @@ class Artist < ApplicationRecord
include TagMethods
include BanMethods
extend SearchMethods
- include ApiMethods
def status
if is_banned? && is_active?
diff --git a/app/models/artist_version.rb b/app/models/artist_version.rb
index 029b20546..2d116114a 100644
--- a/app/models/artist_version.rb
+++ b/app/models/artist_version.rb
@@ -68,7 +68,7 @@ class ArtistVersion < ApplicationRecord
end
def other_names_diff(version)
- latest_names = artist.other_names_array || []
+ latest_names = artist.other_names || []
new_names = other_names
old_names = version.present? ? version.other_names : []
diff --git a/app/views/artists/_form.html.erb b/app/views/artists/_form.html.erb
index 8dadd1fbe..7ae80753f 100644
--- a/app/views/artists/_form.html.erb
+++ b/app/views/artists/_form.html.erb
@@ -14,7 +14,7 @@
<%= @artist.name %>
<% end %>
- <%= f.input :other_names_comma, :hint => "Separate with commas", :as => :text, :label => "Other names" %>
+ <%= f.input :other_names_string, :hint => "Separate with spaces", :as => :text, :label => "Other names" %>
<%= f.input :group_name %>
<%= f.input :url_string, :label => "URLs", :as => :text, :input_html => {:size => "50x5", :value => params.dig(:artist, :url_string) || @artist.urls.join("\n")}, :hint => "You can prefix a URL with - to mark it as dead." %>
diff --git a/app/views/artists/_summary.html.erb b/app/views/artists/_summary.html.erb
index e9eb006d3..9c50f7561 100644
--- a/app/views/artists/_summary.html.erb
+++ b/app/views/artists/_summary.html.erb
@@ -7,7 +7,7 @@
Tag Alias <%= artist.tag_alias_name %>
<% end %>
<% if artist.other_names.present? %>
- Other Names <%= link_to_artists(artist.other_names.split(/ /)) %>
+ Other Names <%= link_to_artists(artist.other_names) %>
<% end %>
<% if artist.group_name.present? %>
Group <%= link_to_artist(artist.group_name) %>
diff --git a/app/views/artists/index.html.erb b/app/views/artists/index.html.erb
index 66c74fa14..2b8eca9ba 100644
--- a/app/views/artists/index.html.erb
+++ b/app/views/artists/index.html.erb
@@ -19,7 +19,7 @@
(group: <%= link_to(artist.group_name, artist_path(artist)) %>)
<% end %>
- <%= artist.other_names %> |
+ <%= artist.other_names_string %> |
<%= artist.status %> |
<% end %>
<% end %>
diff --git a/db/migrate/20181114202744_change_other_names_to_array_on_artists.rb b/db/migrate/20181114202744_change_other_names_to_array_on_artists.rb
new file mode 100644
index 000000000..a56183c4f
--- /dev/null
+++ b/db/migrate/20181114202744_change_other_names_to_array_on_artists.rb
@@ -0,0 +1,23 @@
+class ChangeOtherNamesToArrayOnArtists < ActiveRecord::Migration[5.2]
+ def up
+ Artist.without_timeout do
+ remove_index :artists, name: "index_artists_on_other_names_trgm"
+ change_column :artists, :other_names, "text[]", using: "array_remove(regexp_split_to_array(other_names, '\\s+'), '')", default: "{}"
+ add_index :artists, :other_names, using: :gin
+
+ remove_column :artists, :other_names_index
+ execute "DROP TRIGGER trigger_artists_on_update ON artists"
+ end
+ end
+
+ def down
+ Artist.without_timeout do
+ remove_index :artists, :other_names
+ change_column :artists, :other_names, "text", using: "array_to_string(other_names, ' ')", default: nil
+ add_index :artists, :other_names, name: "index_artists_on_other_names_trgm", using: :gin, opclass: :gin_trgm_ops
+
+ add_column :artists, :other_names_index, :tsvector
+ execute "CREATE TRIGGER trigger_artists_on_update BEFORE INSERT OR UPDATE ON artists FOR EACH ROW EXECUTE PROCEDURE tsvector_update_trigger('other_names_index', 'public.danbooru', 'other_names')"
+ end
+ end
+end
diff --git a/db/structure.sql b/db/structure.sql
index 97d96fbaf..81291c3c1 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -765,8 +765,7 @@ CREATE TABLE public.artists (
creator_id integer NOT NULL,
is_active boolean DEFAULT true NOT NULL,
is_banned boolean DEFAULT false NOT NULL,
- other_names text,
- other_names_index tsvector,
+ other_names text[] DEFAULT '{}'::text[],
group_name character varying,
created_at timestamp without time zone,
updated_at timestamp without time zone
@@ -5078,17 +5077,10 @@ CREATE INDEX index_artists_on_name_trgm ON public.artists USING gin (name public
--
--- Name: index_artists_on_other_names_index; Type: INDEX; Schema: public; Owner: -
+-- Name: index_artists_on_other_names; Type: INDEX; Schema: public; Owner: -
--
-CREATE INDEX index_artists_on_other_names_index ON public.artists USING gin (other_names_index);
-
-
---
--- Name: index_artists_on_other_names_trgm; Type: INDEX; Schema: public; Owner: -
---
-
-CREATE INDEX index_artists_on_other_names_trgm ON public.artists USING gin (other_names public.gin_trgm_ops);
+CREATE INDEX index_artists_on_other_names ON public.artists USING gin (other_names);
--
@@ -7345,13 +7337,6 @@ CREATE INDEX index_wiki_pages_on_updated_at ON public.wiki_pages USING btree (up
CREATE TRIGGER insert_favorites_trigger BEFORE INSERT ON public.favorites FOR EACH ROW EXECUTE PROCEDURE public.favorites_insert_trigger();
---
--- Name: artists trigger_artists_on_update; Type: TRIGGER; Schema: public; Owner: -
---
-
-CREATE TRIGGER trigger_artists_on_update BEFORE INSERT OR UPDATE ON public.artists FOR EACH ROW EXECUTE PROCEDURE tsvector_update_trigger('other_names_index', 'public.danbooru', 'other_names');
-
-
--
-- Name: comments trigger_comments_on_update; Type: TRIGGER; Schema: public; Owner: -
--
@@ -7574,6 +7559,7 @@ INSERT INTO "schema_migrations" (version) VALUES
('20181108205842'),
('20181113174914'),
('20181114180205'),
-('20181114185032');
+('20181114185032'),
+('20181114202744');
diff --git a/test/unit/artist_test.rb b/test/unit/artist_test.rb
index bd40c45ae..76117910d 100644
--- a/test/unit/artist_test.rb
+++ b/test/unit/artist_test.rb
@@ -407,8 +407,8 @@ class ArtistTest < ActiveSupport::TestCase
end
should "normalize its other names" do
- artist = FactoryBot.create(:artist, :name => "a1", :other_names_comma => "aaa, bbb, ccc ddd")
- assert_equal("aaa, bbb, ccc_ddd", artist.other_names_comma)
+ artist = FactoryBot.create(:artist, :name => "a1", :other_names => "aaa bbb ccc_ddd")
+ assert_equal("aaa bbb ccc_ddd", artist.other_names_string)
end
should "search on its name should return results" do
@@ -421,11 +421,11 @@ class ArtistTest < ActiveSupport::TestCase
end
should "search on other names should return matches" do
- artist = FactoryBot.create(:artist, :name => "artist", :other_names_comma => "aaa, ccc ddd")
+ artist = FactoryBot.create(:artist, :name => "artist", :other_names_string => "aaa ccc_ddd")
- assert_nil(Artist.search(other_names_like: "*artist*").first)
- assert_not_nil(Artist.search(other_names_like: "*aaa*").first)
- assert_not_nil(Artist.search(other_names_like: "*ccc_ddd*").first)
+ assert_nil(Artist.search(any_other_name_like: "*artist*").first)
+ assert_not_nil(Artist.search(any_other_name_like: "*aaa*").first)
+ assert_not_nil(Artist.search(any_other_name_like: "*ccc_ddd*").first)
assert_not_nil(Artist.search(name: "artist").first)
assert_not_nil(Artist.search(:any_name_matches => "aaa").first)
assert_not_nil(Artist.search(:any_name_matches => "/a/").first)
@@ -478,7 +478,7 @@ class ArtistTest < ActiveSupport::TestCase
assert_equal(%w[yyy], first_version.other_names)
artist.revert_to!(first_version)
artist.reload
- assert_equal("yyy", artist.other_names)
+ assert_equal(%w[yyy], artist.other_names)
end
should "update the category of the tag when created" do