jaculabilis/danbooru
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

artists/edit: refactor editing nested wiki pages.

Browse Source 
Refactor to use accepts_nested_attributes_for instead of the notes
attribute to facilitate editing wikis on the artist edit page.

This fixes the notes attribute unintentionally showing up in the API.

This also changes it so that renaming an artist entry doesn't
automatically rename the corresponding wiki page. This had bad behavior
when there was a conflict between wiki pages (the wikis would be
silently merged, which usually isn't what you want). It also didn't warn
about wiki links being broken by renames.
This commit is contained in:
evazion
2020-02-16 18:27:57 -06:00
parent 6f3d0b542c
commit ef3188a7fe
6 changed files with 22 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
app/controllers/artists_controller.rb
View File

@@ -6,10 +6,12 @@ class ArtistsController < ApplicationController
def new
@artist = Artist.new_with_defaults(artist_params(:new))
@artist.build_wiki_page if @artist.wiki_page.nil?
respond_with(@artist)
end
def edit
@artist.build_wiki_page if @artist.wiki_page.nil?
respond_with(@artist)
end
@@ -93,6 +95,7 @@ class ArtistsController < ApplicationController
def artist_params(context = nil)
permitted_params = %i[name other_names other_names_string group_name url_string notes is_active]
permitted_params << { wiki_page_attributes: %i[id body] }
permitted_params << :source if context == :new
params.fetch(:artist, {}).permit(permitted_params)

65
app/models/artist.rb
View File

@@ -8,7 +8,6 @@ class Artist < ApplicationRecord
before_validation :normalize_name
before_validation :normalize_other_names
after_save :create_version
after_save :update_wiki
after_save :clear_url_string_changed
validate :validate_tag_category
validates :name, tag_name: true, uniqueness: true
@@ -19,7 +18,8 @@ class Artist < ApplicationRecord
has_one :wiki_page, :foreign_key => "title", :primary_key => "name"
has_one :tag_alias, :foreign_key => "antecedent_name", :primary_key => "name"
belongs_to :tag, foreign_key: "name", primary_key: "name", default: -> { Tag.new(name: name, category: Tag.categories.artist) }
attribute :notes, :string
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) }
@@ -222,7 +222,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? || saved_change_to_notes? || force
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 merge_version?
merge_version
else
@@ -295,64 +295,6 @@ class Artist < ApplicationRecord
end
end
module NoteMethods
extend ActiveSupport::Concern
def notes
@notes || wiki_page.try(:body)
end
def notes=(text)
if notes != text
notes_will_change!
@notes = text
end
end
def reload(options = nil)
flush_cache
if instance_variable_defined?(:@notes)
remove_instance_variable(:@notes)
end
super
end
def notes_changed?
attribute_changed?("notes")
end
def notes_will_change!
attribute_will_change!("notes")
end
def update_wiki
if persisted? && saved_change_to_name? && attribute_before_last_save("name").present? && WikiPage.titled(attribute_before_last_save("name")).exists?
# we're renaming the artist, so rename the corresponding wiki page
old_page = WikiPage.titled(name_before_last_save).first
if wiki_page.present?
# a wiki page with the new name already exists, so update the content
wiki_page.update(body: "#{wiki_page.body}\n\n#{@notes}")
else
# a wiki page doesn't already exist for the new name, so rename the old one
old_page.update(title: name, body: @notes)
end
elsif wiki_page.nil?
# if there are any notes, we need to create a new wiki page
if @notes.present?
create_wiki_page(body: @notes, title: name)
end
elsif (!@notes.nil? && (wiki_page.body != @notes)) || wiki_page.title != name
# if anything changed, we need to update the wiki page
wiki_page.body = @notes unless @notes.nil?
wiki_page.title = name
wiki_page.save
end
end
end
module TagMethods
def validate_tag_category
return unless is_active? && name_changed?
@@ -487,7 +429,6 @@ class Artist < ApplicationRecord
include NameMethods
include VersionMethods
extend FactoryMethods
include NoteMethods
include TagMethods
include BanMethods
extend SearchMethods

14
app/views/artists/_form.html.erb
View File

@@ -1,15 +1,13 @@
<%= edit_form_for(@artist) do |f| %>
<% if @artist.new_record? %>
<%= f.input :name, as: :string, input_html: { data: { autocomplete: "tag" } } %>
<% else %>
<%= f.input :name, as: :string, input_html: { data: { autocomplete: "tag" } }, hint: "Change to rename this artist entry and its wiki page" %>
<% end %>
<%= f.input :name, as: :string, input_html: { "data-autocomplete": "tag" } %>
<%= f.input :other_names_string, label: "Other names", as: :text, input_html: { size: "50x1" }, hint: '<b style="color: red;">NEW</b> Separate names with spaces, not commas. Use underscores for spaces inside names.'.html_safe %>
<%= 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." %>
<%= f.input :url_string, :label => "URLs", :as => :text, :input_html => {:size => "50x15", :value => params.dig(:artist, :url_string) || @artist.urls.join("\n")}, :hint => "You can prefix a URL with - to mark it as dead." %>
<%= f.simple_fields_for :wiki_page do |fw| %>
<%= fw.input :body, label: "Wiki", input_html: { size: "50x15" } %>
<% end %>
<%= dtext_field "artist", "notes" %>
<%= f.button :submit, "Submit" %>
<%= dtext_preview_button "artist", "notes" %>
<% end %>

4
app/views/artists/_show.html.erb
View File

@@ -5,9 +5,9 @@
<div id="a-show">
<h1>Artist: <%= link_to @artist.pretty_name, posts_path(tags: @artist.name), class: tag_class(@artist.tag) %></h1>
<% if @artist.notes.present? %>
<% if @artist.wiki_page.present? %>
<div class="prose">
<%= format_text(@artist.notes, :disable_mentions => true) %>
<%= format_text(@artist.wiki_page.body, :disable_mentions => true) %>
</div>
<p><%= link_to "View wiki page", @artist.wiki_page %></p>

37
test/functional/artists_controller_test.rb
View File

@@ -32,7 +32,7 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest
@admin = create(:admin_user)
@user = create(:user)
as_user do
@artist = create(:artist, notes: "message")
@artist = create(:artist)
@masao = create(:artist, name: "masao", url_string: "http://www.pixiv.net/member.php?id=32777")
@artgerm = create(:artist, name: "artgerm", url_string: "http://artgerm.deviantart.com/")
end
@@ -135,23 +135,23 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest
assert_redirected_to(artist_path(artist.id))
end
context "with an artist that has notes" do
context "with an artist that has a wiki page" do
setup do
as(@admin) do
@artist = create(:artist, name: "aaa", notes: "testing", url_string: "http://example.com")
@artist = create(:artist, name: "aaa", url_string: "http://example.com")
@wiki_page = create(:wiki_page, title: "aaa", body: "testing")
end
@wiki_page = @artist.wiki_page
@another_user = create(:user)
end
should "update an artist" do
should "update the wiki with the artist" do
old_timestamp = @wiki_page.updated_at
travel(1.minute) do
put_auth artist_path(@artist.id), @user, params: {artist: {notes: "rex", url_string: "http://example.com\nhttp://monet.com"}}
put_auth artist_path(@artist.id), @user, params: {artist: { wiki_page_attributes: { body: "rex" }, url_string: "http://example.com\nhttp://monet.com"}}
end
@artist.reload
@wiki_page = @artist.wiki_page
assert_equal("rex", @artist.notes)
assert_equal("rex", @artist.wiki_page.body)
assert_not_equal(old_timestamp, @wiki_page.updated_at)
assert_redirected_to(artist_path(@artist.id))
end
@@ -160,31 +160,10 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest
old_timestamp = @wiki_page.updated_at
travel(1.minute)
as(@another_user) { @artist.update(notes: "testing") }
as(@another_user) { @artist.update(wiki_page_attributes: { body: "testing" }) }
assert_equal(old_timestamp.to_i, @artist.reload.wiki_page.updated_at.to_i)
end
context "when renaming an artist" do
should "automatically rename the artist's wiki page" do
assert_difference("WikiPage.count", 0) do
put_auth artist_path(@artist.id), @user, params: {artist: {name: "bbb", notes: "more testing"}}
end
@wiki_page.reload
assert_equal("bbb", @wiki_page.title)
assert_equal("more testing", @wiki_page.body)
end
should "merge the new notes with the existing wiki page's contents if a wiki page for the new name already exists" do
as_user do
@existing_wiki_page = create(:wiki_page, title: "bbb", body: "xxx")
end
put_auth artist_path(@artist.id), @user, params: {artist: {name: "bbb", notes: "yyy"}}
@existing_wiki_page.reload
assert_equal("bbb", @existing_wiki_page.title)
assert_equal("xxx\n\nyyy", @existing_wiki_page.body)
end
end
end
should "delete an artist" do

18
test/unit/artist_test.rb
View File

@@ -104,24 +104,6 @@ class ArtistTest < ActiveSupport::TestCase
end
end
should "create a new wiki page to store any note information" do
artist = nil
assert_difference("WikiPage.count") do
artist = FactoryBot.create(:artist, :name => "aaa", :notes => "testing")
end
assert_equal("testing", artist.notes)
assert_equal("testing", artist.wiki_page.body)
assert_equal(artist.name, artist.wiki_page.title)
end
should "update the wiki page when notes are assigned" do
artist = FactoryBot.create(:artist, :name => "aaa", :notes => "testing")
artist.update_attribute(:notes, "kokoko")
artist.reload
assert_equal("kokoko", artist.notes)
assert_equal("kokoko", artist.wiki_page.body)
end
should "normalize its name" do
artist = FactoryBot.create(:artist, :name => " AAA BBB ")
assert_equal("aaa_bbb", artist.name)