search

flexion / ef-cms

An Electronic Filing / Case Management System.
23 stars 10 forks source link

Detailed Error Messaging: Encrypted/Password Protected PDF #10001

Closed cholly75 closed 1 month ago

cholly75 commented 1 year ago

As petitioner or practitioner, so that I know how to correct problems when uploading files , I need more detailed responses when I upload an encrypted or password protected PDF.

Currently when a user uploads a file in DAWSON, if the file does not meet certain criteria we display an error in the UI and ask them to try again. Even though there are a few commonly seen specific causes for the error, we do not give the user any indication of what the problem might be to help them successfully upload the file on subsequent retries.

We would like to display custom error messaging in these cases to help our external users understand what problems they need to address in their file before attempting to re-upload it. We have been able to connect several root causes for upload problems with specific logfile messages in Kibana, and we should display helpful information to the user based on the error seen in those messages.

Pre-Conditions

Acceptance Criteria

Notes

image.png

image.png

Tasks

Test Cases

Story Definition of Ready (updated on 12/23/22)

The following criteria must be met in order for the user story to be picked up by the Flexion development team. The user story must:

Definition of Done (Updated 5-19-22)

Product Owner

UX

Engineering

ttlenard commented 1 year ago

Test Cases

1) Petitioner uploads an encrypted or password protected PDF for their STIN; Petitioner receives error message immediately after upload; Petitioner is unable to proceed to the next step.

Expected Results:

*Repeat this test as a Private Practitioner

2) Kibana logs display the error message that the user experienced when they uploaded the encrypted/Password Protected STIN.

Expected Results:

3) Petitioner uploads an encrypted or password protected PDF for their Petition; Petitioner receives error message immediately after upload; Petitioner is unable to proceed to the next step.

Expected Results:

*Repeat this test as a Private Practitioner

4) Kibana logs display the error message that the user experienced when they uploaded the encrypted/Password Protected Petition.

Expected Results:

5) Petitioner uploads an encrypted or password protected PDF for their CDS; Petitioner receives error message immediately after upload; Petitioner is unable to proceed to the next step.

Expected Results:

Repeat this test by selecting all of the business types available Repeat this test as a Private Practitioner

6) Kibana logs display the error message that the user experienced when they uploaded the encrypted/Password Protected CDS.

Expected Results:

7) Petitioner files a document on their case; uploads an encrypted or password protected PDF; Petitioner receives error message immediately after upload; Petitioner is unable to proceed to the next step.

Expected Results:

*Repeat this test with a Private Practitioner and an IRS Practitioner.

8) Kibana logs display the error message that the user experienced when they uploaded the encrypted/Password Protected document.

Expected Results:

9) Petitioner files a supporting document with their filing on their case; uploads an encrypted or password protected PDF for the supporting document; Petitioner receives error message immediately after upload; Petitioner is unable to proceed to the next step.

Expected Results:

Repeat this test by adding multiple supporting documents, ensuring that for each supporting document, you receive the error message (can add up to 5 supporting documents for each filing) Repeat this test with a Private Practitioner and an IRS Practitioner.

10) Kibana logs display the error message that the user experienced when they uploaded the encrypted/Password Protected document.

Expected Results:

11) Repeat test cases 1-10 using a mobile device; error messages received on Mobile device screens match mockups.

jhansen-flexion commented 1 year ago

Note: 02 and 03 blocked because this code is the same as those stories. We want to use this story to inform how the other two stories will move forward.

rachelschneiderman commented 1 year ago

https://trello.com/c/J4S7ZhKx/1187-convert-validation-messages-for-entities-to-using-joi-syntax https://trello.com/c/KZCJa3fh/1188-convert-all-entities-to-es6-style-classes https://trello.com/c/9XuhSxO8/1189-parent-entities-that-expect-to-display-validation-errors-for-child-entities-need-to-know-child-entitiy-schema-at-or-before-run-t https://trello.com/c/NnW4cAkj/1190-modify-joivalidationentitygetformattederrormessages-to-not-be-recursive-use-standard-joischemavalidate

swongCO commented 2 months ago

Pre refinement

Mwindo commented 2 months ago

I've looked over this and #10002. After some helpful discussion with Zach, here are my thoughts:

Ideally, we want whatever solution we have here to be 1) helpful for the user (this is probably most important), 2) consistent with the app.

RE: 1) What would be most helpful for the user? Immediate feedback upon file selection rather than having to wait on submit (especially if there are multiple files in the form), similar to the alert generated by limitFileSize.ts. This suggests we should use some frontend validation.

RE: 2) How do we currently show errors in the app?

Which is the best approach in our case? Probably showing an inline error or an alert. I think showing ErrorNotification accords more with form submission than with file selection, and, as mentioned above, I think validation of basic file constraints should occur on file selection. (So long as we can do these checks quickly!)

Therefore, my recommendation is to use pdfjs-dist to do some basic pdf validation on the frontend. We can have something like ClientPdfValidator, which checks for file size, encryption, and non-pdf .pdfs. We can use this in StateDrivenFileInput so that it is available for every file upload. On errors, I suggest we follow an approach similar to what we do for file size, but show a custom modal alert rather than a JS alert (which doesn't allow links, which we might want for explanations of "Why is my pdf encrypted?"). (I suggest we also use this custom alert for the file size error, too.) If we still want logs for these errors in OpenSearch, we can fire some event to the backend to do so.

Note that this approach would not use entity validation. That said, I think we have at least some conceptual justification here: we validate fields one way, validate file selections in another. In other words, file selection validation would come before modifying the form, similar to an invalid keystroke. This doesn't feel terribly messy.

Mwindo commented 2 months ago

I've worked on this a little more. I think inline, the original suggestion, works too. I'll work on implementing that.

Mwindo commented 2 months ago

I've talked with the team about two options forward for #10001 and #10002. (Tenille also mentioned it might be worth investigating #10003, which I will look into as well.) Option 1 was inline errors with yucky code, option 2 was modal errors with better code. We opted for option 2, the modal errors.

A few important points/open questions that were brought up:

  1. Modal errors provide us more space to use links (e.g., "Why is my pdf encrypted?")
  2. We need to make sure that the modal errors are mobile-friendly
  3. We need to make sure that the modal errors are accessible and screen-reader friendly
  4. Do we display all error messages at once (e.g., given a file that is both too big and encrypted), or one at a time?
  5. How do we handle errors that still happen at the end of an (maybe multi-step) upload process?
  6. Make sure that pdf validation only happens for pdf since files are not always pdfs. (This is already taken care of.)

A few cursory thoughts: RE: 4: I think one at a time is easiest and makes sense. I'm not sure how much more helpful aggregate messages would be, and I think this would be a rare case. RE: 5: I think we can essentially keep the same behavior in place (showing a catch-all modal), maybe modifying the language slightly. The earlier, on-file-select validation should catch most errors before that. Then we can incrementally improve as needed.

mwestereng1 commented 2 months ago

UX Notes:

All notes apply to 10001, 10002, and 10003

Figma File showing different messaging examples.

Mwindo commented 2 months ago

A quick update: after a brief discussion, we have decided to show the file size too big error first (when applicable) to avoid potentially parsing a giant file when validating.

Mwindo commented 2 months ago

Another update: 1) Because the error messages should never overlap (e.g., we cannot read a corrupted PDF to tell if it is encrypted, and vice-versa), we have decided to forego the multiple-error-message modal until needed. 2) After experimenting with big files and throttling my browser, we have decided to show the loading modal overlay while the file is being validated.

Mwindo commented 2 months ago

I've pushed to flexion experimental1 and am doing/have done some tests before polishing the code and preparing to push to test. One of the main things I am testing is that large files (around the maximum of 250 MB) do not take too long to validate on different devices. (Since file uploading is such an essential part of the app, I want to test thoroughly!) All of my results so far have been positive:

Mwindo commented 2 months ago

Ultimately, the new approach in 10001-10003 has been to validate files on file selection, which I think is a great start.

A good follow-up ticket: provide better messaging on failed submissions. This means a couple of different things:

  1. Rather than the generic "maybe your firewall is blocking the submission" error, we can check network connectivity and whether the request got through (because, if not, that suggests firewall issues) and update messages accordingly.
  2. Some places do not even display a generic file upload error when something goes wrong but instead show a 400 error. For instance, here is what happens if a user uploads a corrupt pdf. (This particular failure will now be caught earlier by file selection validation, but you can imagine something else going wrong instead.)

Private Zenhub Video

ttlenard commented 2 months ago

@Mwindo you can use this URL for the troubleshooting link: https://ustaxcourt.gov/dawson_faqs_case_management.html#FileUpload

ttlenard commented 2 months ago

@Mwindo Some testing feedback:

If you upload a file that was signed using Adobe sign, it will put the document in a Password Protected/secured state. Documents in this state are still being able to be uploaded without error, until the final submission step, where the user is now getting the firewall error since it didn't catch it at the initial upload. See the secured test document that I sent you via slack. This document results in the "Invalid PDF" error in the logs. Thanks!

Mwindo commented 2 months ago

Thanks @ttlenard! My implementation only checked password protection on read, not on edit (which we need since DAWSON modifies PDFs by stamping, adding cover sheets, etc.). I have a fix I will get reviewed that checks for password protection on edit as well.

ttlenard commented 2 months ago

@Mwindo Some testing feedback:

@cholly75 All you will need to test when the adjustment above is made is to test as an admissions clerk and upload various file types (including excel, csv, etc.) into the Practitioner database. You shouldn't receive any pop-up indicating that you can't upload the documents, even if they are in a file type not specifically specified in the instructional text.

Mwindo commented 1 month ago

Thanks @ttlenard! (I misunderstood an earlier comment about not validating files--I was thinking, "Don't do any file-specific validation" instead of "don't do any generic file-type validation at all"). I've updated the code in test.

Mwindo commented 1 month ago

Thanks @ttlenard! (I misunderstood an earlier comment about not validating files--I was thinking, "Don't do any file-specific validation" instead of "don't do any generic file-type validation at all"). I've updated the code in test.