-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Adding failure callback to ui file uploader #27092
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
Adding failure callback to ui file uploader #27092
Conversation
Hi @bartoszkubicki. Thank you for your contribution
For more details, please, review the Magento Contributor Guide documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @bartoszkubicki!
Could you please address build failures.
fabfd53
to
9492001
Compare
@lenaorobei I have fixed some issue, but now I am stuck. Unit tests are marked as failed, but there is no info what exactly failed even if I visit allure report (and I am writing js not php!). Functional tests failed because of strange looking communicate - something about browser and selenium - but it doesn't seem to be connected to code changes. Static tests are failed because of copy-paste in some module I didn't touched. What are we supposed to do with this one? |
@bartoszkubicki sorry for the late response. I'm going to update your branch to see new build results. Most likely static failures are related to the es6. |
@magento run all tests |
@magento run Static Tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the build console log:
Error Message
Parsing error: The keyword 'let' is reserved
Stacktrace
line 78, col 13, Error - Parsing error: The keyword 'let' is reserved
Please avoid the use of ES6 syntax. Thank you.
@lenaorobei Is there any reason why anyone should not use es6? Why there is such a statement in static test? |
I think @DrewML is the best person to answer this question. |
@magento create issue |
@lenaorobei I was curious why such a standard is kept. ECMA 6 seems to be quite supported in all main browsers. |
@bartoszkubicki historically this decision is was made to keep IE compatibility. Now it requires revision and reconsideration, but that's the rule for now. |
af081f2
to
4251151
Compare
@lenaorobei Didn't know that we are going to deal with such a archeology ;) Code corrected, hope this time all checks will pass. |
4251151
to
db3554a
Compare
@magento run all tests |
Hi @gabrieldagama, thank you for the review. |
@gabrieldagama you were faster :) I was going to look at that this weeekend. So everything is ready? |
@magento run all tests |
✔️ QA passed |
Note: Functional Tests B2B , Functional Tests CE and Functional Tests EE are failed |
The failing tests are not related to this PR. |
Note: Automation tests are passed |
Hi @bartoszkubicki, thank you for your contribution! |
@lenaorobei Please back-port it to magento 2.3 |
Description (*)
Magento_Ui
file uploader works fine. However, recently when I have used it for custom file upload, and when I had error while uploading file there was no info what happened. Uploader just stopped uploading. After debugging it turned out it was 413 from nginx, and next steps were clear (bumping post, upload and client body size). I think if we have an error we should get info in dev tools console.Magento_Downloadable
file uploader in adminhtml is heavily based on this uploader, so whole thing can be investigated and tested while creating downloadable product.By the way, I had refactored formatting a little bit.
Manual testing scenarios (*)
client_max_body_size
)Questions or comments
Contribution checklist (*)
Resolved issues: