Partner profile -- rework additional documents to be more workable

[cielf]

Summary

Currently, the Additional documents only allows access to one document. We want to really allow multiple documents, and to also allow deleting documents

Why?

We need both the ability to have multiple documents and user deletion of old documents to support long term partner bank relationships.

Details

• This is on the partner profile edit - both as a bank and as a partner
• We should be able to actually upload more than one document
• We need to add the ability to selectively delete documents. The interface for this should similar to how we allow removal of items. Provide a remove button, which, when pushed, makes the file disappear from the interface. Once saved, it is gone.

HINT: From Discussion on PR 4505

"The Partner::Profile model does in theory support multiple attached documents:

has_many_attached :documents
But when I attach a new one, there's an ActiveStorage::PurgeJob running that removes the old one."

Criteria for completion

• Behaviour as described above
• Tests to support
• This will also need a change to the user docs

[danielabar]

Found this in the Rails Guide on Active Storage that might explain the existing observed behaviour that old docs are getting removed when more are attached later:

Replacing vs Adding Attachments

By default in Rails, attaching files to a has_many_attached association will replace any existing attachments. To keep existing attachments, you can use hidden form fields with the signed_id of each attached file

[danielabar]

@cielf Just fyi, this commit fixes the original observed bug that if a user uploads an attached doc during profile edit, saves, then comes back later to add more, the first one is removed. It also supports attaching multiple docs, then coming back and adding one or more later, and all are preserved.

From there I can continue on the additional requirements re: selectively deleting.

But just in case the bug of losing documents when uploading more is urgent, that initial commit fixes that.

[cielf]

I think it would be a fine idea to put that in as an initial PR that partially resolves this issue, as it is fixing a current problem, and the rest is adding functionality. Please and thanks!

[danielabar]

With regards to the related issue #4472 , where if a user selects a new document for upload, and enters something invalid in one of the other fields, and them submits the form for update, then they get a 500 Internal Server Error.

I have a solution on my branch that doesn't involve changing to direct uploads and new CORS config:

Firstly, when the form renders with validation errors, check that the existing documents are persisted before attempting to render them. This fixes the 500 Internal Server Error that would otherwise occur from trying to render a new unpersisted document.

Second, in the update action, it's possible to detect that user was trying to upload something. In this case, if a validation error occurs AND user was trying to upload an attached doc, render an additional alert telling them they'll need to select that doc again.

It will look like this:

Note that my solution fixes this for the Attached Documents section, not the IRS document (didn't want to scope creep into another ticket). If this solution is acceptable, then the developer working on the other ticket could use this solution as well.

[cielf]

In an ideal world, they wouldn't have to re-upload.
It's better to have a world where they have to re-upload than a world where they lose the upload silently. IMO

[github-actions]

This issue is marked as stale due to no activity within 30 days. If no further activity is detected within 7 days, it will be unassigned.

[danielabar]

Still a work-in-progress, although working on kind of related #4472 first.