New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uploaders - Refactor and fixes #5478
base: main
Are you sure you want to change the base?
Conversation
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. |
Check #5484 before merging this. |
Hey @pxpm I tested fields of uploaders-test-branch. I'm facing many issues with it:
I'm considering testing subfields after the Green Flag on the above direct-use fields. |
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 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. |
Hey @pxpm, I'm facing the same issues even with a fresh installation.
|
@karandatwani92 test issues:
|
Hey @pxpm The above bugs are fixed✅. Now I started testing subfields under
|
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. Thanks again 🙏 |
Hey @pxpm New Issue: Create new, edit, and save without changes(validation error even when the file is already attached): |
[ci skip] [skip ci]
[ci skip] [skip ci]
[ci skip] [skip ci]
[ci skip] [skip ci]
[ci skip] [skip ci]
[ci skip] [skip ci]
[ci skip] [skip ci]
[ci skip] [skip ci]
[ci skip] [skip ci]
[ci skip] [skip ci]
[ci skip] [skip ci]
[ci skip] [skip ci]
[ci skip] [skip ci]
Add uploaders tests
[ci skip] [skip ci]
1 is not a bug, in the demo app 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. |
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