diff --git a/app/controllers/todos_controller.rb b/app/controllers/todos_controller.rb index 4bffd371..c8276a4a 100644 --- a/app/controllers/todos_controller.rb +++ b/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 diff --git a/app/models/attachment.rb b/app/models/attachment.rb index 2b38bf51..c33fdc22 100644 --- a/app/models/attachment.rb +++ b/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 diff --git a/app/models/todo.rb b/app/models/todo.rb index 8e4ee7c6..a39e5964 100644 --- a/app/models/todo.rb +++ b/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' } diff --git a/app/models/user.rb b/app/models/user.rb index 49bc4c2e..d693c0f1 100644 --- a/app/models/user.rb +++ b/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? diff --git a/test/models/todo_test.rb b/test/models/todo_test.rb index dcdf1a2d..50602a52 100644 --- a/test/models/todo_test.rb +++ b/test/models/todo_test.rb @@ -542,4 +542,28 @@ class TodoTest < ActiveSupport::TestCase assert_equal "
test
", 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