Skip to content

Commit

Permalink
Merge pull request #1202 from alphagov/serve-whitehall-asset-from-med…
Browse files Browse the repository at this point in the history
…ia-download-endpoint

Enable serving whitehall asset from media download endpoint
  • Loading branch information
lauraghiorghisor-tw authored Sep 20, 2023
2 parents 74e90fb + e138396 commit 0db8fc4
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 37 deletions.
3 changes: 1 addition & 2 deletions app/controllers/media_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,7 @@ def filename_current?

def asset_servable?
asset.filename_valid?(params[:filename]) &&
asset.uploaded? &&
asset.mainstream?
asset.uploaded?
end

def asset
Expand Down
4 changes: 0 additions & 4 deletions app/models/asset.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,6 @@ def public_url_path
"/media/#{id}/#{filename}"
end

def mainstream?
true
end

def file=(file)
old_filename = filename
super(file).tap do
Expand Down
4 changes: 0 additions & 4 deletions app/models/whitehall_asset.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,4 @@ def last_modified
def public_url_path
legacy_url_path
end

def mainstream?
false
end
end
20 changes: 10 additions & 10 deletions spec/controllers/media_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,16 @@ def download
get :download, **params
end

context "with a whitehall asset" do
let(:asset) { FactoryBot.create(:uploaded_whitehall_asset) }

it "proxies whitehall asset to S3 via Nginx" do
expect(controller).to receive(:proxy_to_s3_via_nginx).with(asset)

get :download, **params
end
end

it "sets Cache-Control header to expire in 30 minutes and be publicly cacheable" do
get :download, **params

Expand Down Expand Up @@ -520,16 +530,6 @@ def download
end
end

context "with an otherwise servable whitehall asset" do
let(:path) { "/government/uploads/asset.png" }
let(:asset) { FactoryBot.create(:uploaded_whitehall_asset, legacy_url_path: path) }

it "responds with 404 Not Found" do
get :download, **params
expect(response).to have_http_status(:not_found)
end
end

context "with an infected file" do
let(:asset) { FactoryBot.create(:infected_asset) }

Expand Down
10 changes: 1 addition & 9 deletions spec/models/asset_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@
described_class.new(file: load_fixture_file("asset.png"))
end

it "returns public URL path for mainstream asset" do
it "returns public URL path for asset" do
expected_path = download_media_path(id: asset.id, filename: asset.filename)
expect(asset.public_url_path).to eq(expected_path)
end
Expand Down Expand Up @@ -1035,14 +1035,6 @@
end
end

describe "#mainstream?" do
let(:asset) { described_class.new }

it "returns truth-y" do
expect(asset).to be_mainstream
end
end

describe "#upload_success!" do
context "when asset is unscanned" do
let(:asset) { FactoryBot.create(:asset) }
Expand Down
8 changes: 0 additions & 8 deletions spec/models/whitehall_asset_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,14 +137,6 @@
end
end

describe "#mainstream?" do
let(:asset) { described_class.new }

it "returns false-y" do
expect(asset).not_to be_mainstream
end
end

describe ".from_params" do
let(:format_from_params) { "png" }
let(:path_from_params) { "government/uploads/path/to/asset" }
Expand Down

0 comments on commit 0db8fc4

Please sign in to comment.