Skip to content
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

Multiple upload #94

Closed
wants to merge 44 commits into from

Conversation

Elfangor93
Copy link
Member

@Elfangor93 Elfangor93 commented Apr 24, 2023

Multi-upload with uppy, tus and async-sema.

This PR adds a form to upload multiple images into the same category.
You find the form in the Joomla backend in Components > JoomGallery > Images > New > Multiple New.

All configuration settings which are not red in the tab "General Settings" (level one tab) are taken into account and will influence the result or the upload process itself.

Upload form

grafik

On the right hand side the general image data can be inserted, the same way is it was done in JG3 Drag'N'Drop uploader. Data enterd here is used for all the images.
Inside the uppy form to the left there is a edit button below each image preview allowing to adjust title, author and description for each image individually. This data on each image will override the general data from the right form if available.
If activated in the configuration the image metadata will override the general and individual data from the upload form.
(Priority: Metadata > individual image data > general form data)

Below the uppy form the debug mode can be activated. This works similarly as in JG3 Drag'N'Drop uploader. But the uploader will display debug data additionally in the browser console.

Request protocol for one image

grafik
Multiple requests to the server are perfomed for each upload. Except for the last POST requests, all the requests are from the tus communication protocol uploading the files to the server.

System architecture

Test instructions

  • Component has to be new installed. There are changes in the DB which need to be applied.
  • Since this PR adds client side application (uppy form), we need to test on different browsers.
  • PR was coded using PHP 7.4.x. Has to bested on PHP 8.x.
  • If you find a bug please post as much as possible
    • Error message in the view
    • Browser console log (Browser developer tools -> Console tab)
    • Request response of the failing request (Browser developer tools -> Network analysis tab)
    • System details (PHP version, ...)

@AlexanderSupp
Copy link

I tested the actual status. Multiple uploads works. Images stored in temp file. It's great to see how multiple images load in the same time.
I wait for next action to see the images in the normal JG4 file structure.
BTW:
I uninstalled JG4 and then I installed multiple-upload. This installation process does not work.
I installed JG4 main and then multiple-upload and that works.

@reni68

This comment was marked as outdated.

@reni68

This comment was marked as outdated.

@MrMusic
Copy link
Member

MrMusic commented Apr 25, 2023

A hint from me:
Thanks for testing, but this PR status is still on 'Draft', which means it is still in progress and not yet finished...

@Elfangor93
Copy link
Member Author

Elfangor93 commented May 19, 2023

I uploaded 40 files with Nmb parallel processes 1 and everything works.
I uploaded the same 40 files with Nmb parallel processes 10, and I get errors.

@AlexanderSupp Can you please give me the output of the browser console too?
Maybe there is a JavaScript error...

@Elfangor93
Copy link
Member Author

Elfangor93 commented May 19, 2023

There is no error message, only the note:
"Upload failed ?
0 of 0 files uploaded"

@eumel1602 Can you please add the output of the browser console?
I am pretty sure that there are some JavaScript errors...

@Elfangor93
Copy link
Member Author

You can't make it bigger manually. bug?

The size of the uppy dashboard is dynamically calculeated in the uppy app. If you want that to be changed you have to open an issue in the uppy repository (https://github.com/transloadit/uppy). I can not change that behavior in JoomGallery.

@MrMusic
Copy link
Member

MrMusic commented May 19, 2023

First:
Most of the problems should be fixed - thank you @Elfangor93

Still a few problems for me:

1: When klicking on upload, there is a '404 Not Found' error in the browser console for each image. Example:
HEAD http://localhost/jg4-dev/administrator/index.php?option=com_joomgallery&task=images.tusupload&uuid=5281b736a4c9d30c8f9ed27080412aec
Nevertheless, all images are created.

2: Problem when uploading very large images (6000 x 4000px; about 10MB)
When using GD (surely there is an overflow of the memory limit):
Javascript Error: Uncaught (in promise) SyntaxError: JSON.parse: unterminated string at line 1 column 12 of the JSON data
in webpack://uppy-uploader/src/index.js: 87:23

When using ImageMagick:
Also no upload possible.
Debug Information:

Upload of file xxx using Uppy failed.
This file is too large to upload. You can change the limits for your site in the component options.
Save failed with the following error: Unable to upload file.

Where it is a limit here?
When using Imagemagick should the php memory limit not matter?

3: In general, it should be noted that a image record is always created in the database, even if an error occurs during the upload and no images are created.

@AlexanderSupp

This comment was marked as off-topic.

@AlexanderSupp
Copy link

I uploaded 40 files with Nmb parallel processes 1 and everything works.
I uploaded the same 40 files with Nmb parallel processes 10, and I get errors.

@AlexanderSupp Can you please give me the output of the browser console too? Maybe there is a JavaScript error...

I set Nmb parallel processes to 2. And I limited the images to 11.
So I hope you can find something that helps. It was my first time to use the browser console.

console-export-2023-5-19_12-46-5.txt

Screenshot 2023-05-19 123921
Screenshot 2023-05-19 124648

@eumel1602
Copy link
Collaborator

eumel1602 commented May 19, 2023

AlexanderSupp: "Take a look at my Config. Simply, I select a file. I select the pencil-icon. I typed in Title, Description, Author. I select save. I select upload.
Title and Description will not place in the fields. Author is OK."

Did you notice that this is a bug of the upload method:

Confirmed. Since this is an issue of the uppy library this has to be fixed in the uppy project. Bug is already reported in the
corresponding repository:
transloadit/uppy#4427

Only one of the three things is saved at a time. (The joomgallery V4 can't do anything about that)

@Elfangor93
Copy link
Member Author

Elfangor93 commented May 19, 2023

When klicking on upload, there is a '404 Not Found' error in the browser console for each image. Example:
HEAD http://localhost/jg4-dev/administrator/index.php?option=com_joomgallery&task=images.tusupload&uuid=5281b736a4c9d30c8f9ed27080412aec
Nevertheless, all images are created.

Yes. This is correct like that. If you upload the same image with uppy multiple times, uppy will send out a request to the TUS server checking if the image is already available in the destination location on the server. If not, the TUS server response back with a 404, telling the client that this image is not yet uploaded.
For more information see TUS docu

@Elfangor93
Copy link
Member Author

Debug Information:
Upload of file xxx using Uppy failed.
This file is too large to upload. You can change the limits for your site in the component options.
Save failed with the following error: Unable to upload file.
Where it is a limit here?

@MrMusic This limit comes from the com_media settings. We are using the filesystem-plugins to handle the filesystem. Since this plugins seem to read out some settings from com_media this limits also affect us now...
grafik

@AlexanderSupp
Copy link

AlexanderSupp commented May 19, 2023

Debug Information:
Upload of file xxx using Uppy failed.
This file is too large to upload. You can change the limits for your site in the component options.
Save failed with the following error: Unable to upload file.
Where it is a limit here?

@MrMusic This limit comes from the com_media settings. We are using the filesystem-plugins to handle the filesystem. Since this plugins seem to read out some settings from com_media this limits also affect us now...

Information:
In all my test systems, I set Media Maximum Size (in MB) to zero.

@AlexanderSupp
Copy link

AlexanderSupp: "Take a look at my Config. Simply, I select a file. I select the pencil-icon. I typed in Title, Description, Author. I select save. I select upload.
Title and Description will not place in the fields. Author is OK."

Did you notice that this is a bug of the upload method:

Confirmed. Since this is an issue of the uppy library this has to be fixed in the uppy project. Bug is already reported in the
corresponding repository:
transloadit/uppy#4427

Only one of the three things is saved at a time. (The joomgallery V4 can't do anything about that)

Thanks for this info. I haven't realized that.

@Elfangor93
Copy link
Member Author

Elfangor93 commented May 19, 2023

2: Problem when uploading very large images (6000 x 4000px; about 10MB)
When using GD (surely there is an overflow of the memory limit):
Javascript Error: Uncaught (in promise) SyntaxError: JSON.parse: unterminated string at line 1 column 12 of the JSON data
in webpack://uppy-uploader/src/index.js: 87:23

@MrMusic No a memory limit overflow looks like this:
grafik
What you get is an invalid json reply from the request. Can you please send me the content of the server request when this error happens? In there we should see what went wrong...

@eumel1602
Copy link
Collaborator

There is no error message, only the note:
"Upload failed ?
0 of 0 files uploaded"

@eumel1602 Can you please add the output of the browser console? I am pretty sure that there are some JavaScript errors...

I hope you mean this:
browserconsole

@Elfangor93
Copy link
Member Author

@eumel1602 Just to clarify: You get the above posted JavaScript error when you add more than 64 images to the uppy dashboard and press the upload button?

@eumel1602
Copy link
Collaborator

@eumel1602 Just to clarify: You get the above posted JavaScript error when you add more than 64 images to the uppy dashboard and press the upload button?

yes, exactly ! (in this case 99 pieces, small .jpg pictures...

@Elfangor93
Copy link
Member Author

You get the above posted JavaScript error when you add more than 64 images to the uppy dashboard and press the upload button?

Oke, I think this JavaScript errors are not solvable in an easy way. I have to shift this to a new PullRequest. I need to digg into uppy-plugin development for this. The error accures because of the hacky way I am doing the postprocessing of the uploads.

@MrMusic
Copy link
Member

MrMusic commented May 20, 2023

I can confirm the problem when Nmb parallel processes is greater 1 and image procesor is ImageMagick.
It is probably because it is trying to access the tmp/tmp_img.jpg file?

This limit comes from the com_media settings...

Thank you for the clarification. I had not expected this.

@reilldesign
Copy link
Collaborator

Image selection will be cleared if the form is invalid (miising title).

It would be great if the error is displayed but the image selection remains and does not need to be restarted.

Image-Selection

@eumel1602
Copy link
Collaborator

eumel1602 commented May 24, 2023

Image selection will be cleared if the form is invalid (miising title).

It would be great if the error is displayed but the image selection remains and does not need to be restarted.

You are probably not using the latest version of the PR? This bug has already been addressed and fixed...
Please download the new (current) PR again and reinstall it. Then that should work.
See my picture:
falle

Thank you for helping us!

@reilldesign
Copy link
Collaborator

I didn't notice the commit from May 19, 2023 because i had already installed it last week.

It works fine!

@Elfangor93
Copy link
Member Author

Development of the multiple upload continues in PR #103 since issue #100 can not be solved using the code provided in this PR.

@Elfangor93 Elfangor93 closed this Jun 5, 2023
@Elfangor93 Elfangor93 deleted the multiple-upload branch June 19, 2023 09:38
@Elfangor93 Elfangor93 removed needs testing This issue or pull request needs further testing GUI testing Tests can be perfomed using the GUI labels Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DB changed New installation needed to test this change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants