diff --git a/app/controllers/post_replacements_controller.rb b/app/controllers/post_replacements_controller.rb index 325773a19..05ec2a175 100644 --- a/app/controllers/post_replacements_controller.rb +++ b/app/controllers/post_replacements_controller.rb @@ -11,9 +11,14 @@ class PostReplacementsController < ApplicationController def create @post_replacement = authorize PostReplacement.new(creator: CurrentUser.user, post_id: params[:post_id], **permitted_attributes(PostReplacement)) @post_replacement.save - @post_replacement.process! - respond_with(@post_replacement, location: @post_replacement.post, notice: "Post replaced") + if request.format.html? && @post_replacement.errors.any? + flash[:notice] = @post_replacement.errors.full_messages.join("; ") + redirect_to @post_replacement.post + else + flash[:notice] = "Post replaced" + respond_with(@post_replacement, location: @post_replacement.post) + end end def update diff --git a/app/logical/post_replacement_processor.rb b/app/logical/post_replacement_processor.rb index 44e4533e6..14b0590c6 100644 --- a/app/logical/post_replacement_processor.rb +++ b/app/logical/post_replacement_processor.rb @@ -12,8 +12,7 @@ class PostReplacementProcessor media_file = get_file_for_upload(replacement.replacement_url, nil, replacement.replacement_file&.tempfile) if Post.where.not(id: post.id).exists?(md5: media_file.md5) - replacement.errors.add(:base, "Duplicate: post with md5 #{media_file.md5} already exists") - return + raise "Duplicate of post ##{Post.find_by_md5(media_file.md5).id}" end if media_file.md5 == post.md5 @@ -44,11 +43,11 @@ class PostReplacementProcessor post.tag_string = replacement.tags rescale_notes(post) - - replacement.save! post.save! - post.update_iqdb + rescue Exception => exception + replacement.errors.add(:base, exception.message) + raise ActiveRecord::Rollback end def rescale_notes(post) diff --git a/app/models/post_replacement.rb b/app/models/post_replacement.rb index 0b1ffc6b9..099a9c4fd 100644 --- a/app/models/post_replacement.rb +++ b/app/models/post_replacement.rb @@ -3,9 +3,11 @@ class PostReplacement < ApplicationRecord belongs_to :post belongs_to :creator, class_name: "User" - before_validation :initialize_fields, on: :create - attr_accessor :replacement_file, :final_source, :tags + before_validation :initialize_fields, on: :create + before_create :process! + + attr_accessor :replacement_file, :final_source, :tags attribute :replacement_url, default: "" def initialize_fields diff --git a/app/views/post_replacements/_new.html.erb b/app/views/post_replacements/_new.html.erb index f6552a7a0..d60441fcb 100644 --- a/app/views/post_replacements/_new.html.erb +++ b/app/views/post_replacements/_new.html.erb @@ -1,7 +1,7 @@
<%= embed_wiki("help:replacement_notice") %> - <%= edit_form_for(post_replacement, url: post_replacements_path(post_id: post_replacement.post_id), method: :post, remote: true) do |f| %> + <%= edit_form_for(post_replacement, url: post_replacements_path(post_id: post_replacement.post_id), method: :post) do |f| %> <%= f.input :replacement_file, label: "File", as: :file %> <%= f.input :replacement_url, label: "Replacement URL", hint: "The source URL to download the replacement from.", as: :string, input_html: { value: post_replacement.post.normalized_source } %> <%= f.input :final_source, label: "Final Source", hint: "If present, the source field will be changed to this after replacement.", as: :string %> diff --git a/app/views/post_replacements/create.js.erb b/app/views/post_replacements/create.js.erb deleted file mode 100644 index 7662f71ec..000000000 --- a/app/views/post_replacements/create.js.erb +++ /dev/null @@ -1,2 +0,0 @@ -Danbooru.notice("Post replaced."); -location.reload(); diff --git a/test/factories/post_replacement.rb b/test/factories/post_replacement.rb index 20d2a8acb..0379493a2 100644 --- a/test/factories/post_replacement.rb +++ b/test/factories/post_replacement.rb @@ -2,7 +2,8 @@ FactoryBot.define do factory(:post_replacement) do post factory: :post, source: FFaker::Internet.http_url original_url { FFaker::Internet.http_url } - replacement_url { FFaker::Internet.http_url } + replacement_url { "" } + replacement_file { Rack::Test::UploadedFile.new("test/files/test.jpg") } creator end end diff --git a/test/functional/post_replacements_controller_test.rb b/test/functional/post_replacements_controller_test.rb index cebb18484..9380dfd6e 100644 --- a/test/functional/post_replacements_controller_test.rb +++ b/test/functional/post_replacements_controller_test.rb @@ -154,6 +154,23 @@ class PostReplacementsControllerTest < ActionDispatch::IntegrationTest end end + context "a replacement that fails" do + should "not create a post replacement record" do + @post = create(:post) + + assert_no_difference("PostReplacement.count") do + post_auth post_replacements_path, create(:moderator_user), params: { + post_id: @post.id, + post_replacement: { + replacement_file: Rack::Test::UploadedFile.new("test/files/ugoira.json"), + } + } + + assert_redirected_to @post + end + end + end + should "not allow non-mods to replace posts" do assert_difference("PostReplacement.count", 0) do @post = create(:post) @@ -187,8 +204,8 @@ class PostReplacementsControllerTest < ActionDispatch::IntegrationTest @admin = create(:admin_user) @mod = create(:moderator_user, name: "yukari") - @post_replacement = create(:post_replacement, creator: @mod, post: create(:post, tag_string: "touhou")) - @admin_replacement = create(:post_replacement, creator: create(:admin_user), replacement_url: "https://danbooru.donmai.us") + @post_replacement = create(:post_replacement, creator: @mod, post: create(:post, tag_string: "touhou"), replacement_file: Rack::Test::UploadedFile.new("test/files/test.png")) + @admin_replacement = create(:post_replacement, creator: @admin, replacement_file: Rack::Test::UploadedFile.new("test/files/test.jpg")) end should "render" do @@ -197,7 +214,6 @@ class PostReplacementsControllerTest < ActionDispatch::IntegrationTest end should respond_to_search({}).with { [@admin_replacement, @post_replacement] } - should respond_to_search(replacement_url_like: "*danbooru*").with { @admin_replacement } context "using includes" do should respond_to_search(post_tags_match: "touhou").with { @post_replacement }