fix: fix clearing uploading status before upload success event

[sissbruecker]

Description

Currently the upload element clears (setting it to false) the uploading status on a file element when:

• there is a progress event
• and the progress equals 100

...but before the XHR ready state changes, and thus possibly some time before the upload-success or upload-failed event is sent.

This can be a problem in the Flow component when doing multi-file uploads. Basically whenever the Flow component receives a success or failure event, it will check if all files have uploading == false and then report that all uploads are finished. But when using multiple files that state can be true before all files have sent their success event, which can lead to the following observed event flow on the Flow component:

File 1 success
File 2 success
All finished
File 3 success

In the example above, when File 2 notified about the success event, progress for File 3 was already at 100%, so the uploading flag was already set to false which lead to the Flow component reporting all finished. However the ready state of the XHR did not change yet and thus there was no success event for File 3 yet.

This change modifies the behaviour of the uploading flag, so that it is only cleared when the XHR ready state changes, which is the same time when the element sends the success or failure event. The meaning of the uploading flag is not 100% clear to me as it is not documented, and maybe it's misused by the Flow component. However there is no other flag that indicates that the upload is finished (either success or failure) and it seems more logical to me to connect this flag to the ready state change event instead of a progress event.

For tests I simply changed the expectations of the existing tests, I could not identify any further tests that would be necessary.

Fixes vaadin/vaadin-upload#368

Type of change

• Bugfix

Checklist

• I have read the contribution guide: https://vaadin.com/docs-beta/latest/guide/contributing/overview/
• I have added a description following the guideline.
• The issue is created in the corresponding repository and I have referenced it.
• I have added tests to ensure my change is effective and works as intended.
• New and existing tests are passing locally with my change.
• I have performed self-review and corrected misspellings.

[sissbruecker]

An alternative approach would be to introduce state to either the web or Flow component about the number of pending uploads and decrease them after an upload finished event.

[sissbruecker]

Unrelated, but this was easy to fix. The _createXhr method does not accept a parameter.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[web-padawan]

LGTM.

Looks like setting uploading flag to false was there since the initial implementation vaadin/vaadin-upload@b9f64ff. And the UploadFlow was built by the Flow team later. Apparently they didn't face this edge case back then, that's why the line wasn't removed.