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

Spree::Product.updated_at should be modified after PATCH /api/products/{id} #6035

Closed
edmondchui opened this issue Feb 11, 2015 · 4 comments
Closed

Comments

@edmondchui
Copy link
Contributor

In the Spree::API::ProductController#update method

@product = find_product(params[:id])
authorize! :update, @product
options = { variants_attrs: variants_params, options_attrs: option_types_params }
@product = Core::Importer::Product.new(@product, product_params, options).update
if @product.errors.empty?
  respond_with(@product.reload, :status => 200, :default_template => :show)
else
  invalid_resource!(@product)
end

The updated_at is not modified by the call Core::Importer::Product#update.

Also, the @product.reload call seems unnecessary in the respond_with line.

I think the fix includes two parts:

  1. Set the updated_at to Time.now before ``update`, i.e. change

    @product = Core::Importer::Product.new(@product, product_params, options).update

    to

    @product = Core::Importer::Product.new(@product, product_params, options)
    @product.updated_at = Time.now
    @product.update
    
  2. Get rid of the necessary reload, i.e. change

      respond_with(@product.reload, :status => 200, :default_template => :show)

    to

      respond_with(@product, :status => 200, :default_template => :show)
    
@JDutil
Copy link
Member

JDutil commented Feb 11, 2015

Would you mind submitting a PR?

@edmondchui
Copy link
Contributor Author

  1. A better place to fix the issue is within Spree::Core::Importer::Product#update. We just touch the product before returning at the end of the update method. Touching the product has the benefit of calling the after_touch callbacks on Spree::Product.
  2. We cannot get rid of the @product.reload call at the end of Spree::Api::ProductController#update because the reload call actually reload all updated associations.

@JDutil
Copy link
Member

JDutil commented Feb 12, 2015

Could you submit another PR against this repo?

@nishant-cyro
Copy link
Contributor

@edmondchui - This one is working fine for me in master branch.

cc: @priyank-gupta @Mafi88 @damianlegawiec

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

No branches or pull requests

6 participants