Uploaders - Refactor and fixes

[pxpm]

This PR aims to fix and at the same time refactor uploaders and it's related validation classes.

Since launched some issues were identified for example the lack of validation support inside repeatables, or the tight coupling between an "ajax uploader" and a "dropzone uploader", that would prevent us from adding other types of ajax uploads like EasyMDE. Issues in dropzone validation .. etc.

There is a companion PR to this one in: https://github.com/Laravel-Backpack/PRO/pull/232

[tabacitu]

Please please PLEASE start writing a title and description for every PR you create Pedro. It takes 2-3 minutes, man. And it helps YOU and the rest of the team understand what's happening there.

I swear I will start closing PRs that are not descriptive. You've been warned.

[pxpm]

Check #5484 before merging this.

[karandatwani92]

Hey @pxpm

I tested fields of uploaders-test-branch. I'm facing many issues with it:

• 1) Base64 Image CRUD::field('image')->type('image'); results✔️(fixed in demo branch):
  

• 2) Dropezone not working without validation, results:❌

• 3) EasyMDE
  Setting custom destination & path breaks it.❌

CRUD::field('easymde')->type('easymde')->withFiles([
  'disk' => 'public',
  'path' => 'destinations'
]);

• 4) Upload Multiple IDK exactly when, but some combination empty the array and removes the past stored items of another Multiple Upload field.❌

I'm considering testing subfields after the Green Flag on the above direct-use fields.

[pxpm]

I am sorry @karandatwani92

But I am not able to reproduce any of the issue you mentioned:

I think I made the mistake of giving the test branch I used, I should have just let you guys create your own tests. It was intended to be used aa a guide, the branch it's not working AFAIK, it's missing routes for example.

I think in 1) it's clearly missing ->withFiles(). 🤷

I couldn't reproduce any of the other bugs you mentioned. The image of the easy mde in the screenshot is in:

CRUD::field('easymde')->type('easymde')->withFiles([
            'path' => 'test',
        ]);

Let me know if I can help you with something.

[karandatwani92]

Hey @pxpm, I'm facing the same issues even with a fresh installation.
PLUS, I found one more bug:

• Using an upload field with ['disk'=>'public', 'path'=>'destination'] attributes appends the path to the stored path(old file on different path), which breaks the URL. It is not supposed to append anything to the DB stored path/value. The deletion of the old file is working fine in this case.

[pxpm]

@karandatwani92 test issues:

• Dropzone don't work without defining validation.

• EasyMDE appends an extra slash (/) when defining the upload with some custom path and that breaks file url.

• Single file upload behaves different than multiple file. Single file upload builds the file using the disk and prefix, while multiple uploads use the file as is in the database.
  I am not sure this one is related with this PR specifically. As far as I can see, this PR does not touch Uploaders.php class, so it does not change the retrieve behavior, nor the changes done in upload.blade.php touch any of this bit.
  Looking at upload.blade.php history https://github.com/Laravel-Backpack/CRUD/blame/main/src/resources/views/crud/fields/upload.blade.php I don't see any changes that would have changed this behavior, so I can assume that's something @karandatwani92 found while testing this, but it's not specifically related to this changes, so I opened [WIP] upload vs upload_multiple file display inconsistent behavior.  #5491 for further discussion on the topic.

[karandatwani92]

Hey @pxpm

The above bugs are fixed✅. Now I started testing subfields under repeatable and there is an issue:

• Suppose we created two rows and saved them. Go to Edit and Reorder - (It breaks saving: causes validation error)❌

[pxpm]

Hey @pxpm

The above bugs are fixed✅. Now I started testing subfields under repeatable and there is an issue:

• Suppose we created two rows and saved them. Go to Edit and Reorder - (It breaks saving: causes validation error)❌

Thanks @karandatwani92 haven't spotted that. 🙏 nice catch!

I've just pushed fixes both in this PR and in PRO PR to address that issue.
Please note that I rebased both branches, so if you have any troubles with a git pull, please just delete the local branch and download a new copy.

Thanks again 🙏

[karandatwani92]

Hey @pxpm

New Issue: Create new, edit, and save without changes(validation error even when the file is already attached):

[karandatwani92]

New Bugs:

1. Message for Default Validation Error is missing. (See the difference.)
   
2. If I switch the repeatable row position, the subfield upload value stays, but the file gets deleted.
   

[pxpm]

1 is not a bug, in the demo app mimetypes is not added to translations, it comes by default with Laravel since Laravel 9 if I am not mistaken, we never bothered adding it to the demo because we don't use it.

2 might be a bug, I am not sure if I have a test case covering that, if not I will add one and evaluate that scenario.

Thanks.

[pxpm]

@karandatwani92 I've identified the issue you reported in 2 and added a test alongside with the fix for it. 🙏

[karandatwani92]

Hey @pxpm

I tested it, and it is working completely fine. Please continue with the media uploader and we'll re-test again.