From 43c4158d36e323a09d9178a4be2d3bccd1da4a3e Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 30 Jan 2022 02:31:22 -0600 Subject: [PATCH] uploads: merge tags when a duplicate is uploaded (fix #3130). Automatically merge tags when uploading a duplicate. There are two cases: * You try to upload an image, but it's already on Danbooru. In this case you'll be immediately redirected to the original post, before you can start tagging the upload. * You're uploading an image, it wasn't a dupe when you first opened the upload page, but you got sniped while tagging it. In this case your tags will be merged with the original post, and you will be redirected to the original post. There are a few corner cases: * If you don't have permission to edit the original post, for example because it's banned or has a censored tag, then your tags won't be merged and will be silently ignored. * Only the tags, rating, and parent ID will be merged. The source and artist commentary won't be merged. This is so that if an artist uploads the exact same file to multiple sites, the new source won't override the original source. * Some tags might be contradictory. For example, the new post might be tagged translation_request, but the original post might already be translated. It's up to the user to fix these things afterwards. --- app/controllers/posts_controller.rb | 11 ++++++++--- app/controllers/uploads_controller.rb | 8 +++++++- app/models/media_asset.rb | 1 + test/functional/posts_controller_test.rb | 19 +++++++++++++++++-- test/functional/uploads_controller_test.rb | 8 ++++++++ test/unit/upload_service_test.rb | 21 --------------------- 6 files changed, 41 insertions(+), 27 deletions(-) diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 109f840ad..8d4b4564c 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -67,12 +67,17 @@ class PostsController < ApplicationController @post = authorize Post.new_from_upload(permitted_attributes(Post)) @post.save - if @post.errors.any? + if @post.errors.none? + respond_with(@post) + elsif @post.errors.of_kind?(:md5, :taken) + @original_post = Post.find_by!(md5: @post.md5) + @original_post.update(rating: @post.rating, parent_id: @post.parent_id, tag_string: "#{@original_post.tag_string} #{@post.tag_string}") + flash[:notice] = "Duplicate of post ##{@original_post.id}; merging tags" + redirect_to @original_post + else @upload = UploadMediaAsset.find(params[:post][:upload_media_asset_id]).upload flash[:notice] = @post.errors.full_messages.join("; ") respond_with(@post, render: { template: "uploads/show" }) - else - respond_with(@post) end end diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 67beb3c70..6f3a47ec0 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -38,6 +38,12 @@ class UploadsController < ApplicationController def show @upload = authorize Upload.find(params[:id]) @post = Post.new(uploader: @upload.uploader, uploader_ip_addr: @upload.uploader_ip_addr, source: @upload.source, rating: nil, **permitted_attributes(Post)) - respond_with(@upload) + + if request.format.html? && @upload.is_completed? && @upload.media_assets.first&.post.present? + flash[:notice] = "Duplicate of post ##{@upload.media_assets.first.post.id}" + redirect_to @upload.media_assets.first.post + else + respond_with(@upload) + end end end diff --git a/app/models/media_asset.rb b/app/models/media_asset.rb index 1987d182a..a2703832f 100644 --- a/app/models/media_asset.rb +++ b/app/models/media_asset.rb @@ -12,6 +12,7 @@ class MediaAsset < ApplicationRecord LARGE_IMAGE_WIDTH = Danbooru.config.large_image_width STORAGE_SERVICE = Danbooru.config.storage_manager + has_one :post, foreign_key: :md5, primary_key: :md5 has_one :media_metadata, dependent: :destroy has_one :pixiv_ugoira_frame_data, class_name: "PixivUgoiraFrameData", foreign_key: :md5, primary_key: :md5 diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb index 43a32a51a..b53c85f57 100644 --- a/test/functional/posts_controller_test.rb +++ b/test/functional/posts_controller_test.rb @@ -5,9 +5,9 @@ class PostsControllerTest < ActionDispatch::IntegrationTest assert_equal(expected, response.parsed_body.css("link[rel=canonical]").attribute("href").value) end - def create_post!(user: create(:user), rating: "q", tag_string: "tagme", **params) + def create_post!(user: create(:user), media_asset: build(:media_asset), rating: "q", tag_string: "tagme", **params) upload = build(:upload, uploader: user) - asset = create(:upload_media_asset, upload: upload) + asset = create(:upload_media_asset, upload: upload, media_asset: media_asset) post_auth posts_path, user, params: { post: { upload_media_asset_id: asset.id, rating: rating, tag_string: tag_string, **params }} Post.last @@ -655,6 +655,21 @@ class PostsControllerTest < ActionDispatch::IntegrationTest assert_equal("test", @post.tag_string) end + should "re-render the upload page if the upload fails" do + @post = create_post!(rating: "z", tag_string: "tagme") + assert_response :success + end + + should "merge the tags and redirect to the original post if the upload is a duplicate of an existing post" do + media_asset = create(:media_asset) + post1 = create_post!(rating: "s", tag_string: "post1", media_asset: media_asset) + post2 = create_post!(rating: "e", tag_string: "post2", media_asset: media_asset) + + assert_redirected_to post1 + assert_equal("post1 post2", post1.reload.tag_string) + assert_equal("e", post1.rating) + end + should "apply the rating from the tags" do @post = create_post!(rating: nil, tag_string: "rating:s") diff --git a/test/functional/uploads_controller_test.rb b/test/functional/uploads_controller_test.rb index 4961a754a..a1d0fdd7c 100644 --- a/test/functional/uploads_controller_test.rb +++ b/test/functional/uploads_controller_test.rb @@ -130,6 +130,14 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest assert_response :success end + + should "redirect a completed upload to the original post if it's a duplicate of an existing post" do + @upload = create(:completed_file_upload, uploader: @user) + @post = create(:post, md5: @upload.media_assets.first.md5, media_asset: @upload.media_assets.first) + get_auth upload_path(@upload), @user + + assert_redirected_to @post + end end context "create action" do diff --git a/test/unit/upload_service_test.rb b/test/unit/upload_service_test.rb index 7d685a220..ab7ba85a6 100644 --- a/test/unit/upload_service_test.rb +++ b/test/unit/upload_service_test.rb @@ -311,27 +311,6 @@ class UploadServiceTest < ActiveSupport::TestCase CurrentUser.ip_addr = nil end - context "with a preprocessed predecessor" do - setup do - @predecessor = FactoryBot.create(:source_upload, status: "preprocessed", source: @source, image_height: 0, image_width: 0, file_size: 1, md5: 'd34e4cf0a437a5d65f8e82b7bcd02606', file_ext: "jpg") - @tags = 'hello world' - end - - context "when the file has already been uploaded" do - setup do - @asset = MediaAsset.find_by_md5("d34e4cf0a437a5d65f8e82b7bcd02606") - @post = create(:post, md5: "d34e4cf0a437a5d65f8e82b7bcd02606", media_asset: @asset) - @service = subject.new(source: @source) - end - - should "point to the dup post in the upload" do - @upload = subject.new(source: @source, tag_string: @tags).start! - @predecessor.reload - assert_equal("error: ActiveRecord::RecordInvalid - Validation failed: Md5 duplicate: #{@post.id}", @predecessor.status) - end - end - end - context "with a source containing unicode characters" do should "normalize unicode characters in the source field" do source1 = "poke\u0301mon" # pokeĢmon (nfd form)