[WIP] Fix missing uploaded files on invalid edit entry form

[doekenorg]

This PR addresses #2161 and #2317.

The files are no longer processed during the validation step, but are added back to the form in a fake upload process.

To test:

• [ ] GF File Upload field with single and multiple files
• [ ] Validation using the number of files, extension, size, etc.
  • [ ] When testing Multi-File Upload, trigger validation errors when adding files to the existing field

💾 Build file (c9cd0b344).

[mrcasual]

@doekenorg, thanks. I came across this issue:

https://www.loom.com/share/57097b6a05754f4a9b7b340db597fcd8

[mrcasual]

@Mwalek, please test and prioritize for release on Thursday.

[rafaehlers]

@Mwalek, please also make sure to test what happens when duplicating an entry and deleting it later. What happens to the file of the original entry?

[doekenorg]

@Mwalek, please also make sure to test what happens when duplicating an entry and deleting it later. What happens to the file of the original entry?

Yes, this can be tested, but it is out of the scope of this issue. So if this results in a problem, please open a different issue for that 😄

[Mwalek]

@doekenorg I struggled a bit in order to replicate the 2 issues on the current release because non of the issues mention that the problem is specific to multiple file upload. Anyway, I was finally able to reproduce the issue. Then I installed the new build 953b2b9 and noticed a fatal error.

Steps to Reproduce

1. Create a GF form with a Single Line Text Field that is required
2. Add a File Upload field that allows multiple uploads
3. Submit the form in the front end, but only populate the single line text field
4. After you have at least one entry, create a View that displays your form
5. Open the View in the front end and click "Edit Entry"
6. On the Edit Entry screen, upload an image file and remove all the text in the Text field
7. Click update

[mrcasual]

@Mwalek, please use ccee897.

[Mwalek]

@mrcasual the fatal error is gone. But neither 2161 nor 2317 is fixed. Could I be missing something? 🤔 Here's a quick Loom summarising steps I took:

https://www.loom.com/share/50bb02532f84418390c123e765c66e79?sid=ac40ff1d-f61b-4a73-a575-27eb8857a851

[doekenorg]

@mrcasual the fatal error is gone. But neither 2161 nor 2317 is fixed. Could I be missing something? 🤔 Here's a quick Loom summarising steps I took:

https://www.loom.com/share/50bb02532f84418390c123e765c66e79?sid=ac40ff1d-f61b-4a73-a575-27eb8857a851

@Mwalek Can it be that you have a JS cache? I've updated the js files, but the version tag is not; so you might be using old JS. And can you try the latest build e11d525.

[Mwalek]

@doekenorg if it's caching, then I doubt it's on the browser. I tried again with b6d6a45 after clearing all browser data. Perhaps you could give it a try on pr-2326-gv.try.gravitykit.com (test;test).

Views:

• https://pr-2326-gv.try.gravitykit.com/view/march-display-1/
• https://pr-2326-gv.try.gravitykit.com/view/april-display-1/

[Mwalek]

@doekenorg thanks for pointing me in the right direction! Both issues are resolved. However, for #2317, I just want to point out that an error is shown even though the entry is successfully updated.

Also, please ping me whenever IH is ready for testing, thanks!

[doekenorg]

@Mwalek, this is ready for testing. Please check above, as I have added a list with what should at least be tested. But please be very thorough, as this has been a complex issue.

[mrcasual]

@Mwalek, please also add this to your list of acceptance tests to write and prioritize it alongside the other tests.

[Mwalek]

@doekenorg thanks for the changes! I only noticed a couple of issues during my extensive tests. Namely;

• [ ] File lost after validation errors during edit entry. To reproduce, edit an entry with GravityView that has 2 fields (1) a required single line text field called Name and (2) a single upload field. Empty the name field and add a valid file to the file upload field. Click update. When the page reloads with the expected submission error, the image upload still appears to be present. Adding content to the name field and clicking update however, results in the following error shown under the image upload field: "This field is required." Loom: https://www.loom.com/share/34aaff56624d4a1a8f5564b846a67518?sid=18bf1bd4-5beb-4cd8-a8a3-76035ce1670b
• [ ] Max files not entirely respected. To reproduce, edit an entry with GravityView that has 2 fields (1) a required single line text field called Name and (2) a multiple upload field with max number of files = 2. Delete the text in the text field and delete all files in the upload field. Add 2 files in the upload field and try to save your changes. Because of the missing text the upload fails and there will now be more than 2 files in the upload area. It's like the file deletion is undone upon failed validation. Would it be possible to remember that the "old file" was already deleted? Loom: https://www.loom.com/share/dc4d8420a8584252807d379186d223a8?sid=cdb9f822-3d17-4098-9fe6-5a842530c973

Image Hopper

• [ ] It is possible to update an entry without adding required image upload file by simply clicking update again after validation fails. You can make the validation fail by using a file that is bigger than max upload size, for example.

[doekenorg]

@Mwalek, please ignore the Image Hopper issue for now. This also occurs on a regular (new) form; therefore, for now, consider this a bug in IH. I have told Jake about it. Can you test this again with the latest version? 🙏

[Mwalek]

@doekenorg perfect, thank you! No issues were encountered testing the latest build.

@mrcasual I added the E2E tests (IH not included) and they are all passing locally with the latest changes. The tests currently fail in the expand-test-coverage branch because that branch does not have these changes.

[mrcasual]

@Mwalek, please merge your branch into this PR branch, run the tests and ensure that everything's passing in the CI.

[mrcasual]

@Mwalek, you may want to save screenshots/video when tests fails like this. You can then download them from the CI job's artifacts.

[Mwalek]

Thanks @mrcasual, that's very helpful! More so the idea of debugging on CI, which I did not think of until your suggestion. In this case, I already know exactly where (and why) the test is failing because I usually watch the tests "live" via the vscode extension. The problem is my solution(s) don't work on CI. So I'm better off testing them there first via SSH.

[Mwalek]

@mrcasual the tests are all passing now 😄