Fix #4303: Unable to rename artist entries with wiki pages

Remove the ability to edit an artist's wiki page directly from the
artist edit page. Instead the artist edit page has a link to open the
wiki edit page if you need to edit the wiki too.

Fixes an error being thrown when renaming an artist with a wiki page.
The problem is that changing the artist's name breaks the artist's
association with the old wiki page. Rails really wants nested
associations to be based on immutable IDs, not on mutable names, so
dealing with this correctly is difficult.

We don't really want to encourage people to create wiki pages for
artists to begin with, since they're usually just used to duplicate
the artist urls. Making it less convenient to edit artist wiki pages is
an intentional change to discourage creating unnecessary artist wikis.

Finally, this fixes an exploit where it was possible to edit locked wiki
pages through the artist edit page.
This commit is contained in:
evazion
2020-05-12 16:29:24 -05:00
parent 31c7abd2e9
commit ca00563a4d
5 changed files with 9 additions and 38 deletions

View File

@@ -3,13 +3,11 @@ class ArtistsController < ApplicationController
def new
@artist = authorize Artist.new_with_defaults(permitted_attributes(Artist))
@artist.build_wiki_page if @artist.wiki_page.nil?
respond_with(@artist)
end
def edit
@artist = authorize Artist.find(params[:id])
@artist.build_wiki_page if @artist.wiki_page.nil?
respond_with(@artist)
end

View File

@@ -19,8 +19,6 @@ class Artist < ApplicationRecord
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) }
accepts_nested_attributes_for :wiki_page, update_only: true, reject_if: :all_blank
scope :banned, -> { where(is_banned: true) }
scope :unbanned, -> { where(is_banned: false) }

View File

@@ -12,7 +12,7 @@ class ArtistPolicy < ApplicationPolicy
end
def permitted_attributes
[:name, :other_names, :other_names_string, :group_name, :url_string, :is_deleted, { wiki_page_attributes: [:id, :body] }]
[:name, :other_names, :other_names_string, :group_name, :url_string, :is_deleted]
end
def permitted_attributes_for_new

View File

@@ -5,8 +5,14 @@
<%= f.input :group_name %>
<%= 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" } %>
<% if @artist.wiki_page.present? %>
<div class="input">
<label>Wiki (<%= link_to "Edit", edit_wiki_page_path(@artist.wiki_page) %>)</label>
</div>
<div class="prose">
<%= format_text(@artist.wiki_page.body, disable_mentions: true) %>
</div>
<% end %>
<%= f.button :submit, "Submit" %>

View File

@@ -161,37 +161,6 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest
end
end
context "with an artist that has a wiki page" do
setup do
as(@admin) do
@artist = create(:artist, name: "aaa", url_string: "http://example.com")
@wiki_page = create(:wiki_page, title: "aaa", body: "testing")
end
@another_user = create(:user)
end
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: { 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.wiki_page.body)
assert_not_equal(old_timestamp, @wiki_page.updated_at)
assert_redirected_to(artist_path(@artist.id))
end
should "not touch the updated_at fields when nothing is changed" do
old_timestamp = @wiki_page.updated_at
travel(1.minute)
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
end
context "destroy action" do
should "delete an artist" do
delete_auth artist_path(@artist.id), create(:builder_user)