diff --git a/app/controllers/artists_controller.rb b/app/controllers/artists_controller.rb index 97fd01578..836be23c6 100644 --- a/app/controllers/artists_controller.rb +++ b/app/controllers/artists_controller.rb @@ -55,7 +55,7 @@ class ArtistsController < ApplicationController end def destroy - @artist.update_attribute(:is_active, false) + @artist.update_attribute(:is_deleted, true) redirect_to(artist_path(@artist), :notice => "Artist deleted") end @@ -94,7 +94,7 @@ class ArtistsController < ApplicationController end def artist_params(context = nil) - permitted_params = %i[name other_names other_names_string group_name url_string notes is_active] + permitted_params = %i[name other_names other_names_string group_name url_string notes is_deleted] permitted_params << { wiki_page_attributes: %i[id body] } permitted_params << :source if context == :new diff --git a/app/javascript/src/javascripts/autocomplete.js.erb b/app/javascript/src/javascripts/autocomplete.js.erb index 1a57746e3..9e052579b 100644 --- a/app/javascript/src/javascripts/autocomplete.js.erb +++ b/app/javascript/src/javascripts/autocomplete.js.erb @@ -328,7 +328,7 @@ Autocomplete.tag_source = async function(term) { Autocomplete.artist_source = async function(term) { let artists = await $.getJSON("/artists", { "search[name_like]": term.trim().replace(/\s+/g, "_") + "*", - "search[is_active]": true, + "search[is_deleted]": false, "search[order]": "post_count", "limit": Autocomplete.MAX_RESULTS, "expiry": 7 diff --git a/app/logical/post_sets/post.rb b/app/logical/post_sets/post.rb index c19fda4f5..a5347d7fe 100644 --- a/app/logical/post_sets/post.rb +++ b/app/logical/post_sets/post.rb @@ -37,7 +37,7 @@ module PostSets def artist return nil unless tag.present? && tag.category == Tag.categories.artist - return nil unless tag.artist.present? && tag.artist.is_active? + return nil unless tag.artist.present? && !tag.artist.is_deleted? tag.artist end diff --git a/app/models/artist.rb b/app/models/artist.rb index 3e53346b2..c51854912 100644 --- a/app/models/artist.rb +++ b/app/models/artist.rb @@ -20,8 +20,8 @@ class Artist < ApplicationRecord accepts_nested_attributes_for :wiki_page, update_only: true, reject_if: :all_blank - scope :active, -> { where(is_active: true) } - scope :deleted, -> { where(is_active: false) } + scope :active, -> { where(is_deleted: false) } + scope :deleted, -> { where(is_deleted: true) } scope :banned, -> { where(is_banned: true) } scope :unbanned, -> { where(is_banned: false) } @@ -158,7 +158,7 @@ class Artist < ApplicationRecord while artists.empty? && url.size > 10 u = url.sub(/\/+$/, "") + "/" u = u.to_escaped_for_sql_like.gsub(/\*/, '%') + '%' - artists += Artist.joins(:urls).where(["artists.is_active = TRUE AND artist_urls.normalized_url LIKE ? ESCAPE E'\\\\'", u]).limit(10).order("artists.name").all + artists += Artist.joins(:urls).where(["artists.is_deleted = FALSE AND artist_urls.normalized_url LIKE ? ESCAPE E'\\\\'", u]).limit(10).order("artists.name").all url = File.dirname(url) + "/" break if url =~ SITE_BLACKLIST_REGEXP @@ -221,7 +221,7 @@ class Artist < ApplicationRecord module VersionMethods def create_version(force = false) - if saved_change_to_name? || url_string_changed || saved_change_to_is_active? || saved_change_to_is_banned? || saved_change_to_other_names? || saved_change_to_group_name? || force + if saved_change_to_name? || url_string_changed || saved_change_to_is_deleted? || saved_change_to_is_banned? || saved_change_to_other_names? || saved_change_to_group_name? || force if merge_version? merge_version else @@ -237,7 +237,7 @@ class Artist < ApplicationRecord :updater_id => CurrentUser.id, :updater_ip_addr => CurrentUser.ip_addr, :urls => url_array, - :is_active => is_active, + :is_deleted => is_deleted, :is_banned => is_banned, :other_names => other_names, :group_name => group_name @@ -246,7 +246,7 @@ class Artist < ApplicationRecord def merge_version prev = versions.last - prev.update(name: name, urls: url_array, is_active: is_active, is_banned: is_banned, other_names: other_names, group_name: group_name) + prev.update(name: name, urls: url_array, is_deleted: is_deleted, is_banned: is_banned, other_names: other_names, group_name: group_name) end def merge_version? @@ -261,7 +261,7 @@ class Artist < ApplicationRecord self.name = version.name self.url_string = version.urls.join("\n") - self.is_active = version.is_active + self.is_deleted = version.is_deleted self.other_names = version.other_names self.group_name = version.group_name save @@ -296,7 +296,7 @@ class Artist < ApplicationRecord module TagMethods def validate_tag_category - return unless is_active? && name_changed? + return unless !is_deleted? && name_changed? if tag.category_name == "General" tag.update(category: Tag.categories.artist) @@ -385,7 +385,7 @@ class Artist < ApplicationRecord def search(params) q = super - q = q.search_attributes(params, :is_active, :is_banned, :name, :group_name, :other_names) + q = q.search_attributes(params, :is_deleted, :is_banned, :name, :group_name, :other_names) if params[:any_other_name_like] q = q.any_other_name_like(params[:any_other_name_like]) @@ -433,14 +433,14 @@ class Artist < ApplicationRecord extend SearchMethods def status - if is_banned? && is_active? + if is_banned? && !is_deleted? "Banned" elsif is_banned? "Banned Deleted" - elsif is_active? - "Active" - else + elsif is_deleted? "Deleted" + else + "Active" end end diff --git a/app/models/artist_version.rb b/app/models/artist_version.rb index 74c37ad19..b8dc2d705 100644 --- a/app/models/artist_version.rb +++ b/app/models/artist_version.rb @@ -52,11 +52,11 @@ class ArtistVersion < ApplicationRecord end def was_deleted - !is_active && previous.is_active + is_deleted && !previous.is_deleted end def was_undeleted - is_active && !previous.is_active + !is_deleted && previous.is_deleted end def was_banned diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 1f7d45021..37d3e3a07 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -15,7 +15,7 @@ class WikiPage < ApplicationRecord array_attribute :other_names has_one :tag, :foreign_key => "name", :primary_key => "title" - has_one :artist, -> {where(:is_active => true)}, :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 diff --git a/app/views/artists/_search.html.erb b/app/views/artists/_search.html.erb index ccca9f484..f15a6bdeb 100644 --- a/app/views/artists/_search.html.erb +++ b/app/views/artists/_search.html.erb @@ -1,7 +1,7 @@ <%= search_form_for(artists_path) do |f| %> <%= f.input :any_name_matches, label: "Name", hint: "Use * for wildcard", input_html: { value: params[:search][:any_name_matches], data: { autocomplete: "artist" }} %> <%= f.input :url_matches, label: "URL", as: "string", input_html: { value: params[:search][:url_matches] } %> - <%= f.input :is_active, label: "Active?", collection: [["Yes", true], ["No", false]], include_blank: true, selected: params[:search][:is_active] %> + <%= f.input :is_deleted, label: "Deleted?", collection: [["Yes", true], ["No", false]], include_blank: true, selected: params[:search][:is_deleted] %> <%= f.input :is_banned, label: "Banned?", collection: [["Yes", true], ["No", false]], include_blank: true, selected: params[:search][:is_banned] %> <%= f.input :has_tag, label: "Has tag?", collection: [["Yes", true], ["No", false]], include_blank: true, selected: params[:search][:has_tag] %> <%= f.input :order, collection: [["Recently created", "created_at"], ["Last updated", "updated_at"], ["Name", "name"], ["Post count", "post_count"]], selected: params[:search][:order] %> diff --git a/app/views/artists/_secondary_links.html.erb b/app/views/artists/_secondary_links.html.erb index f67a0d8ea..0bd6e7d80 100644 --- a/app/views/artists/_secondary_links.html.erb +++ b/app/views/artists/_secondary_links.html.erb @@ -16,10 +16,10 @@ <% end %> <%= subnav_link_to "History", artist_versions_path(:search => {:artist_id => @artist.id}) %> <% if CurrentUser.is_member? %> - <% if @artist.is_active? %> - <%= subnav_link_to "Delete", artist_path(@artist), method: :delete, "data-shortcut": "shift+d", "data-confirm": "Are you sure you want to delete this artist?" %> + <% if @artist.is_deleted? %> + <%= subnav_link_to "Undelete", artist_path(@artist, format: "js"), method: :put, data: {confirm: "Are you sure you want to undelete this artist?", params: "artist[is_deleted]=false"}, remote: true %> <% else %> - <%= subnav_link_to "Undelete", artist_path(@artist, format: "js"), method: :put, data: {confirm: "Are you sure you want to undelete this artist?", params: "artist[is_active]=true"}, remote: true %> + <%= subnav_link_to "Delete", artist_path(@artist), method: :delete, "data-shortcut": "shift+d", "data-confirm": "Are you sure you want to delete this artist?" %> <% end %> <% end %> <% if CurrentUser.is_admin? %> diff --git a/app/views/artists/index.html.erb b/app/views/artists/index.html.erb index b707e1def..deb76e6ad 100644 --- a/app/views/artists/index.html.erb +++ b/app/views/artists/index.html.erb @@ -18,8 +18,8 @@ <% end %> <% end %> <% t.column "Status" do |artist| %> - <% if !artist.is_active? %> - <%= link_to "Deleted", artists_path(search: { is_active: false }) %> + <% if artist.is_deleted? %> + <%= link_to "Deleted", artists_path(search: { is_deleted: true }) %> <% end %> <% if artist.is_banned? %> @@ -33,10 +33,10 @@ <% if CurrentUser.is_member? %> <%= link_to "Edit", edit_artist_path(artist) %> - <% if artist.is_active? %> - | <%= link_to "Delete", artist_path(artist, artist: { is_active: false }), method: :put, remote: true %> + <% if artist.is_deleted? %> + | <%= link_to "Undelete", artist_path(artist, artist: { is_deleted: false }), method: :put, remote: true %> <% else %> - | <%= link_to "Undelete", artist_path(artist, artist: { is_active: true }), method: :put, remote: true %> + | <%= link_to "Delete", artist_path(artist, artist: { is_deleted: true }), method: :put, remote: true %> <% end %> <% end %> <% end %> diff --git a/app/views/legacy/artists.xml.erb b/app/views/legacy/artists.xml.erb index f545b6051..60b65e14e 100644 --- a/app/views/legacy/artists.xml.erb +++ b/app/views/legacy/artists.xml.erb @@ -1,6 +1,6 @@ <% @artists.each do |artist| %> - " is_active="<%= artist.is_active? %>" name="<%= artist.name %>" updater_id="0" id="<%= artist.id %>" version="0"/> + " is_active="<%= !artist.is_deleted? %>" name="<%= artist.name %>" updater_id="0" id="<%= artist.id %>" version="0"/> <% end %> diff --git a/db/migrate/20200306202253_rename_is_active_on_artists.rb b/db/migrate/20200306202253_rename_is_active_on_artists.rb new file mode 100644 index 000000000..7c84586d9 --- /dev/null +++ b/db/migrate/20200306202253_rename_is_active_on_artists.rb @@ -0,0 +1,25 @@ +class RenameIsActiveOnArtists < ActiveRecord::Migration[6.0] + def up + execute "SET statement_timeout = 0" + + rename_column :artists, :is_active, :is_deleted + change_column_default :artists, :is_deleted, from: true, to: false + execute "UPDATE artists SET is_deleted = NOT is_deleted" + + rename_column :artist_versions, :is_active, :is_deleted + change_column_default :artist_versions, :is_deleted, from: true, to: false + execute "UPDATE artist_versions SET is_deleted = NOT is_deleted" + end + + def down + execute "SET statement_timeout = 0" + + execute "UPDATE artists SET is_deleted = NOT is_deleted" + change_column_default :artists, :is_deleted, from: false, to: true + rename_column :artists, :is_deleted, :is_active + + execute "UPDATE artist_versions SET is_deleted = NOT is_deleted" + change_column_default :artist_versions, :is_deleted, from: false, to: true + rename_column :artist_versions, :is_deleted, :is_active + end +end diff --git a/db/structure.sql b/db/structure.sql index de5da94b8..a407c7333 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -594,7 +594,7 @@ ALTER SEQUENCE public.artist_versions_id_seq OWNED BY public.artist_versions.id; CREATE TABLE public.artists ( id integer NOT NULL, name character varying NOT NULL, - is_active boolean DEFAULT true NOT NULL, + is_deleted boolean DEFAULT false NOT NULL, is_banned boolean DEFAULT false NOT NULL, other_names text[] DEFAULT '{}'::text[] NOT NULL, group_name character varying DEFAULT ''::character varying NOT NULL, @@ -7307,6 +7307,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200123184743'), ('20200217044719'), ('20200223042415'), -('20200223234015'); +('20200223234015'), +('20200306202253'); diff --git a/test/factories/artist.rb b/test/factories/artist.rb index def197791..5240d23cd 100644 --- a/test/factories/artist.rb +++ b/test/factories/artist.rb @@ -1,6 +1,6 @@ FactoryBot.define do factory(:artist) do name { rand(1_000_000).to_s } - is_active { true } + is_deleted { false } end end diff --git a/test/functional/artists_controller_test.rb b/test/functional/artists_controller_test.rb index 39b3841b3..83d11fcc9 100644 --- a/test/functional/artists_controller_test.rb +++ b/test/functional/artists_controller_test.rb @@ -127,7 +127,7 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest should "create an artist" do attributes = FactoryBot.attributes_for(:artist) assert_difference("Artist.count", 1) do - attributes.delete(:is_active) + attributes.delete(:is_deleted) post_auth artists_path, @user, params: {artist: attributes} end artist = Artist.find_by_name(attributes[:name]) @@ -171,14 +171,14 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest delete_auth artist_path(@artist.id), @builder assert_redirected_to(artist_path(@artist.id)) @artist.reload - assert_equal(false, @artist.is_active) + assert_equal(true, @artist.is_deleted) end should "undelete an artist" do @builder = create(:builder_user) - put_auth artist_path(@artist.id), @builder, params: {artist: {is_active: true}} + put_auth artist_path(@artist.id), @builder, params: {artist: {is_deleted: false}} assert_redirected_to(artist_path(@artist.id)) - assert_equal(true, @artist.reload.is_active) + assert_equal(false, @artist.reload.is_deleted) end context "reverting an artist" do diff --git a/test/unit/artist_test.rb b/test/unit/artist_test.rb index deca85f5e..16aa3eafe 100644 --- a/test/unit/artist_test.rb +++ b/test/unit/artist_test.rb @@ -191,7 +191,7 @@ class ArtistTest < ActiveSupport::TestCase end should "hide deleted artists" do - FactoryBot.create(:artist, :name => "warhol", :url_string => "http://warhol.com/a/image.jpg", :is_active => false) + create(:artist, name: "warhol", url_string: "http://warhol.com/a/image.jpg", is_deleted: true) assert_artist_not_found("http://warhol.com/a/image.jpg") end @@ -512,7 +512,7 @@ class ArtistTest < ActiveSupport::TestCase context "that is deleted" do setup do @artist = create(:artist, url_string: "https://google.com") - @artist.update_attribute(:is_active, false) + @artist.update_attribute(:is_deleted, true) @artist.reload end diff --git a/test/unit/artist_url_test.rb b/test/unit/artist_url_test.rb index e66822c53..f1cce9149 100644 --- a/test/unit/artist_url_test.rb +++ b/test/unit/artist_url_test.rb @@ -191,12 +191,12 @@ class ArtistUrlTest < ActiveSupport::TestCase subject { ArtistUrl } should "work" do - @bkub = FactoryBot.create(:artist, name: "bkub", is_active: true, url_string: "https://bkub.com") - @masao = FactoryBot.create(:artist, name: "masao", is_active: false, url_string: "-https://masao.com") + @bkub = create(:artist, name: "bkub", url_string: "https://bkub.com") + @masao = create(:artist, name: "masao", url_string: "-https://masao.com") @bkub_url = @bkub.urls.first @masao_url = @masao.urls.first - assert_search_equals([@bkub_url], is_active: true) + assert_search_equals([@bkub_url], is_deleted: false) assert_search_equals([@bkub_url], artist: { name: "bkub" }) assert_search_equals([@bkub_url], url_matches: "*bkub*")