alphagov/whitehall — pull request #772 by gma (2013-08-19 17:32:56+00:00)
Title: Bulk upload replaces existing files

Story: https://www.pivotaltracker.com/story/show/54105109

Things to pay attention to:

- When an attachment's file is replaced, is there any way it could get inadvertently released on a published edition? (I don't think it's likely, but I'd like somebody else to think through how that should work in light of what happened a few weeks ago).

## Background

When a zip file contains files whose filenames match existing attachments on an edition, we shouldn't add new `Attachment`s (which is what we were doing prior to this commit), but should keep the existing `Attachment` and assign it a new `AttachmentData` instance to store the new file.

`BulkUpload` is used in two scenarios:

1. When we've just unzipped a zip file and need to render a form, asking the user to specify metadata for those attachments, and

2. When the user has just filled in a form with all these attachments which generates a nested attributes compliant hash of attachment/attachment_data parameters.

In (1) we take a list of filenames and hand it to the `BulkUpload.from_files` method. It'll find any existing attachments on the edition that have the same filename, take care of marking old attachments (with the same filename) as being replaced by this new attachment, and build new `Attachment` instances for
any files that haven't previously been attached to this edition.

In (2) the rules are different; we'll have a hash of attributes for multiple attachments, in the format that Rails uses when you use nested attributes in Rails. We assign this hash to the (publicly accessible property) `attachments_attributes`. While we might like to pass the hash into a method named of our own choosing (e.g. `bulk_upload.set_attachments(attrs)`) we're stuck with saying `bulk_upload.attachments_attributes = attrs`. Why? Well, if we were to delete the `attachments_attributes=` method Rails will generate a form that will break Rack's parser when we submit it.

It's "The Rails Way" (TM), or the highway.