-
Notifications
You must be signed in to change notification settings - Fork 800
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
VideoPress dashboard: improve upload error handler #38769
Conversation
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Follow this PR Review Process:
Still unsure? Reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Videopress plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
da7f365
to
4e05d66
Compare
5c8d7e9
to
6255dc6
Compare
6255dc6
to
eac9931
Compare
…uldRetry to abort retries on 400 errors (mostly unrecoverable for now)
…ep errored videos separate and transient
eac9931
to
7b2bcde
Compare
// withCredentials: false, | ||
// autoRetry: true, |
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.
Not needed?
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.
Actually no use for those. There are no such options on the underlying lib
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.
LGTM. Very well done! Thanks!
Thanks @CGastrell ! Two questions:
|
Releasing today if everything goes well |
Thanks! I've updated that article to make it clear that this problem could be due to a broken Jetpack Connection. |
* add debug calls, minor code improvements (map -> forEach, await/async functions, etc) * changelog entry * less debug on uploader lib * make onBeforeRequest async (as expected by tus client lib). Add onShouldRetry to abort retries on 400 errors (mostly unrecoverable for now) * add state action/reducer/selector for SET_VIDEO_UPLOADING_ERROR to keep errored videos separate and transient * include errored videos in all dashboard considerations and derived flags * add state action/reducer to clear upload errors * add error components for grid and row views * implement new error components on grid and list * remove unused parameters --------- Co-authored-by: Douglas <[email protected]>
Fixes https://github.com/Automattic/videopress/issues/1038
Proposed changes:
This PR addresses unhandled upload errors on VideoPress dashboard, providing error components for both list and row views of VideoPress dashboard.
Other information:
Does this pull request change what data or activity we track or use?
No
Testing instructions:
The next instructions should be tested on:
Create an instance, follow the upgrade flow for VideoPress. Once done, you should be able to access VideoPress' dashboard at
wp-admin/admin.php?page=jetpack-videopress#/
The upload process should not show any improvements or handling, but it should work as before.
At this point, the XML backend connection should not be working, hence, video uploads should fail
Go back to VideoPress dashboard and upload a video. It should fail and the video card should show a warning sign. Click on the button to get some context and a link to our troubleshooting guide. Click on the trash icon to get rid of the error.
Do it again, but switch to list/grid view and verify the same functionality exists.
Disabling "Disable XML-RPC" plugin should get things back to normal, but beware of this step on local environments as it turns to get tricky (unsure why).