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

User friendly error when creating a product with an image in an unsupported format #3923

Merged
merged 1 commit into from Jun 26, 2019

Conversation

luisramos0
Copy link
Contributor

What? Why?

Closes #764

Instead of a stack trace the user now sees a nice error message:
Screenshot 2019-06-12 at 23 02 58

We do this by catching the exception in the controller.

What should we test?

Product creation still works correctly.
Uploading a good image still works.
Uploading an image in a weird format (I tested with exr) shows a friendly error.

Release notes

Changelog Category: Fixed
Creating a product with an unsupported image format now shows a user friendly error.

@luisramos0 luisramos0 self-assigned this Jun 12, 2019
@luisramos0 luisramos0 changed the title User friendly error when uploading product image in an unsupported format User friendly error when creating a product with an image in an unsupported format Jun 12, 2019
Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

Cool. 💯 for better UX.

@@ -33,6 +33,10 @@ def create
delete_stock_params_and_set_after do
super
end
rescue Paperclip::Errors::NotIdentifiedByImageMagickError
invoke_callbacks(:create, :fails)
@object.errors.add(:base, t('spree.admin.products.image_upload_error'))
Copy link
Contributor

Choose a reason for hiding this comment

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

may I suggest we let the user know which formats we accept? I think this is the kind of messages that confuse the user "which format am I supposed to use?"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tbh I have no clue what is that list...
I dont see this ImageMagick in our dependencies list... do you know if we use it?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can recommend common formats?

- Error uploading image, you may need to use a different image format
+ This image was not recognised. Please upload an image in PNG or JPG format.

Copy link
Contributor

Choose a reason for hiding this comment

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

We install ImageMagick via ofn-install I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nice one, I found this list: https://imagemagick.org/script/formats.php

Matt: that would make sense.

I'll follow Maikel's advise, looks reasonable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

…and send appropriate error message to client when product image upload fails
@RachL RachL self-assigned this Jun 20, 2019
@RachL RachL added the pr-staged-fr staging.coopcircuits.fr label Jun 20, 2019
@RachL
Copy link
Contributor

RachL commented Jun 20, 2019

@luisramos0 I didn't manage to make it work. I always have an error 500 even with good format like .png or .jpg (by the way I think we need to list these formats somewhere).

Here are my testing notes with link to the image:
https://docs.google.com/document/d/152ZQxXvcK2FAYURYcGmaIIwlH2Vh7NO3NM0RraIG-k8/edit#

Back to you

@RachL RachL removed the pr-staged-fr staging.coopcircuits.fr label Jun 20, 2019
@luisramos0
Copy link
Contributor Author

image uploads are broken in staging FR, can you pls test this in staging uk for example?

@RachL RachL added the pr-staged-uk staging.openfoodnetwork.org.uk label Jun 25, 2019
@RachL
Copy link
Contributor

RachL commented Jun 25, 2019

@luisramos0 indeed it works on UK staging. however I've noticed that you added that friendly message only to product creation, not editing. Is that correct? So we need to open a new issue for that. Moreover, uploading a .svg image does not work as well, but the user is not warned, he only sees an empty spot:

image

@luisramos0
Copy link
Contributor Author

nice.

the goal here is really to guide the user who is creating a product and sees a snail because of the image but has no clue what field is causing the problem...

there's no image in product edit, only edit images, it's a different problem and not so important. see last paragraph here:
#764 (comment)

I'd create a new issue for that and also for the svg problem you describe.

@RachL
Copy link
Contributor

RachL commented Jun 26, 2019

@luisramos0 if you use the submenu Images in product edit and you upload an unknown format, you will see a snail / Error 500. Same pattern than in product creation. If you click on the image in the bulk edit page, then the spinner will stop at some point with no indication. This is indeed less bad. But yes let's merge this as it improves the current situation anyway. I've opened #3976 for the error 500 and #3975 for the other type of image file.

@RachL RachL removed the pr-staged-uk staging.openfoodnetwork.org.uk label Jun 26, 2019
@luisramos0
Copy link
Contributor Author

yes :+1 in both those situations the user is explicitly updating the image, so the user will know the image is the problem. when creating a product, the user has no clue if it's the image or some other field.

@luisramos0 luisramos0 merged commit 00841cb into openfoodfoundation:master Jun 26, 2019
@luisramos0 luisramos0 deleted the prod_image_error branch June 26, 2019 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Product creation crashes with wrong image
5 participants