Codejunky Codejunky - 5 months ago 22
Ruby Question

Rails transaction with dragonfly

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)

Answer

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