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.

Sign up for GitHub

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

Jump to bottom

Display server error message when one exists #7434

Merged
merged 3 commits into from
Jun 22, 2018
Merged

Display server error message when one exists #7434

merged 3 commits into from
Jun 22, 2018

Conversation

danielbachhuber
Copy link
Member

Description

If the server rejects a media upload and includes a message, display that message in the error UI.

Fixes #7137

How has this been tested?

Add the following code snippet in functions.php or similar:

$force_upload_failure = function( $file ) {
	$file['error'] = 'This file upload has been forced to fail.';
	return $file;
};
add_filter( 'wp_handle_sideload_prefilter', $force_upload_failure );
add_filter( 'wp_handle_upload_prefilter', $force_upload_failure );

Attempt to upload an image into the image block. Observe This file upload has been forced to fail. as the error message.

Screenshots

image

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@danielbachhuber danielbachhuber added [Feature] Media Anything that impacts the experience of managing media REST API Interaction Related to REST API labels Jun 21, 2018
@danielbachhuber danielbachhuber added this to the 3.2 milestone Jun 21, 2018
@danielbachhuber danielbachhuber requested a review from a team June 21, 2018 13:05
gziolo
gziolo reviewed Jun 21, 2018
View reviewed changes
utils/mediaupload.js Outdated
code: 'GENERAL',
message: sprintf(
let message;
if ( ! isEmpty( response.responseJSON.message ) ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be safer to use has or get from lodash in here in case responseJSON is not set:

if ( has( response, [ 'responseJSON', 'message' ] ) {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion. Updated in 20d673a

@danielbachhuber danielbachhuber requested a review from a team June 21, 2018 15:58
gziolo
gziolo approved these changes Jun 22, 2018
View reviewed changes
Copy link
Member

@gziolo gziolo left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code-wise, it looks good. It has some merge conflicts to be resolved before merging.

@danielbachhuber danielbachhuber merged commit b16a8b2 into master Jun 22, 2018
@danielbachhuber danielbachhuber deleted the 7137-display-error-2 branch June 22, 2018 12:16
oxyc added a commit to generoi/gutenberg that referenced this pull request Jun 26, 2018
@oxyc
Merge branch 'master' of https://github.com/WordPress/gutenberg
118219b 
* 'master' of https://github.com/WordPress/gutenberg: (69 commits)
  fix: Show permalink editor in editor (WordPress#7494)
  Fix text wrapping in Firefox. (WordPress#7472)
  Try another approach to fixing the sibling inserter in Firefox (WordPress#7530)
  fix: Improve "add block" text in NUX onboarding (WordPress#7511)
  Implement core style of including revisions data on Post response (WordPress#7495)
  Testing: Add e2e test for PluginPostStatusInfo (WordPress#7284)
  Add end 2 end test for sidebar behaviours on mobile and desktop. (WordPress#6877)
  Only save metaboxes when it's not an autosave (WordPress#7502)
  Fix broken links in documentation (WordPress#7532)
  Remove post type 'viewable' compatibility shim (WordPress#7496)
  Fix typo. (WordPress#7528)
  Blocks: Remove wrapping div from paragraph block (WordPress#7477)
  fix: change import for InnerBlocks (WordPress#7484)
  Polish library just a teeeeensy bit (WordPress#7522)
  feat: Add snapshot update script (WordPress#7514)
  Display server error message when one exists (WordPress#7434)
  Fix issues with gallery in IE11. (WordPress#7465)
  Polish region focus style (WordPress#7459)
  Fix IE11 formatting toolbar visibility (WordPress#7413)
  Update plugin version to 3.1. (WordPress#7402)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gziolo gziolo gziolo approved these changes

Assignees
No one assigned
Labels
[Feature] Media Anything that impacts the experience of managing media REST API Interaction Related to REST API
Projects
None yet
Milestone
3.2
Development

Successfully merging this pull request may close these issues.
2 participants
@danielbachhuber @gziolo