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 2.0: Fix GET "/" #2358

Merged

Conversation

sauloperez
Copy link
Contributor

@sauloperez sauloperez commented Jun 5, 2018

What? Why?

This fixes the following error when accessing the app:

Started GET "/" for 127.0.0.1 at 2018-06-05 18:48:25 +0200
Creating scope :deleted. Overwriting existing method Spree::Variant.deleted.
Creating scope :available. Overwriting existing method Spree::PaymentMethod.available.
Processing by HomeController#index as HTML
   (0.2ms)  BEGIN
  Spree::Order Load (0.8ms)  SELECT "spree_orders".* FROM "spree_orders" WHERE "spree_orders"."number" = 'R175461258' LIMIT 1
   (14.1ms)  SELECT COUNT(*) FROM "spree_orders" WHERE (spree_orders.number LIKE 'R175461258%')
  Customer Load (7.8ms)  SELECT "customers".* FROM "customers" WHERE "customers"."enterprise_id" IS NULL AND "customers"."email" IS NULL LIMIT 1
   (21.7ms)  ROLLBACK
Completed 500 Internal Server Error in 171.3ms (ActiveRecord: 47.8ms)
** [Bugsnag] No API key configured, couldn't notify

NoMethodError (undefined method `shipping_method' for #<Spree::Order:0x007fc2d43ec800>):
  app/models/spree/order_decorator.rb:34:in `block (3 levels) in <top (required)>'
  app/helpers/order_cycles_helper.rb:3:in `current_order_cycle'
  app/controllers/application_controller.rb:134:in `check_order_cycle_expiry'


  Rendered /usr/local/opt/rbenv/versions/2.1.5/lib/ruby/gems/2.1.0/gems/actionpack-3.2.21/lib/action_dispatch/middleware/templates/rescues/_trace.erb (2.4ms)
  Rendered /usr/local/opt/rbenv/versions/2.1.5/lib/ruby/gems/2.1.0/gems/actionpack-3.2.21/lib/action_dispatch/middleware/templates/rescues/_request_and_response.erb (1.3ms)
  Rendered /usr/local/opt/rbenv/versions/2.1.5/lib/ruby/gems/2.1.0/gems/actionpack-3.2.21/lib/action_dispatch/middleware/templates/rescues/diagnostics.erb within rescues/layout (223.8ms)
  Rendered /usr/local/opt/rbenv/versions/2.1.5/lib/ruby/gems/2.1.0/gems/actionpack-3.2.21/lib/action_dispatch/middleware/templates/rescues/layout.erb (226.7ms)

We have changed our target Spree 2.0 version to 2.0.4 since we merged #2209 and said version doesn't change #current_order's signature yet. Reverting the commit related to this method fixed it. Now it is possible to access the app's root path again.

I found it while working on #2287.

Copy link
Contributor

@oeoeaio oeoeaio left a comment

Choose a reason for hiding this comment

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

Looks good.

We have changed our target Spree 2.0 version to 2.0.4 since we merged #2209 and said version doesn't change #current_order's signature yet.

So that means that we'll have to revert this revert at some point in the future right?

@sauloperez
Copy link
Contributor Author

sauloperez commented Jun 6, 2018

So that means that we'll have to revert this revert at some point in the future right?

Yep, but who knows when.

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.

Good one. I can't find the original pull request any more where I commented on using a wrapper for Spree's current order. I still think that a wrapper would be better than reverting the reverts.

@mkllnk mkllnk merged commit 0befc71 into openfoodfoundation:2-0-stable Jun 7, 2018
@sauloperez
Copy link
Contributor Author

sauloperez commented Jun 7, 2018

I still think that a wrapper would be better than reverting the reverts.

We'll keep that in mind. Remind us when the time comes, just in case :trollface:

@sauloperez sauloperez deleted the revert-current-order-argument branch June 7, 2018 07:14
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.

3 participants