From ca00563a4db6a7245af5a9fb6d5156664c81f1f6 Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 12 May 2020 16:29:24 -0500 Subject: [PATCH] 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. --- app/controllers/artists_controller.rb | 2 -- app/models/artist.rb | 2 -- app/policies/artist_policy.rb | 2 +- app/views/artists/_form.html.erb | 10 +++++-- test/functional/artists_controller_test.rb | 31 ---------------------- 5 files changed, 9 insertions(+), 38 deletions(-) diff --git a/app/controllers/artists_controller.rb b/app/controllers/artists_controller.rb index a85c7d992..986250c29 100644 --- a/app/controllers/artists_controller.rb +++ b/app/controllers/artists_controller.rb @@ -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 diff --git a/app/models/artist.rb b/app/models/artist.rb index 8e8608d0d..06b0c1af1 100644 --- a/app/models/artist.rb +++ b/app/models/artist.rb @@ -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) } diff --git a/app/policies/artist_policy.rb b/app/policies/artist_policy.rb index ded6d7a6c..1cd5dc2f6 100644 --- a/app/policies/artist_policy.rb +++ b/app/policies/artist_policy.rb @@ -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 diff --git a/app/views/artists/_form.html.erb b/app/views/artists/_form.html.erb index cb4990b63..d4a200708 100644 --- a/app/views/artists/_form.html.erb +++ b/app/views/artists/_form.html.erb @@ -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? %> +
+ +
+ +
+ <%= format_text(@artist.wiki_page.body, disable_mentions: true) %> +
<% end %> <%= f.button :submit, "Submit" %> diff --git a/test/functional/artists_controller_test.rb b/test/functional/artists_controller_test.rb index 97697622b..bba9db451 100644 --- a/test/functional/artists_controller_test.rb +++ b/test/functional/artists_controller_test.rb @@ -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)