solidus icon indicating copy to clipboard operation
solidus copied to clipboard
verified publisher icon solidusio

[Admin][Backend] Cannot edit the `Alternative Text` field on a product's image

Open EmCousin opened this issue 6 years ago • 4 comments

Solidus Version: 2.9.0

To Reproduce

  1. Go to /store/admin/products/[A-PRODUCT-ID]/images
  2. Edit the Alternative Text field of an existing image
  3. The Loading... loader shows up and stays indefinitely

TypeError: response.responseJSON is undefined

Current behavior

The Loading... loader shows up and stays indefinitely. When refreshing the page, the Alternative Text field has not been updated

Expected behavior

The Loading... loader shows up and eventually disappears. When refreshing the page, the Alternative Text field has been updated

Screenshots

image

Desktop (please complete the following information):

  • OS: Mac OS Mojave 10.14.6
  • Browser: Mozilla Firefox Developer Edition
  • Version: 71.0b7 (64 bits)

Additional context

  • Getting a TypeError: response.responseJSON is undefined JS error in the browser console
  • Creating an image with an Alternative Text value works fine.

EmCousin avatar Nov 08 '19 14:11 EmCousin

Hi @EmCousin! I tested this scenario in the following versions: 2.10 and 2.9 and I couldn't reproduce it. solidus_test_1

Desktop

  • OS: Mac OS Catalina 10.15.2
  • Browser: Mozilla Firefox
  • Version: 73.0.1 (64 bits)

Did you customize the JS in your Solidus Admin? or Did you upgrade from a previous version?

jaimelr avatar Mar 04 '20 02:03 jaimelr

Hi @jaimelr,

It turns out I had overriden the Spree::Api::BaseController to add a before_action :authenticate_token! callback. That callback returns head :unauthorized when the authentication fails (which is actually the case in the context of this issue), i.e. no JSON response that can be properly parsed by this line, therefore triggering the JS error TypeError: response.responseJSON is undefined

Editing that line to something like show_flash('error', response.responseJSON && response.responseJSON.error || "An error occurred"); would do the trick. If you agree, I can push a PR?

EmCousin avatar Mar 11 '20 14:03 EmCousin

Awesome! Well, makes sense to add a default error message here but I'll let maybe @kennyadsl decide if it worth at this moment. 🙂

jaimelr avatar Mar 13 '20 01:03 jaimelr

@SamuelMartini what do you think about this one in terms of API consistency with the rest of Solidus? If unauthorized maybe we can avoid showing the feature at all also?

kennyadsl avatar Mar 13 '20 07:03 kennyadsl