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

respond_override for OrdersController#populate breaks OrdersController#update #2210

Closed
laurens opened this issue Nov 13, 2012 · 4 comments
Closed

Comments

@laurens
Copy link
Contributor

laurens commented Nov 13, 2012

I want to use a respond_override for displaying a flash message, after an item has been added to the cart.
So I added an orders_controller_decorator.rb with the following line:

respond_override :populate => { :html => { :success => lambda { redirect_to cart_path, :notice => 'item added' } } }

This works, however it breaks the update action. When I update the quantity of an item in the cart, the page gets redirected to the order url instead of going back to the cart.

I am on Spree 1.2, Ruby 1.9.2p290

Tested on a fresh spree install including spree_auth_devise

Server Log: https://gist.github.com/4069200

@radar
Copy link
Contributor

radar commented Jan 3, 2013

I can confirm this issue exists. Will look into it.

@radar
Copy link
Contributor

radar commented Aug 11, 2013

I can still verify this issue exists in 2-0-stable and master, but I cannot figure out the code that is required to make this work. Marking this issue as "stalled".

@leiyangyou
Copy link
Contributor

The problem seems to lie in the file core/lib/spree/core/controller_helpers/respond_with.rb

if defined_response = collector.response and 
!Spree::BaseController.spree_responders.keys.include?(self.class.to_s.to_sym)

only checks if a particular controller has a overriding responder, but does not check if the overriding responder defines a handler for the current action, this prevents the default handler of the action from running

it probably needs to be changed into this

if defined_response = collector.response and 
!(Spree::BaseController.spree_responders[self.class.to_s.to_sym].try(:[], action_name.to_sym)

but then again, this implementation does not check if the overriding action handler handles the requested format, and whether it provides a failure clause in the case of a failure etc, so there is probably a better way to do this.

RohanM added a commit to openfoodfoundation/spree that referenced this issue Aug 23, 2013
@JDutil JDutil modified the milestones: v2.0.9, v3.0.0 Aug 6, 2014
@JDutil JDutil modified the milestones: v2.5.0, v3.0.0 Oct 10, 2014
@JDutil JDutil modified the milestones: v2.5.0, v3.0.0 Nov 21, 2014
@spreebot spreebot removed the Verified label Jan 9, 2015
@JDutil
Copy link
Member

JDutil commented Jan 9, 2015

triage:verified

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