Skip to content

Commit

Permalink
Add attachment_data_id to file attachments
Browse files Browse the repository at this point in the history
Currently, the properties in the file attachment properties are
insufficient to make a request to asset manager from the server to fetch
the asset.

The API client wants to be called like this:

    GdsApi.asset_manager.media(attachment_data_id, filename)

... but we don't have an attachment_data_id, we only have url /
preview_url (which include the id we want), and a bunch of other fields
which don't. We shouldn't have to parse URLs to find IDs.

We get away with this because in most cases where we're using file
assets, we redirect the user to the full asset URL (so we don't need to
fetch it on the server side).

The one example I'm aware of where we do need to request assets on the
server side is CSV previews, and we've ended up with a pretty wild
workaround in that case. Specifically, we route some requests to the
assets domains (e.g. assets.publishing.service.gov.uk) to frontend.
This way, the preview_url ends up being served by frontend itself. This
means the URL gets parsed by Rails' routing logic, and we end up with
the ID in a param which we can use to call asset manager: see
https://github.com/alphagov/frontend/blob/8b075a3e26d775ad97b6b473ddaa22386e07cad2/app/controllers/csv_preview_controller.rb#L27

I think if we had access to the asset manager id (attachment_data_id)
earlier on, we could probably avoid the need to put preview_url in the
content item at all, and just let frontend load the asset and preview
it.

My primary motivation here isn't CSV previews though, it's landing pages
for the programme for government project. We want to be able to load CSV
data and render it into a line chart. We have basically the exact same
issue there - we can't request the asset using the API client without
parsing the asset's URL.

We'll need a corresponding commit in Whitehall (and maybe other
publishing apps, if they have attachments?) to start sending this new
field through to the API. It's optional, so this shouldn't break
anything in the meantime.
  • Loading branch information
richardTowers committed Nov 21, 2024
1 parent 056cd20 commit de98c73
Showing 1 changed file with 1 addition and 0 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
local FileAttachmentAssetProperties = {
accessible: { type: "boolean", },
alternative_format_contact_email: { type: "string", },
attachment_data_id: { type: "string", },
attachment_type: { type: "string", enum: ["file"], },
content_type: { type: "string", },
file_size: { type: "integer", },
Expand Down

0 comments on commit de98c73

Please sign in to comment.