artists: rename is_active flag to is_deleted.

Rename is_active to is_deleted. This is for better consistency with
other models, and to reduce confusion over what "active" means for
artists. Sometimes users think active is for whether the artist is
actively producing work.
This commit is contained in:
evazion
2020-03-06 14:50:21 -06:00
parent 49a3538933
commit 4c11e339bd
16 changed files with 68 additions and 42 deletions

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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] %>

View File

@@ -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? %>

View File

@@ -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 %>

View File

@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<artists type="array">
<% @artists.each do |artist| %>
<artist group_name="<%= artist.group_name %>" other_names="<%= artist.other_names %>" urls="<%= artist.urls.map {|x| x.url}.join(" ") %>" is_active="<%= artist.is_active? %>" name="<%= artist.name %>" updater_id="0" id="<%= artist.id %>" version="0"/>
<artist group_name="<%= artist.group_name %>" other_names="<%= artist.other_names %>" urls="<%= artist.urls.map {|x| x.url}.join(" ") %>" is_active="<%= !artist.is_deleted? %>" name="<%= artist.name %>" updater_id="0" id="<%= artist.id %>" version="0"/>
<% end %>
</artists>

View File

@@ -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

View File

@@ -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');

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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*")