Skip to content
This repository was archived by the owner on Jul 13, 2023. It is now read-only.

Fixed case when file is not present on cloud #2173

Closed
wants to merge 2 commits into from
Closed

Fixed case when file is not present on cloud #2173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
58bad59
Fixed case when file is not present on cloud
vimutter Apr 15, 2016
1ff4a9e
Update fog_spec.rb
vimutter Apr 15, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/paperclip/storage/fog.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ def copy_to_local_file(style, local_dest_path)
log("copying #{path(style)} to local file #{local_dest_path}")
::File.open(local_dest_path, 'wb') do |local_file|
file = directory.files.get(path(style))
return false unless file
local_file.write(file.body)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd write this as:

if file = directory.files.get(path(style))
  local_file.write file.body
end

Do you think it reads better?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tute Usually we avoid assignment in conditionals as it always triggers internal question whether this line is typo or not. I.e. it requires check of surrounding of the line, which is sometimes ok.
I.e. I can change this line if you wish so.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tute Usually we avoid assignment in conditionals as it always triggers internal question whether this line is typo or not. I.e. it requires check of surrounding of the line, which is sometimes ok.
I.e. I can change this line if you wish so.

I don't feel very strongly either way, so following the Hound (who didn't bark at this line) is good with me. 👍

end
rescue ::Fog::Errors::Error => e
Expand Down
7 changes: 7 additions & 0 deletions spec/paperclip/storage/fog_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,13 @@ def custom_method
tempfile.close
end

it 'is able to be handled when missing while copying to a local file' do
tempfile = Tempfile.new("known_location")
tempfile.binmode
assert_equal false, @dummy.avatar.copy_to_local_file(:original, tempfile.path)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line is too long. [86/80]

tempfile.close
end

it "passes the content type to the Fog::Storage::AWS::Files instance" do
Fog::Storage::AWS::Files.any_instance.expects(:create).with do |hash|
hash[:content_type]
Expand Down