[#4472] Persist file uploads through validation errors

[danielabar]

Resolves #4472

Important

Before this PR is merged to production, it will require setting CORS headers on Azure to support Active Storage Direct Uploads feature.

Description

The original issue reported that if a user filling out the partner profile form selected a file (eg: IRS Determination Letter) and filled out the form in a way that resulted in a validation error (eg: area served numbers not adding up to 100%), then the user would get a 500 server error page after attempting to save their changes.

This is because the view was attempting to render a link to the attached file via the rails_blob_path helper, assuming it was persisted, but it wasn't. It turns out, this is an aspect of Active Storage - the file is only associated with the model upon a successful save. So any time a file input is in a form that could also have validation errors, this issue can occur.

Specifically on Partner Profile editing, there are two single file attachment fields that are affected. Selecting one or both of these, together with any form validation issue will result in the general error page:

1. IRS Determination letter proof_of_partner_status
2. Form 990 Filed proof_of_form_990

This occurs on both the step-wise partner profile editing form, and the legacy all-in-one form. Both have been resolved here, but system testing is only for the step-wise form as the legacy form is planned for removal.

The immediate fix would be to check if the file is persisted? rather than merely attached? before rendering the downloadable file link. That by itself would resolve the 500 server error page. But that still leaves a usability issue - in that the user would have to notice that the file had not been attached, and select it from their file system again.

See discussion starting with this comment for options that were considered.

This can be resolved to provide a nicer user experience, requiring the introduction of a few new pieces:

1. Enabling Active Storage Direct Uploads:

When direct upload is enabled, JavaScript will upload the file directly to storage (on this project, it's Azure for production) from the client's browser, when the form is submitted. If the form submission fails on a validation error, the file has still been saved in storage, even if it's not associated to the model.

What this opens up is the ability to reference something like profile.proof_of_partner_status.blob.filename to display the filename to the user in a view, even if it's not yet associated. Also the profile.proof_of_partner_status.signed_id is available in-memory, to submit as a hidden field, which allows the file saving to be attempted again on the next form submission.

2. Custom File Input:

The default browser behaviour of the file input type is that it only displays the file a user selected, just after user has physically selected a file from their file system. It doesn't expose a label or something that can be customized to display a file name even if user has not just selected a file.

To resolve this, the simple_form gem, together with bootstrap styles (already available on this project) provides a # custom file input (see that definition in config/initializers/simple_form_bootstrap.rb). Using this class on the file input, together with a custom label supports being able to fill in the file input with a file name, even if user has not just selected a file. This means if form is being rendered with validation errors, and user had previously selected a file, it can still be displayed, and submitted again when user clicks Save again.

This behaviour is available in a partial: app/views/shared/_custom_file_input.html.erb

3. File Input Label Stimulus Controller

The custom file input mentioned above does not have the browser default behaviour of automatically displaying a filename if user has just selected it. So a small amount of JavaScript via a Stimulus controller app/javascript/controllers/file_input_label_controller.js is needed to provide this behaviour.

Type of change

• Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

For automated testing, see the new test added to spec/system/partners/profile_edit_system_spec.rb: "persists file upload when there are validation errors".

For manual testing:

1. Launch a Rails console bin/rails c and enable the step-wise editing feature flag: Flipper.enable(:partner_step_form).
2. Login as the invited seed user [email protected], OR login as an org admin and invite a new user.
3. Visit the edit profile view: http://localhost:3000/partners/profile/edit
4. Open up Agency Information section and upload a file for IRS Determination Letter.
5. Optionally also open up Agency Stability seciton and upload a file for Form 990.
6. Open up Media Information section and uncheck "No social media presence" and wipe out all urls in the social media fields. This is to intentionally generate a validation error.
7. Click Save Progress
8. You should be returned to the edit view with a validation error message at the top.
9. Now re-open the sections where you had selected a file (Agency Information and optionally Agency Stability) - your file name should still be displayed in the file input.
10. Fix the validation error in Media Information section and click Save Progress.
11. Your IRS file (and optionally Form 990 file) should have been saved. If you open these sections, a downloadable link should be rendered.

Screenshots

Here's what the new file input looks like with no file selected:

Here's what the new file input looks like with a file selected - it will look like this whether the user has literally just selected a file, or if the form renders with a validation error and user had previously selected this file:

Here's what the new file input looks like when the file has been successfully associated to the partner profile model:

[cielf]

This is only meant to address the single file issues, correct? It looks pretty good for that, functionally.

@dorner - I presume the note about production above will also apply to staging, yes?

[danielabar]

This is only meant to address the single file issues, correct? It looks pretty good for that, functionally.

@dorner - I presume the note about production above will also apply to staging, yes?

Yes this addresses single file (IRS letter, and Form 990) on Partner Profile edit form (step-wise and legacy versions).

There's a failing system test I need to look at.

[danielabar]

Fixed the failing system test - it was for the 403 static error page, was loading app JS, which as of this PR, also loads active storage for direct uploads, and was causing JS error in console. Turns out the static error pages don't need any of the app js, so removed all of it from all these pages.

Tangent: The logout link on static error pages results in an error (no show action in users controller). But this happens even without my changes so could be a separate issue.

[cielf]

"Tangent: The logout link on static error pages results in an error (no show action in users controller). But this happens even without my changes so could be a separate issue".
I'll throw that in the inbox, though it's happening often enough (10 times in the last week, 3 users) that I think it might be more widespread than that.

[dorner]

This looks amazing! @cielf did you want to kick the tires any further, or is this good to merge?

[cielf]

One very small thing I just noticed. Instead of repeating "Form 990 Filed" for the prompt on the Form 990 file, can we just say "Form 990"? Thanks.

[danielabar]

Form 990 Filed

Updated label text to Form 990

[cielf]

Ok -- with that change, this has the thumbs up from me. However, before merging I just want to double-check that the conditions @danielabar mentioned are met. (call out to @dorner )