Skip to content

Commit

Permalink
Removed the ability for nil to delete an attachment, sadly.
Browse files Browse the repository at this point in the history
  • Loading branch information
Jon Yurek committed Mar 12, 2009
1 parent f44ede7 commit 2db00be
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 26 deletions.
22 changes: 21 additions & 1 deletion lib/paperclip/attachment.rb
Expand Up @@ -62,6 +62,9 @@ def initialize name, instance, options = {}
# If the file that is assigned is not valid, the processing (i.e.
# thumbnailing, etc) will NOT be run.
def assign uploaded_file
# This is because of changes in Rails 2.3 that cause blank fields to send nil
return nil if uploaded_file.nil?

%w(file_name).each do |field|
unless @instance.class.column_names.include?("#{name}_#{field}")
raise PaperclipError.new("#{@instance.class} model does not have required column '#{name}_#{field}'")
Expand Down Expand Up @@ -158,6 +161,23 @@ def save
end
end

# Clears out the attachment. Has the same effect as previously assigning
# nil to the attachment. Does NOT save. If you wish to clear AND save,
# use #destroy.
def clear
queue_existing_for_delete
@errors = {}
@validation_errors = nil
end

# Destroys the attachment. Has the same effect as previously assigning
# nil to the attachment *and saving*. This is permanent. If you wish to
# wipe out the existing attachment but not save, use #clear.
def destroy
clear
save
end

# Returns the name of the file as originally assigned, and lives in the
# <attachment>_file_name attribute of the model.
def original_filename
Expand Down Expand Up @@ -273,7 +293,7 @@ def logging? #:nodoc:
end

def valid_assignment? file #:nodoc:
file.nil? || (file.respond_to?(:original_filename) && file.respond_to?(:content_type))
file.respond_to?(:original_filename) && file.respond_to?(:content_type)
end

def validate #:nodoc:
Expand Down
25 changes: 22 additions & 3 deletions test/attachment_test.rb
Expand Up @@ -592,20 +592,39 @@ def do_after_all; end
file.close
end

context "and deleted" do
context "and trying to delete" do
setup do
@existing_names = @attachment.styles.keys.collect do |style|
@attachment.path(style)
end
end

should "not delete the files saving in a deprecated manner" do
@attachment.expects(:instance_write).with(:file_name, nil).never
@attachment.expects(:instance_write).with(:content_type, nil).never
@attachment.expects(:instance_write).with(:file_size, nil).never
@attachment.expects(:instance_write).with(:updated_at, nil).never
@attachment.assign nil
@attachment.save
@existing_names.each{|f| assert File.exists?(f) }
end

should "delete the files when you call #clear and #save" do
@attachment.expects(:instance_write).with(:file_name, nil)
@attachment.expects(:instance_write).with(:content_type, nil)
@attachment.expects(:instance_write).with(:file_size, nil)
@attachment.expects(:instance_write).with(:updated_at, nil)
@attachment.assign nil
@attachment.clear
@attachment.save
@existing_names.each{|f| assert ! File.exists?(f) }
end

should "delete the files" do
should "delete the files when you call #delete" do
@attachment.expects(:instance_write).with(:file_name, nil)
@attachment.expects(:instance_write).with(:content_type, nil)
@attachment.expects(:instance_write).with(:file_size, nil)
@attachment.expects(:instance_write).with(:updated_at, nil)
@attachment.destroy
@existing_names.each{|f| assert ! File.exists?(f) }
end
end
Expand Down
30 changes: 8 additions & 22 deletions test/integration_test.rb
Expand Up @@ -96,7 +96,7 @@ class IntegrationTest < Test::Unit::TestCase

context "and deleted" do
setup do
@dummy.avatar = nil
@dummy.avatar.clear
@dummy.save
end

Expand Down Expand Up @@ -235,7 +235,7 @@ class IntegrationTest < Test::Unit::TestCase
assert File.exists?(p)
end

@dummy.avatar = nil
@dummy.avatar.clear
assert_nil @dummy.avatar_file_name
assert @dummy.valid?
assert @dummy.save
Expand All @@ -258,15 +258,15 @@ class IntegrationTest < Test::Unit::TestCase

saved_paths = [:thumb, :medium, :large, :original].collect{|s| @dummy.avatar.path(s) }

@d2.avatar = nil
@d2.avatar.clear
assert @d2.save

saved_paths.each do |p|
assert ! File.exists?(p)
end
end

should "know the difference between good files, bad files, not files, and nil" do
should "know the difference between good files, bad files, and not files" do
expected = @dummy.avatar.to_file
@dummy.avatar = "not a file"
assert @dummy.valid?
Expand All @@ -275,25 +275,21 @@ class IntegrationTest < Test::Unit::TestCase

@dummy.avatar = @bad_file
assert ! @dummy.valid?
@dummy.avatar = nil
assert @dummy.valid?, @dummy.errors.inspect
end

should "know the difference between good files, bad files, not files, and nil when validating" do
should "know the difference between good files, bad files, and not files when validating" do
Dummy.validates_attachment_presence :avatar
@d2 = Dummy.find(@dummy.id)
@d2.avatar = @file
assert @d2.valid?, @d2.errors.full_messages.inspect
@d2.avatar = @bad_file
assert ! @d2.valid?
@d2.avatar = nil
assert ! @d2.valid?
end

should "be able to reload without saving and not have the file disappear" do
@dummy.avatar = @file
assert @dummy.save
@dummy.avatar = nil
@dummy.avatar.clear
assert_nil @dummy.avatar_file_name
@dummy.reload
assert_equal "5k.png", @dummy.avatar_file_name
Expand All @@ -316,16 +312,6 @@ class IntegrationTest < Test::Unit::TestCase
assert_equal `identify -format "%wx%h" "#{@dummy.avatar.path(:original)}"`,
`identify -format "%wx%h" "#{@dummy2.avatar.path(:original)}"`
end

should "work when assigned a nil file" do
@dummy2.avatar = nil
@dummy2.save

@dummy.avatar = @dummy2.avatar
@dummy.save

assert !@dummy.avatar?
end
end

end
Expand Down Expand Up @@ -423,7 +409,7 @@ def s3_headers_for attachment, style
assert key.exists?
end

@dummy.avatar = nil
@dummy.avatar.clear
assert_nil @dummy.avatar_file_name
assert @dummy.valid?
assert @dummy.save
Expand All @@ -446,7 +432,7 @@ def s3_headers_for attachment, style

saved_keys = [:thumb, :medium, :large, :original].collect{|s| @dummy.avatar.to_file(s) }

@d2.avatar = nil
@d2.avatar.clear
assert @d2.save

saved_keys.each do |key|
Expand Down

0 comments on commit 2db00be

Please sign in to comment.