Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Marking Whitehall attachments as deleted should delete corresponding Asset Manager assets when appropriate #509

Closed
floehopper opened this issue Mar 6, 2018 · 7 comments
Assignees

Comments

@floehopper
Copy link
Contributor

I've been considering this in Deleting a parent Whitehall document should delete underlying assets in Asset Manager, but have just realised that is outside the scope of that issue, so I'm creating this one. There are a bunch of comments on that issue which are relevant to this one.

Note that Whitehall::AssetManagerStorage does implement functionality for destruction of the uploader's model. Thus when an AttachmentData is destroyed, a job is queued to delete the corresponding asset in Asset Manager. However, in the majority of cases when an attachment is deleted in Whitehall, the AttachmentData is not destroyed. Thus there is more work to do and hence the need for this card.

@floehopper
Copy link
Contributor Author

Our latest thinking is to change Whitehall so that instances of AttachmentData are never destroyed. I've opened this PR to implement this idea: alphagov/whitehall#3835.

@floehopper
Copy link
Contributor Author

I've merged alphagov/whitehall#3853 to address this.

@floehopper
Copy link
Contributor Author

I'm currently testing this in integration.

@floehopper
Copy link
Contributor Author

The following output from the Asset Manager Rails console in integration demonstrates that the main & thumbnail assets associated with a Whitehall PDF attachment are marked as deleted when the attachment is deleted in Whitehall.

Before attachment deleted

Main asset

irb(main):001:0> wa = WhitehallAsset.find_by(legacy_url_path: '/government/uploads/system/uploads/attachment_data/file/674437/2018-01-14_netLec7.pdf')
=> #<WhitehallAsset _id: 5aa9022f5381ea02efa33fce, deleted_at(deleted_at): nil, created_at: "2018-03-14T11:06:23.360Z", updated_at: "2018-03-14T11:06:24.446Z", replacement_id: nil, state: "uploaded", filename_history: [], uuid: "99f78a81-fe3b-488b-b254-1a97ae72aa50", draft: true, redirect_url: nil, etag: "5aa9022f-5c414", last_modified: "2018-03-14T11:06:23.587Z", md5_hexdigest: "3a1be7a717309631fcd2cb03cd8c0048", size: 377876, access_limited: [], parent_document_url: nil, file: "2018-01-14_netLec7.pdf", _type: "WhitehallAsset", legacy_url_path: "/government/uploads/system/uploads/attachment_data/file/674437/2018-01-14_netLec7.pdf", legacy_etag: nil, legacy_last_modified: nil>

irb(main):002:0> wa.state
=> "uploaded"

irb(main):003:0> wa.draft?
=> true

irb(main):004:0> wa.deleted?
=> false

Thumbnail asset

irb(main):005:0> twa = WhitehallAsset.find_by(legacy_url_path: '/government/uploads/system/uploads/attachment_data/file/674437/thumbnail_2018-01-14_netLec7.pdf.png')
=> #<WhitehallAsset _id: 5aa9022f5d7f096c7cb00ab3, deleted_at(deleted_at): nil, created_at: "2018-03-14T11:06:23.474Z", updated_at: "2018-03-14T11:06:24.076Z", replacement_id: nil, state: "uploaded", filename_history: [], uuid: "1bca823b-e4d0-48f1-aac3-e48706ce1189", draft: true, redirect_url: nil, etag: "5aa9022f-1bf5", last_modified: "2018-03-14T11:06:23.574Z", md5_hexdigest: "24bf42516f134fc070078dc2f9c12893", size: 7157, access_limited: [], parent_document_url: nil, file: "thumbnail_2018-01-14_netLec7.pdf.png", _type: "WhitehallAsset", legacy_url_path: "/government/uploads/system/uploads/attachment_data/file/674437/thumbnail_2018-01-14_netLec7.pdf.png", legacy_etag: nil, legacy_last_modified: nil>

irb(main):006:0> twa.state
=> "uploaded"

irb(main):007:0> twa.draft?
=> true

irb(main):008:0> twa.deleted?
=> false

After attachment deleted

Main asset

irb(main):009:0> wa.reload
=> #<WhitehallAsset _id: 5aa9022f5381ea02efa33fce, deleted_at(deleted_at): "2018-03-14T11:11:56.206Z", created_at: "2018-03-14T11:06:23.360Z", updated_at: "2018-03-14T11:06:24.446Z", replacement_id: nil, state: "uploaded", filename_history: [], uuid: "99f78a81-fe3b-488b-b254-1a97ae72aa50", draft: true, redirect_url: nil, etag: "5aa9022f-5c414", last_modified: "2018-03-14T11:06:23.587Z", md5_hexdigest: "3a1be7a717309631fcd2cb03cd8c0048", size: 377876, access_limited: [], parent_document_url: nil, file: "2018-01-14_netLec7.pdf", _type: "WhitehallAsset", legacy_url_path: "/government/uploads/system/uploads/attachment_data/file/674437/2018-01-14_netLec7.pdf", legacy_etag: nil, legacy_last_modified: nil>

irb(main):010:0> wa.deleted?
=> true

### Thumbnail asset

irb(main):011:0> twa.reload
=> #<WhitehallAsset _id: 5aa9022f5d7f096c7cb00ab3, deleted_at(deleted_at): "2018-03-14T11:11:56.243Z", created_at: "2018-03-14T11:06:23.474Z", updated_at: "2018-03-14T11:06:24.076Z", replacement_id: nil, state: "uploaded", filename_history: [], uuid: "1bca823b-e4d0-48f1-aac3-e48706ce1189", draft: true, redirect_url: nil, etag: "5aa9022f-1bf5", last_modified: "2018-03-14T11:06:23.574Z", md5_hexdigest: "24bf42516f134fc070078dc2f9c12893", size: 7157, access_limited: [], parent_document_url: nil, file: "thumbnail_2018-01-14_netLec7.pdf.png", _type: "WhitehallAsset", legacy_url_path: "/government/uploads/system/uploads/attachment_data/file/674437/thumbnail_2018-01-14_netLec7.pdf.png", legacy_etag: nil, legacy_last_modified: nil>

irb(main):012:0> twa.deleted?
=> true

@floehopper
Copy link
Contributor Author

The following output from the Asset Manager Rails console in integration demonstrates that the main & thumbnail assets associated with a Whitehall PDF attachment are marked as deleted when the attachment's parent edition is discarded in Whitehall.

Before draft edition discarded

Main asset

irb(main):001:0> wa = WhitehallAsset.find_by(legacy_url_path: '/government/uploads/system/uploads/attachment_data/file/674438/2018-01-14_netLec7.pdf')
=> #<WhitehallAsset _id: 5aa904f55d7f090b429767c8, deleted_at(deleted_at): nil, created_at: "2018-03-14T11:18:13.774Z", updated_at: "2018-03-14T11:18:14.131Z", replacement_id: nil, state: "uploaded", filename_history: [], uuid: "d07d54ce-2a3f-4a16-9fc7-da4cc0517210", draft: true, redirect_url: nil, etag: "5aa904f5-5c414", last_modified: "2018-03-14T11:18:13.865Z", md5_hexdigest: "3a1be7a717309631fcd2cb03cd8c0048", size: 377876, access_limited: [], parent_document_url: nil, file: "2018-01-14_netLec7.pdf", _type: "WhitehallAsset", legacy_url_path: "/government/uploads/system/uploads/attachment_data/file/674438/2018-01-14_netLec7.pdf", legacy_etag: nil, legacy_last_modified: nil>

irb(main):002:0> wa.state
=> "uploaded"

irb(main):003:0> wa.draft?
=> true

irb(main):004:0> wa.deleted?
=> false

Thumbnail asset

irb(main):005:0> twa = WhitehallAsset.find_by(legacy_url_path: '/government/uploads/system/uploads/attachment_data/file/674438/thumbnail_2018-01-14_netLec7.pdf.png')
=> #<WhitehallAsset _id: 5aa904f55381ea0380552753, deleted_at(deleted_at): nil, created_at: "2018-03-14T11:18:13.675Z", updated_at: "2018-03-14T11:18:14.257Z", replacement_id: nil, state: "uploaded", filename_history: [], uuid: "c2365e47-da80-416b-9af7-2455741f6b8c", draft: true, redirect_url: nil, etag: "5aa904f5-1bf5", last_modified: "2018-03-14T11:18:13.970Z", md5_hexdigest: "24bf42516f134fc070078dc2f9c12893", size: 7157, access_limited: [], parent_document_url: nil, file: "thumbnail_2018-01-14_netLec7.pdf.png", _type: "WhitehallAsset", legacy_url_path: "/government/uploads/system/uploads/attachment_data/file/674438/thumbnail_2018-01-14_netLec7.pdf.png", legacy_etag: nil, legacy_last_modified: nil>

irb(main):006:0> twa.state
=> "uploaded"

irb(main):007:0> twa.draft?
=> true

irb(main):008:0> twa.deleted?
=> false

After draft edition discarded

Main asset

irb(main):012:0* wa.reload
=> #<WhitehallAsset _id: 5aa904f55d7f090b429767c8, deleted_at(deleted_at): "2018-03-14T11:21:10.630Z", created_at: "2018-03-14T11:18:13.774Z", updated_at: "2018-03-14T11:18:14.131Z", replacement_id: nil, state: "uploaded", filename_history: [], uuid: "d07d54ce-2a3f-4a16-9fc7-da4cc0517210", draft: true, redirect_url: nil, etag: "5aa904f5-5c414", last_modified: "2018-03-14T11:18:13.865Z", md5_hexdigest: "3a1be7a717309631fcd2cb03cd8c0048", size: 377876, access_limited: [], parent_document_url: nil, file: "2018-01-14_netLec7.pdf", _type: "WhitehallAsset", legacy_url_path: "/government/uploads/system/uploads/attachment_data/file/674438/2018-01-14_netLec7.pdf", legacy_etag: nil, legacy_last_modified: nil>

irb(main):013:0> wa.deleted?
=> true

Thumbnail asset

irb(main):014:0> twa.reload
=> #<WhitehallAsset _id: 5aa904f55381ea0380552753, deleted_at(deleted_at): "2018-03-14T11:21:10.674Z", created_at: "2018-03-14T11:18:13.675Z", updated_at: "2018-03-14T11:18:14.257Z", replacement_id: nil, state: "uploaded", filename_history: [], uuid: "c2365e47-da80-416b-9af7-2455741f6b8c", draft: true, redirect_url: nil, etag: "5aa904f5-1bf5", last_modified: "2018-03-14T11:18:13.970Z", md5_hexdigest: "24bf42516f134fc070078dc2f9c12893", size: 7157, access_limited: [], parent_document_url: nil, file: "thumbnail_2018-01-14_netLec7.pdf.png", _type: "WhitehallAsset", legacy_url_path: "/government/uploads/system/uploads/attachment_data/file/674438/thumbnail_2018-01-14_netLec7.pdf.png", legacy_etag: nil, legacy_last_modified: nil>

irb(main):015:0> twa.deleted?
=> true

@floehopper
Copy link
Contributor Author

The above testing means I'm happy for this to be deployed to staging/production.

@chrisroos
Copy link
Contributor

@floehopper and I have just confirmed that this is working as expected in staging. We created a draft consultation, added an attachment and observed that we could access the attachment using both draft-origin (Whitehall) and draft-assets (Asset Manager). We then deleted the attachment from the consultation and observed that we could no longer view it using the draft-assets host.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants