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

cf2 file fields, only one file is attached and uploaded #2818

Closed
Shelob9 opened this issue Nov 21, 2018 · 5 comments
Closed

cf2 file fields, only one file is attached and uploaded #2818

Shelob9 opened this issue Nov 21, 2018 · 5 comments
Assignees
Labels
Milestone

Comments

@Shelob9
Copy link
Collaborator

Shelob9 commented Nov 21, 2018

@New0 found that for files field, when multiple is enabled, only one file is attached to mailer and one file uploaded to media library.

@Shelob9 Shelob9 added the Bug label Nov 21, 2018
@Shelob9 Shelob9 added this to the 1.8.0 milestone Nov 21, 2018
@Shelob9
Copy link
Collaborator Author

Shelob9 commented Nov 21, 2018

Relateded sort of: #2814

New0 added a commit that referenced this issue Nov 26, 2018
Error "TypeError: "b.jqxhr.responseJSON is undefined"" will seems to be fixed by #2820
New0 added a commit that referenced this issue Nov 26, 2018
@Shelob9
Copy link
Collaborator Author

Shelob9 commented Nov 27, 2018

  1. There is a typo in the inline docs about what type of argument for function hashAndUpload in render/index.js

  2. I observed that if I have one file field and I add two files, when I submit, I only see 2 HTTP requests, I would expect three. In the media library I see that only the first was added, I would expect to see two.

The tests here, which are server-side tests prove that if the right data was already on the server, it would attach both.

I think the problem is purely in the client, only one upload happens before allowing submission to complete.

@Shelob9
Copy link
Collaborator Author

Shelob9 commented Nov 27, 2018

After uploading first file ~L155 clients/render/index.js the field is removed from pending, without triggering upload of second file.

else if (response.hasOwnProperty('control')) {
									removeFromPending(fieldId);
									removeFromBlocking(fieldId);
									cf2.uploadCompleted.push(fieldId);
									$form.submit();
								}else{```

@Shelob9
Copy link
Collaborator Author

Shelob9 commented Nov 27, 2018

To make this testable, we need to move the closure for cf.ajax.request a function. Then we need to make the response handlers separate functions. Then we can test that if there are 2 files, only gets removed from cf2.pending if both are uploaded. We need to test that the cf2 object is updated correctly.

Given that file field has 2 files. Expect one file uploaded and submit is still blocked based on that field. Given that second file has uploaded, expect submission not to be blocked.

Mocking jQuery - https://stackoverflow.com/a/45997857/1469799

Pseudo-tests https://gist.github.com/Shelob9/0ed44838d53e3aacd349136f5a6d7135#file-pseudo-tests-js

Shelob9 pushed a commit that referenced this issue Dec 18, 2018
Shelob9 pushed a commit that referenced this issue Dec 18, 2018
Shelob9 pushed a commit that referenced this issue Dec 18, 2018
Shelob9 pushed a commit that referenced this issue Dec 18, 2018
New0 added a commit that referenced this issue Dec 21, 2018
New0 added a commit that referenced this issue Jan 3, 2019
New0 added a commit that referenced this issue Jan 4, 2019
- Made hashAndUpload testable
- Added a processFiles function to expect a result based on the files array
New0 added a commit that referenced this issue Jan 7, 2019
New0 added a commit that referenced this issue Jan 7, 2019
New0 added a commit that referenced this issue Jan 7, 2019
New0 added a commit that referenced this issue Jan 7, 2019
… -> Values set to three files) that was previously failing
New0 added a commit that referenced this issue Jan 15, 2019
This fixes #2818 


* Make the request event an exportable const in util.js
First export, the onRequest const will be cut into smaller pieces finally have response handlers seperate functions

* WIP examples of tests needed, needs to be adjusted

* WIP work on request functions
Cut to minimum peces, then will add the callbacks needed to test return values

* revert most changes in last commit #2818

* add missing mock data file #2818

* tests for cf2 util methods #2818

* use new cf2 var util methods in render/index #2818

* #2818 WIP tests for fileUpload callbacks

* Work on tests for handleFileUploadResponse #2818

* Move fileUploads file to render folder instead of tests/render folder #2818

* Add fileUploads.js to render ( not attached to previous commit) #2818

* Use handleFileUploadResponse(response,cf2,$form,messages,field) in index.js #2818

* Document handleFileUploadResponse() #2818

* #2818 make createMediaFromFile testable

* #2818 make the Files process testable  ( WIP needs more tests)
- Made hashAndUpload testable
- Added a processFiles function to expect a result based on the files array

* #2818 Document processFiles and hashAndUpload in clients/render/fileUploads

* #2818 Test number of calls to hashAndUpload based on number of files passed to processFiles

* Make files const passed to processFiles testable
Created processFileField function

* Update processFileField description #2818

* Setup jQuery for jest #2818

* Failing test that shows the first issue that leads to only one File being uploaded #2818

* Fix #2818 and pass test ( Test the files const passed to processFiles -> Values set to three files) that was previously failing

* Remove export of spark-md5 from util

* Shorten test data definitions

* Adapt test to $form.submit

* Document handleFileUploadError

* Document lastFile argument in handleFileUploadResponse

* Add theComponent to processData before processFileField

* Test different number of files as values passed to processFileField

* Make translatable strings for text

* Set jQuery version to 1.12.4

* Remove old skipped test ( moved this to fileUploads.test.js )

* Add translations to mocked data

* Work on the Travis test that's failing for trunk version of WordPress (Test passes in phpstorm)

* Restore test 'Test the files const passed to processFiles'

* Define shouldBeValidating

* Reordered tests in render/fileUpload

* Update branch with develop

* Update vendor folder

* Revert "Update vendor folder"

This reverts commit 2e521ed.

* Update vendor

* Update composer.lock
New0 added a commit that referenced this issue Jan 21, 2019
* Make the request event an exportable const in util.js
First export, the onRequest const will be cut into smaller pieces finally have response handlers seperate functions

* WIP examples of tests needed, needs to be adjusted

* WIP work on request functions
Cut to minimum peces, then will add the callbacks needed to test return values

* revert most changes in last commit #2818

* add missing mock data file #2818

* tests for cf2 util methods #2818

* use new cf2 var util methods in render/index #2818

* #2818 WIP tests for fileUpload callbacks

* Work on tests for handleFileUploadResponse #2818

* Move fileUploads file to render folder instead of tests/render folder #2818

* Add fileUploads.js to render ( not attached to previous commit) #2818

* Use handleFileUploadResponse(response,cf2,$form,messages,field) in index.js #2818

* Document handleFileUploadResponse() #2818

* #2818 make createMediaFromFile testable

* #2818 make the Files process testable  ( WIP needs more tests)
- Made hashAndUpload testable
- Added a processFiles function to expect a result based on the files array

* #2818 Document processFiles and hashAndUpload in clients/render/fileUploads

* #2818 Test number of calls to hashAndUpload based on number of files passed to processFiles

* Make files const passed to processFiles testable
Created processFileField function

* Update processFileField description #2818

* Setup jQuery for jest #2818

* Failing test that shows the first issue that leads to only one File being uploaded #2818

* Fix #2818 and pass test ( Test the files const passed to processFiles -> Values set to three files) that was previously failing

* Remove export of spark-md5 from util

* Shorten test data definitions

* Adapt test to $form.submit

* Document handleFileUploadError

* Document lastFile argument in handleFileUploadResponse

* Add theComponent to processData before processFileField

* Test different number of files as values passed to processFileField

* Make translatable strings for text

* Set jQuery version to 1.12.4

* Remove old skipped test ( moved this to fileUploads.test.js )

* Add translations to mocked data

* Work on the Travis test that's failing for trunk version of WordPress (Test passes in phpstorm)

* Restore test 'Test the files const passed to processFiles'

* Define shouldBeValidating

* Reordered tests in render/fileUpload

* Update branch with develop

* Update vendor folder

* Revert "Update vendor folder"

This reverts commit 2e521ed.

* Update vendor

* Update composer.lock
@New0
Copy link
Collaborator

New0 commented Jan 21, 2019

Fixed by #2910

@New0 New0 closed this as completed Jan 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants