Skip to content

Commit

Permalink
Correctly redirect assets containing periods in the filename
Browse files Browse the repository at this point in the history
E.g. the minister-of-funk.960x640.jpg test file.

This was causing the people.feature:57 to fail because it was being
redirected to minister-of-funk.960x640 (i.e. it was dropping the jpg
extension).

This seems to be because Rails was treating '960x640' as the extension
(previously defined in routes.rb) and 'jpg' as the format in the call to
`redirect_to`.

Renaming `extension` to `format` in routes appears to do the trick.

The change to the assets_test illustrated the problem before the change
in this commit.

TODO: Roll this into the previous commit. Without it the previous commit
fails the people.feature.
  • Loading branch information
chrisroos committed Dec 19, 2017
1 parent 8bb5481 commit 1c0847c
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 4 deletions.
2 changes: 1 addition & 1 deletion config/routes.rb
Expand Up @@ -422,5 +422,5 @@ def external_redirect(path_prefix, target)
get '/government/uploads/system/uploads/consultation_response_form/*path.:extension' => LongLifeRedirect.new('/government/uploads/system/uploads/consultation_response_form_data/')
get '/government/uploads/system/uploads/attachment_data/file/:id/*file.:extension' => "attachments#show"
get '/government/uploads/system/uploads/attachment_data/file/:id/*file.:extension/preview' => "attachments#preview", as: :preview_attachment
get '/government/uploads/*path.:extension' => "public_uploads#show", as: :public_upload
get '/government/uploads/*path.:format' => "public_uploads#show", as: :public_upload
end
4 changes: 2 additions & 2 deletions test/functional/public_uploads_controller_test.rb
Expand Up @@ -5,7 +5,7 @@ class PublicUploadsControllerTest < ActionController::TestCase
Plek.any_instance.stubs(:public_asset_host).returns('http://asset-host.com')
request.host = 'not-asset-host.com'

get :show, params: { path: 'asset', extension: 'txt' }
get :show, params: { path: 'asset', format: 'txt' }

assert_redirected_to 'http://asset-host.com/government/uploads/asset.txt'
end
Expand All @@ -17,7 +17,7 @@ class PublicUploadsControllerTest < ActionController::TestCase
Plek.any_instance.stubs(:public_asset_host).returns('http://asset-host.com')
request.host = 'asset-host.com'

get :show, params: { path: 'asset', extension: 'txt' }
get :show, params: { path: 'asset', format: 'txt' }

assert_response 200
end
Expand Down
2 changes: 1 addition & 1 deletion test/integration/assets_test.rb
Expand Up @@ -2,7 +2,7 @@

class AssetsTest < ActionDispatch::IntegrationTest
setup do
asset_filename = 'asset.txt'
asset_filename = 'asset.tmp.txt'
@asset_filesystem_path = File.join(Whitehall.clean_uploads_root, asset_filename)
FileUtils.touch(@asset_filesystem_path)

Expand Down

0 comments on commit 1c0847c

Please sign in to comment.