Reputation: 632
I am currently working on a Rails project. I have a nested form where I am allowing the user to upload multiple pictures. I am using dragonfly for the uploading I have also set up some constraints on the size and formats of the images a user can upload and they are working fine. My problem is that when I try to upload invalid images even though they won't be uploaded the rest of the form still gets saved. To mitigate this problem I initially wrote the create method as follows:
def create
@announcements = Announcement.new(announcement_params)
images = []
respond_to do |format|
params[:photos]['image'].each do |pic|
img = Photo.new(image: pic)
if img.valid?
images << img
else
@photos = @announcement.photos.build
format.html { render: 'new' }
end
if @announcement.save
params[:photos]['image'].each do |pic|
@photos = @announcement.photos.create(image: pic)
end
format.html { redirect_to @announcement }
else
format.html { render: 'new' }
end
end
end
This works fine, but as I am sure a lot of you would agree there is a lot of repetition and code is ugly. I worte a code just as a fast workaround until I can find a better solution. I tried to use transactions as in the following:
def create
@announcement = Announcement.new(announcement_params)
respond_to do |format|
ActiveRecord::Base.transaction do
begin
@announcement.save
params[:photos]["image"].each do |pic|
@photos = @announcement.photos.create!(image: pic)
end
format.html { redirect_to @announcement }
rescue
format.html { render action: 'new' }
end
end
end
end
But it's not working can anyone tell me what I am doing wrong, or suggest a better way to do this. Thanks
Edit: here is an excerpt of my logs:
Started POST "/announcements" for 127.0.0.1 at 2015-09-20 15:07:31 +0100
Processing by AnnouncementsController#create as HTML Parameters: {"utf8"=>"✓",
"authenticity_token"=>"fjU1kjnxqSdDwTqieOpTTCH56//p65AynqNyQQX6yiu84zwO0bJeQ3ZKr8tEBvGSZJphclxKkoys2bFp771hWg==", "announcement"=>{"title"=>"Testing wrong image size", "content"=>"Testing wrong image format with transaction code "}, "photos"=>{"image"=>[#<ActionDispatch::Http::UploadedFile:0x007fe9a83f9b18 @tempfile=#<Tempfile:/tmp/RackMultipart20150920-10097-muom0i.jpg>, @original_filename="AhiiZFK2.jpg", @content_type="image/jpeg", @headers="Content-Disposition: form-data; name=\"photos[image][]\"; filename=\"AhiiZFK2.jpg\"\r\nContent-Type: image/jpeg\r\n">]}, "commit"=>"Créer l'announce"}
User Load (0.6ms) SELECT "users".* FROM "users" WHERE "users"."id" = $1 ORDER BY "users"."id" ASC LIMIT 1 [["id", 1]]
(0.3ms) BEGIN
SQL (0.9ms) INSERT INTO "announcements" ("title", "content", "created_at", "updated_at") VALUES ($1, $2, $3, $4) RETURNING "id" [["title", "Testing wrong image size"], ["content", "Testing wrong image format with transaction code "], ["created_at", "2015-09-20 14:07:31.247186"], ["updated_at", "2015-09-20 14:07:31.247186"]]
DRAGONFLY: shell command: 'identify' '-ping' '-format' '%m %w %h' '/tmp/RackMultipart20150920-10097-muom0i.jpg'
(41.4ms) COMMIT
Photo Load (0.8ms) SELECT "photos".* FROM "photos" WHERE "photos"."announcement_id" = $1 [["announcement_id", 1]]
Rendered announcements/new.html.erb within layouts/application (24.1ms)
Rendered layouts/_header.html.erb (1.9ms)
Rendered layouts/_sidebar.html.erb (0.5ms)
Completed 200 OK in 3287ms (Views: 3154.3ms | ActiveRecord: 44.1ms)
Upvotes: 0
Views: 159
Reputation: 3900
If you don't want data in your database when a record is invalid, you need to abort the transaction. This is done when an exception is raised.
But the way you nested your code, an exception from create!
is rescued, and the transaction block can finish normally.
You need to nest it like this:
begin
ActiveRecord::Base.transaction do
@announcement.save!
params[:photos]["image"].each do |pic|
@announcement.photos.create!(image: pic)
end
end # transaction
format.html { redirect_to @announcement }
rescue
logger.warn "transaction aborted"
format.html { render action: 'new' }
end
To further improve, you put the database stuff in an own method, so that database calls and render calls are not mixed.
To explicitly abort (or in database terms "rollback") the transaction, you can raise a ActiveRecord::Rollback
exception. That aborts the transaction and is not re-thrown outside the transaction block. http://api.rubyonrails.org/classes/ActiveRecord/Rollback.html
Upvotes: 1