Save response form documents as draft assets until publication v2 [WHIT-3750] - #11764
Open
eYinka wants to merge 8 commits into
Open
Save response form documents as draft assets until publication v2 [WHIT-3750]#11764eYinka wants to merge 8 commits into
eYinka wants to merge 8 commits into
Conversation
Response documents need to know which edition (and therefore which access controls) they belong to, so Asset Manager can gate them the same way file attachments already are. This adds the accessor only - to be used by future commits.
eYinka
commented
Sep 4, 2026
| .map(&:name) | ||
|
|
||
| asset_data_types.each(&block) | ||
| %w[AttachmentData ImageData].each(&block) |
Contributor
Author
There was a problem hiding this comment.
AssetData is auto-discovered by this shared test helper
(for_each_asset_data_type) and it previously scanned by module inclusion. Since
Consultation/CallForEvidence response form models now include AssetData too, that scan was changed to an explicit %w[AttachmentData ImageData] list to avoid sweeping response form data into shared tests built for a generic edition. Hopefully this is okay?
eYinka
force-pushed
the
save-response-form-docs-as-draft-2
branch
from
September 4, 2026 11:50
776e054 to
0df0e71
Compare
ChrisBAshton
reviewed
Sep 7, 2026
ChrisBAshton
left a comment
ChrisBAshton
left a comment
Contributor
There was a problem hiding this comment.
Looks great! 🎉 Thanks for iterating this PR from the other one, I do really like the fact that we're converging onto one way of doing assets.
Some thoughts below, will approve when ready (since it would need a re-approval post-rebase anyway)
This is prep work for reusing AssetData in ConsultationResponseFormData/ CallForEvidenceResponseFormData. AssetData expects every item in its attachments collection to respond to `#attachable` and `#deleted?`, and expects the including model to define a `#replaced?` behaviour itself - which only comes from a separate Replaceable concern. This adds those methods to ConsultationResponseForm/CallForEvidenceResponseForm and ConsultationResponseFormData/CallForEvidenceResponseFormData, all returning sensible fixed values since response documents have no soft-delete or replacement concept. No behaviour change yet and nothing calls these until further commits.
Consultation and call for evidence response documents were always uploaded as live assets in Asset Manager, even while the owning edition was still a draft or access-limited. This commit switches them to use the same draft/access-limited storage flow that File attachments use, so they're gated by the parent edition's access controls until it is published. Note: `AssetData` is auto-discovered by a shared test helper (`for_each_asset_data_type`) in `test/attachment_test_helper.rb` and it previously scanned by module inclusion. Since Consultation/CallForEvidence response form models now include `AssetData` too, that scan was changed to an explicit `%w[AttachmentData ImageData]` list to avoid sweeping response form data into shared tests built for generic editions.
Response form data models have no `replaced_by_id` column / Replaceable concern (unlike AttachmentData/ImageData), so AssetManagerAttachmentMetadataJob's `replaced_by` lookup would raise an error for them whenever their parent documents are updated. I've now added a guard to the lookup so that it skips the replacement operations for such models.
This flips response document assets from draft to live once the owning Consultation/CallForEvidence is published. It mirrors how AttachmentAssetPublisher already does this for file attachments and images. There's a trade-off though: this couples the otherwise edition-agnostic publisher to Consultation/CallForEvidence's specific association chains. Probably okay, pending when we migrate these document types to standard edition. The same trade-off already exists in `EditionAuthBypassAssetPropagator` which branches on the same two edition types for the same reason, so this follows the existing approach.
…dits Previously, changing a consultation/call for evidence's access-limiting after its response document was uploaded never propagated to the asset in Asset Manager since `AssetManagerAttachmentMetadataJob` only re-ran at upload time. This is because `ServiceListeners::AttachmentUpdater` (which re-syncs on every edition save) had no branch for response form data, only Attachment/Image. I've now added that branch and also included some explicit tests for that behaviour.
…eleted DraftAttachmentAssetDiscarder was built to only handle Attachment/Image, so a draft Consultation/CallForEvidence that is deleted before ever being published would leave its response document as an orphaned draft asset in Asset Manager. AssetData's `needs_discarding?` (attachments.size == 1) gives us an ideal answer for response form data. i.e The attachments method on a Response Form Data always returns exactly one item (the response form, or Attachment::Null as a fallback). Therefore, it's always safe to discard; unlike in AttachmentData/ImageData, where size > 1 would mean the same data is still referenced by another (possibly) live attachment.
Storage::PreviewableStorage was only ever used by ImageUploader and ResponseDocumentUploader. Both have since moved to Storage::AttachmentStorage (images in #11667, response documents in this PR), so nothing references PreviewableStorage anymore.
eYinka
force-pushed
the
save-response-form-docs-as-draft-2
branch
from
September 7, 2026 12:33
0df0e71 to
8276923
Compare
ChrisBAshton
approved these changes
Sep 8, 2026
ChrisBAshton
left a comment
ChrisBAshton
left a comment
Contributor
There was a problem hiding this comment.
Looks good ⭐ (Assuming it's been put through its paces on integration)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Alternative solution to #11755
What
Consultation and Call for Evidence response documents are now stored as draft assets in Asset Manager while the owning edition is still a draft or access-limited, and are flipped to live only when the edition is published; which matches how file attachments already behave.
Why
By design, response documents were stored as live assets in Asset Manager from the moment they were uploaded, regardless of whether the owning edition was still a draft or access-limited. File attachments, by contrast, have always been stored as draft and flipped to live on publish. This meant response documents were reachable by anyone with the asset URL before the document was ever published.
This change brings response documents in line with file attachments i.e draft/access-limited editions' response documents are no longer publicly retrievable before publication, and the same preview link that grants access to a draft edition also inherently grants access to its response document.
Before/After
draft: false- live immediatelydraft: true- draft until the edition is publishedStorage::PreviewableStorageStorage::AttachmentStorageaccess_limited_organisation_ids/access_limited_user_idsare set from the parent editionauth_bypass_id)AttachmentAssetPublisherflips response document assets from draft to live, mirroring the existing attachment/image flowparent_document_url, draft state)PreviewableStoragenever passed the owning edition through, soAssetManagerAttachmentMetadataJobnever ran for response documentsJIRA
This application is owned by the Whitehall Experience team. Please let us know in #govuk-whitehall-experience-tech when you raise any PRs.
Follow these steps if you are doing a Rails upgrade.