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

Set error state when there is an upload error in during file upload #24017

Merged
merged 1 commit into from
Aug 5, 2020

Conversation

kirilzh
Copy link
Contributor

@kirilzh kirilzh commented Jul 17, 2020

Description

Fixes #23579

The app was hanging in an infinite loading state when file upload resulted in an error because of an unsupported file type. The problem was that in here

validFiles.push( mediaFile );
// Set temporary URL to create placeholder media file, this is replaced
// with final file from media gallery when upload is `done` below
filesSet.push( { url: createBlobURL( mediaFile ) } );
onFileChange( filesSet );

the uploaded file is not recognized but is considered to be a valid one. Next when onFileChange is called it runs

onSelectFile( media ) {
if ( media && media.url ) {
this.setState( { hasError: false } );
this.props.setAttributes( {
href: media.url,
fileName: media.title,
textLinkHref: media.url,
id: media.id,
} );
}
}

which sets hasError state to false. At this point the server has not yet responded with an error, so the front-end shows a loader screen. Next the API call is made and createMediaFromFile throws.

try {
const savedMedia = await createMediaFromFile(
mediaFile,
additionalData
);
const mediaObject = {
...omit( savedMedia, [ 'alt_text', 'source_url' ] ),
alt: savedMedia.alt_text,
caption: get( savedMedia, [ 'caption', 'raw' ], '' ),
title: savedMedia.title.raw,
url: savedMedia.source_url,
};
setAndUpdateFiles( idx, mediaObject );
} catch ( error ) {
// Reset to empty on failure.
setAndUpdateFiles( idx, null );
let message;
if ( has( error, [ 'message' ] ) ) {
message = get( error, [ 'message' ] );
} else {
message = sprintf(
// translators: %s: file name
__( 'Error while uploading file %s to the media library.' ),
mediaFile.name
);
}
onError( {
code: 'GENERAL',
message,
file: mediaFile,
} );
}

onError runs but the state is never updated in

onUploadError( message ) {
const { noticeOperations } = this.props;
noticeOperations.removeAllNotices();
noticeOperations.createErrorNotice( message );
}

How has this been tested?

Manually tested with a file with .unsupported extension.

Screenshots

image

Types of changes

Bug fix (non-breaking change)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.

@ZebulanStanphill ZebulanStanphill added the [Type] Bug An existing feature does not function as intended label Aug 5, 2020
@ItsJonQ ItsJonQ self-requested a review August 5, 2020 14:39
@ItsJonQ
Copy link

ItsJonQ commented Aug 5, 2020

This looks great! I'll test it out on my machine in about 30-60 mins. Will provide an update + review when I can!

Copy link

@ItsJonQ ItsJonQ left a comment

Choose a reason for hiding this comment

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

🚀 from me! Thanks @kirilzh for helping with this. I tested it locally, and can confirm that unsupported extensions (correctly) trigger the error message.

@ItsJonQ ItsJonQ merged commit b1c1fb8 into WordPress:master Aug 5, 2020
@github-actions github-actions bot added this to the Gutenberg 8.8 milestone Aug 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

File block doesn't handle unsupported upload error
3 participants