Fix #4999: Unexpected error: ActiveRecord::RecordNotUnique sometimes appears when uploading posts
Fix two issues that could lead to duplicate errors when creating posts: * Fix the submit button on the upload form to disable itself on submit, to prevent accidental double submit errors. * Fix a race condition when checking for MD5 duplicates. MD5 uniqueness is checked on both the Rails level, with a uniqueness validation, and on the database level, with a unique index on the md5 column. Creating a post could fail with an ActiveRecord::RecordNotUnique error if the uniqueness validation in Rails passed, but the uniqueness constraint in the database failed. In this case, we catch the RecordNotUnique error and convert it to a Rails validation error so we can treat it like a normal validation failure.
This commit is contained in:
@@ -66,7 +66,7 @@ class PostsController < ApplicationController
|
||||
def create
|
||||
@upload_media_asset = UploadMediaAsset.find(params[:upload_media_asset_id])
|
||||
@post = authorize Post.new_from_upload(@upload_media_asset.upload, @upload_media_asset.media_asset, **permitted_attributes(Post).to_h.symbolize_keys)
|
||||
@post.save
|
||||
@post.save_if_unique(:md5)
|
||||
|
||||
if @post.errors.none?
|
||||
if @post.warnings.any?
|
||||
|
||||
@@ -172,6 +172,16 @@ class ApplicationRecord < ActiveRecord::Base
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
# Save the record, but convert RecordNotUnique exceptions thrown by the database into
|
||||
# Rails validation errors. This way duplicate records only return one type of error.
|
||||
# This assumes the table only has one uniqueness constraint in the database.
|
||||
def save_if_unique(column)
|
||||
save
|
||||
rescue ActiveRecord::RecordNotUnique => e
|
||||
self.errors.add(column, :taken)
|
||||
false
|
||||
end
|
||||
end
|
||||
|
||||
concerning :UserMethods do
|
||||
|
||||
@@ -92,7 +92,7 @@
|
||||
<%= render "related_tags/buttons" %>
|
||||
</div>
|
||||
|
||||
<%= f.submit "Upload", id: "submit-button", data: { disable_with: false } %>
|
||||
<%= f.submit "Post" %>
|
||||
|
||||
<% if CurrentUser.can_upload_free? %>
|
||||
<%= f.input :is_pending, as: :boolean, label: "Upload for approval", wrapper_html: { class: "inline-block" }, input_html: { checked: @post.is_pending? } %>
|
||||
|
||||
Reference in New Issue
Block a user