Skip to content


Subversion checkout URL

You can clone with
Download ZIP
Browse files

Stop checking file.present? in Attachment#file_extension

This looks like a remnant of add630e.

AttachmentUploader#present? actually checks whether the underlying
file exists and contains data [1], which locally it never will if
you've imported a preview/production database without taking local
copies of the attachments from S3.

All we really need to know is whether there's an object to delegate
the #url call to, so we might as well call #url directly and let the
delegation mechanism deal with nils.

  • Loading branch information...
commit 0637cf16f1c987cf74060ee25f5962b046ad0ce8 1 parent b30359d
@tomstuart tomstuart authored
Showing with 8 additions and 2 deletions.
  1. +2 −2 app/models/attachment.rb
  2. +6 −0 test/unit/attachment_test.rb
4 app/models/attachment.rb
@@ -1,7 +1,7 @@
class Attachment < ActiveRecord::Base
mount_uploader :file, AttachmentUploader, mount_on: :carrierwave_file
- delegate :url, to: :file
+ delegate :url, to: :file, allow_nil: true
validates :title, :file, presence: true
@@ -14,7 +14,7 @@ def filename
def file_extension
- File.extname(url).gsub(/\./, "") if file.present? && url.present?
+ File.extname(url).gsub(/\./, "") if url.present?
def pdf?
6 test/unit/attachment_test.rb
@@ -137,6 +137,12 @@ class AttachmentTest < ActiveSupport::TestCase
assert_nil attachment.file_extension
+ test "should return file extension if URL present but file empty" do
+ attachment = build(:attachment)
+ attachment.stubs(file: stub("uploader", empty?: true, url: "greenpaper.pdf"))
+ assert_equal "pdf", attachment.file_extension
+ end
test "should return file extension if file present" do
greenpaper_pdf = fixture_file_upload('greenpaper.pdf', 'application/pdf')
attachment = build(:attachment, file: greenpaper_pdf)
Please sign in to comment.
Something went wrong with that request. Please try again.