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)

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