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

Fix API url when editing image alt text in admin #2625

Merged
merged 1 commit into from
Apr 18, 2018

Conversation

loicginoux
Copy link
Contributor

When you upload a product image in BO, you can update the alternative text and save the change.
When doing this the url generated is
/en/api/variants/<PRODUCT_SLUG>/images/4

This is not actually causing any bug on the default solidus behaviour because the variant is not used in
Spree::Api::ImagesController#update but if we would like to use the variant via the method Spree::Api::ImagesController#scope for example, it would produce an error (this is the case in our app for example)

     def scope
        if params[:product_id]
          Spree::Product.friendly.find(params[:product_id])
        elsif params[:variant_id]
          Spree::Variant.find(params[:variant_id])
        end
      end

Example of log when saving the image alternative text:

Started PUT "/en/api/variants/75-aoc-esp-rioja-crianza-rg-marques-caceres-13c6-acq/images/19" for 83.60.66.210 at 2018-03-12 11:38:56 +0100
Processing by Spree::Api::ImagesController#update as JSON
   Parameters: {"image"=>{"alt"=>"seigneur dagil"}, "variant_id"=>"75-aoc-esp-rioja-crianza-rg-marques-caceres-13c6-acq", "id"=>"19", "locale"=>"en"}

As you see from the log we could not find the variant via the product slug.

Replacing the path by
/en/api/products/<PRODUCT_SLUG>/images/4
should fix this issue.

@loicginoux loicginoux changed the title fix path when editing image in bo fix path when editing image alternative text in bo Mar 12, 2018
@loicginoux loicginoux changed the title fix path when editing image alternative text in bo fix path when editing image alternative text in backoffice Mar 12, 2018
Copy link
Contributor

@jhawthorn jhawthorn left a comment

Choose a reason for hiding this comment

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

This change looks correct. Also, maybe we should be using that scope in #update, the route we're hitting shouldn't work here (let's merge this and fix that in a separate PR)

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks!

@gmacdougall gmacdougall merged commit ce02135 into solidusio:master Apr 18, 2018
@jhawthorn jhawthorn changed the title fix path when editing image alternative text in backoffice Fix API url when editing image alt text in admin May 9, 2018
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.

4 participants