From 741462ae6828da03752fee5e01b118ffeb360be2 Mon Sep 17 00:00:00 2001 From: evazion Date: Wed, 14 Nov 2018 14:22:06 -0600 Subject: [PATCH] artist versions: convert other_names, url_string to arrays (#3987). --- app/models/artist.rb | 12 ++++++------ app/models/artist_version.rb | 19 +++++++------------ ...ge_strings_to_arrays_on_artist_versions.rb | 19 +++++++++++++++++++ db/structure.sql | 7 ++++--- test/unit/artist_test.rb | 12 ++++++------ 5 files changed, 42 insertions(+), 27 deletions(-) create mode 100644 db/migrate/20181114185032_change_strings_to_arrays_on_artist_versions.rb diff --git a/app/models/artist.rb b/app/models/artist.rb index af2b08ce8..327049170 100644 --- a/app/models/artist.rb +++ b/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 diff --git a/app/models/artist_version.rb b/app/models/artist_version.rb index 3bfa6c7f1..029b20546 100644 --- a/app/models/artist_version.rb +++ b/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 diff --git a/db/migrate/20181114185032_change_strings_to_arrays_on_artist_versions.rb b/db/migrate/20181114185032_change_strings_to_arrays_on_artist_versions.rb new file mode 100644 index 000000000..c0a696a8e --- /dev/null +++ b/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 diff --git a/db/structure.sql b/db/structure.sql index 31a94f6d5..97d96fbaf 100644 --- a/db/structure.sql +++ b/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'); diff --git a/test/unit/artist_test.rb b/test/unit/artist_test.rb index e7559022d..bd40c45ae 100644 --- a/test/unit/artist_test.rb +++ b/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