make attachments accesible from user model and fix cleaning up attachment on destroy

app/controllers/todos_controller.rb

@@ -817,14 +817,12 @@ class TodosController < ApplicationController
   def attachment
     id = params[:id]
     filename = params[:filename]
-    attachment = Attachment.where(id: id).first
+    attachment = current_user.attachments.find(id)
 
     if attachment 
-      if attachment.todo.user == current_user
-        send_file(attachment.file.path)
-      else
-        head :forbidden
-      end
+      send_file(attachment.file.path,
+        disposition: 'attachment',
+        type: 'message/rfc822')
     else
       head :not_found
     end

app/models/attachment.rb

@@ -7,4 +7,14 @@ class Attachment < ActiveRecord::Base
 
   do_not_validate_attachment_file_type :file
   # validates_attachment_content_type :file, :content_type => ["text/plain"]
+
+  before_destroy :delete_attached_file
+
+  private
+
+  def delete_attached_file
+    file = nil
+    save!
+  end
+
 end

app/models/todo.rb

@@ -25,7 +25,7 @@ class Todo < ActiveRecord::Base
   has_many :pending_successors, -> {where('todos.state = ?', 'pending')}, :through => :predecessor_dependencies,
     :source => :successor
 
-  has_many :attachments, dependent: :delete_all
+  has_many :attachments, dependent: :destroy
 
   # scopes for states of this todo
   scope :active, -> { where state: 'active' }

app/models/user.rb

@@ -96,6 +96,7 @@ class User < ActiveRecord::Base
 
   has_many :notes, -> { order "created_at DESC" }, dependent: :delete_all
   has_one :preference, dependent: :destroy
+  has_many :attachments, through: :todos
 
   validates_presence_of :login
   validates_presence_of :password, if: :password_required?

test/models/todo_test.rb

@@ -542,4 +542,28 @@ class TodoTest < ActiveSupport::TestCase
     assert_equal "<p><strong>test</strong></p>", todo.rendered_notes
   end
 
+  def test_attachments_are_removed_after_delete
+    # Given a user and a todo withou any attachments
+    todo = @not_completed1
+    assert_equal 0, todo.attachments.count, "we start without attachments"
+    assert_equal 0, todo.user.attachments.count, "the user has no attachments"
+
+    # When I add a file as attachment to a todo of this user
+    attachment = todo.attachments.build
+    attachment.file = File.open(File.join(Rails.root, 'test', 'fixtures', 'email_with_multipart.txt'))
+    attachment.save!
+    new_path = attachment.file.path
+
+    # then the attachment should be there
+    assert File.exists?(new_path), "attachment should be on file system"
+    assert_equal 1, todo.attachments.reload.count, "should have one attachment"
+
+    # When I destroy the todo
+    todo.destroy!
+
+    # Then the attachement and file should nogt be there anymore
+    assert_equal 0, todo.user.attachments.reload.count
+    assert !File.exists?(new_path), "attachment should not be on file system"
+  end
+
 end