artist versions: convert other_names, url_string to arrays (#3987).

app/models/artist.rb

@@ -174,11 +174,11 @@ class Artist < ApplicationRecord
     end
 
     def url_array
-      urls.map(&:to_s)
+      urls.map(&:to_s).sort
     end
 
     def url_string
-      url_array.sort.join("\n")
+      url_array.join("\n")
     end
 
     def url_string=(string)
@@ -275,7 +275,7 @@ class Artist < ApplicationRecord
         :name => name,
         :updater_id => CurrentUser.id,
         :updater_ip_addr => CurrentUser.ip_addr,
-        :url_string => url_string,
+        :urls => url_array,
         :is_active => is_active,
         :is_banned => is_banned,
         :other_names => other_names,
@@ -287,7 +287,7 @@ class Artist < ApplicationRecord
       prev = versions.last
       prev.update_attributes(
         :name => name,
-        :url_string => url_string,
+        :urls => url_array,
         :is_active => is_active,
         :is_banned => is_banned,
         :other_names => other_names,
@@ -306,9 +306,9 @@ class Artist < ApplicationRecord
       end
 
       self.name = version.name
-      self.url_string = version.url_string
+      self.url_string = version.urls.join("\n")
       self.is_active = version.is_active
-      self.other_names = version.other_names
+      self.other_names = version.other_names.join(" ")
       self.group_name = version.group_name
       save
     end

app/models/artist_version.rb

@@ -1,4 +1,7 @@
 class ArtistVersion < ApplicationRecord
+  array_attribute :urls
+  array_attribute :other_names
+
   belongs_to_updater
   belongs_to :artist
   delegate :visible?, :to => :artist
@@ -47,18 +50,10 @@ class ArtistVersion < ApplicationRecord
 
   extend SearchMethods
 
-  def url_array
-    url_string.to_s.split(/[[:space:]]+/)
-  end
-
-  def other_names_array
-    other_names.to_s.split(/[[:space:]]+/)
-  end
-
   def urls_diff(version)
     latest_urls = artist.url_array || []
-    new_urls = url_array
-    old_urls = version.present? ? version.url_array : []
+    new_urls = urls
+    old_urls = version.present? ? version.urls : []
 
     added_urls = new_urls - old_urls
     removed_urls = old_urls - new_urls
@@ -74,8 +69,8 @@ class ArtistVersion < ApplicationRecord
 
   def other_names_diff(version)
     latest_names = artist.other_names_array || []
-    new_names = other_names_array
-    old_names = version.present? ? version.other_names_array : []
+    new_names = other_names
+    old_names = version.present? ? version.other_names : []
 
     added_names = new_names - old_names
     removed_names = old_names - new_names

db/migrate/20181114185032_change_strings_to_arrays_on_artist_versions.rb

@@ -0,0 +1,19 @@
+class ChangeStringsToArraysOnArtistVersions < ActiveRecord::Migration[5.2]
+  def up
+    ArtistVersion.without_timeout do
+      change_column :artist_versions, :other_names, "text[]", using: "array_remove(regexp_split_to_array(other_names, '\\s+'), '')", default: "{}"
+
+      change_column :artist_versions, :url_string, "text[]", using: "array_remove(regexp_split_to_array(url_string, '\\s+'), '')", default: "{}"
+      rename_column :artist_versions, :url_string, :urls
+    end
+  end
+
+  def down
+    ArtistVersion.without_timeout do
+      rename_column :artist_versions, :urls, :url_string
+      change_column :artist_versions, :url_string,  "text", using: "array_to_string(url_string,  '\\n')", default: nil
+
+      change_column :artist_versions, :other_names, "text", using: "array_to_string(other_names, ' ')", default: nil
+    end
+  end
+end

db/structure.sql

@@ -726,9 +726,9 @@ CREATE TABLE public.artist_versions (
     updater_id integer NOT NULL,
     updater_ip_addr inet NOT NULL,
     is_active boolean DEFAULT true NOT NULL,
-    other_names text,
+    other_names text[] DEFAULT '{}'::text[],
     group_name character varying,
-    url_string text,
+    urls text[] DEFAULT '{}'::text[],
     is_banned boolean DEFAULT false NOT NULL,
     created_at timestamp without time zone,
     updated_at timestamp without time zone
@@ -7573,6 +7573,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20181108162204'),
 ('20181108205842'),
 ('20181113174914'),
-('20181114180205');
+('20181114180205'),
+('20181114185032');
 
 

test/unit/artist_test.rb

@@ -475,7 +475,7 @@ class ArtistTest < ActiveSupport::TestCase
       end
 
       first_version = ArtistVersion.first
-      assert_equal("yyy", first_version.other_names)
+      assert_equal(%w[yyy], first_version.other_names)
       artist.revert_to!(first_version)
       artist.reload
       assert_equal("yyy", artist.other_names)
@@ -506,35 +506,35 @@ class ArtistTest < ActiveSupport::TestCase
       should "create a new version when an url is added" do
         assert_difference("ArtistVersion.count") do
           @artist.update(:url_string => "http://foo.com http://bar.com")
-          assert_equal(%w[http://bar.com http://foo.com], @artist.versions.last.url_array)
+          assert_equal(%w[http://bar.com http://foo.com], @artist.versions.last.urls)
         end
       end
 
       should "create a new version when an url is removed" do
         assert_difference("ArtistVersion.count") do
           @artist.update(:url_string => "")
-          assert_equal(%w[], @artist.versions.last.url_array)
+          assert_equal(%w[], @artist.versions.last.urls)
         end
       end
 
       should "create a new version when an url is marked inactive" do
         assert_difference("ArtistVersion.count") do
           @artist.update(:url_string => "-http://foo.com")
-          assert_equal(%w[-http://foo.com], @artist.versions.last.url_array)
+          assert_equal(%w[-http://foo.com], @artist.versions.last.urls)
         end
       end
 
       should "not create a new version when nothing has changed" do
         assert_no_difference("ArtistVersion.count") do
           @artist.save
-          assert_equal(%w[http://foo.com], @artist.versions.last.url_array)
+          assert_equal(%w[http://foo.com], @artist.versions.last.urls)
         end
       end
 
       should "not save invalid urls" do
         assert_no_difference("ArtistVersion.count") do
           @artist.update(:url_string => "http://foo.com www.example.com")
-          assert_equal(%w[http://foo.com], @artist.versions.last.url_array)
+          assert_equal(%w[http://foo.com], @artist.versions.last.urls)
         end
       end
     end