From 88301301d76cf4dbfc514a58c301f4e54cbce22f Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Mon, 18 Sep 2017 10:18:20 -0700 Subject: [PATCH 1/9] Remove explicit DOM id from spec Capybara can find the element --- backend/spec/features/admin/orders/order_details_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/spec/features/admin/orders/order_details_spec.rb b/backend/spec/features/admin/orders/order_details_spec.rb index 3df4abcc192..fdc9f2695d5 100644 --- a/backend/spec/features/admin/orders/order_details_spec.rb +++ b/backend/spec/features/admin/orders/order_details_spec.rb @@ -537,7 +537,7 @@ within("tr", text: "Shipping Method") do click_icon :edit end - select "UPS Ground $100.00", from: "selected_shipping_rate_id" + select "UPS Ground $100.00" click_icon :check expect(page).not_to have_css('#selected_shipping_rate_id') From dd768e317b02a87891dc987b282266d21f9df0fa Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 14 Sep 2017 00:14:53 -0700 Subject: [PATCH 2/9] Add estimated_rates endpoint --- .../spree/api/shipments_controller.rb | 8 ++++- .../shipments/estimated_rates.json.jbuilder | 3 ++ api/config/routes.rb | 2 ++ .../spree/api/shipments_controller_spec.rb | 29 +++++++++++++++++++ 4 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 api/app/views/spree/api/shipments/estimated_rates.json.jbuilder diff --git a/api/app/controllers/spree/api/shipments_controller.rb b/api/app/controllers/spree/api/shipments_controller.rb index 835c6ec0989..fc294c7b892 100644 --- a/api/app/controllers/spree/api/shipments_controller.rb +++ b/api/app/controllers/spree/api/shipments_controller.rb @@ -2,7 +2,7 @@ module Spree module Api class ShipmentsController < Spree::Api::BaseController before_action :find_order_on_create, only: :create - before_action :find_shipment, only: [:update, :ship, :ready, :add, :remove] + before_action :find_shipment, only: [:update, :ship, :ready, :add, :remove, :estimated_rates] before_action :load_transfer_params, only: [:transfer_to_location, :transfer_to_shipment] around_action :lock_order, except: [:mine] before_action :update_shipment, only: [:ship, :ready, :add, :remove] @@ -22,6 +22,12 @@ def mine end end + def estimated_rates + authorize! :update, @shipment + estimator = Spree::Config.stock.estimator_class.new + @shipping_rates = estimator.shipping_rates(@shipment.to_package) + end + def create authorize! :create, Shipment quantity = params[:quantity].to_i diff --git a/api/app/views/spree/api/shipments/estimated_rates.json.jbuilder b/api/app/views/spree/api/shipments/estimated_rates.json.jbuilder new file mode 100644 index 00000000000..87a064617fe --- /dev/null +++ b/api/app/views/spree/api/shipments/estimated_rates.json.jbuilder @@ -0,0 +1,3 @@ +json.shipping_rates @shipping_rates do |shipping_rate| + json.partial!("spree/api/shipping_rates/shipping_rate", shipping_rate: shipping_rate) +end diff --git a/api/config/routes.rb b/api/config/routes.rb index 3afb92da049..d32b4fdb826 100644 --- a/api/config/routes.rb +++ b/api/config/routes.rb @@ -82,6 +82,8 @@ end member do + get :estimated_rates + put :ready put :ship put :add diff --git a/api/spec/requests/spree/api/shipments_controller_spec.rb b/api/spec/requests/spree/api/shipments_controller_spec.rb index 4a36a6070d4..f1d2abcd14f 100644 --- a/api/spec/requests/spree/api/shipments_controller_spec.rb +++ b/api/spec/requests/spree/api/shipments_controller_spec.rb @@ -237,6 +237,35 @@ end end + describe "#estimated_rates" do + let(:shipping_method) { shipment.shipping_method } + + sign_in_as_admin! + + subject do + get spree.estimated_rates_api_shipment_path(shipment) + end + + it "returns the correct response" do + subject + + expect(response).to be_success + expect(json_response).to eq( + "shipping_rates"=> [ + { + "id" => nil, + "name" => shipping_method.name, + "cost" => "100.0", + "selected" => true, + "shipping_method_id" => shipping_method.id, + "shipping_method_code" => shipping_method.code, + "display_cost" => "$100.00" + } + ] + ) + end + end + describe "#ship" do let(:shipment) { create(:order_ready_to_ship).shipments.first } From 756b4d546c436f835c85d1fea50ad575189eb6ba Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 14 Sep 2017 00:24:25 -0700 Subject: [PATCH 3/9] Add Shipment#select_shipping_method --- core/app/models/spree/shipment.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/core/app/models/spree/shipment.rb b/core/app/models/spree/shipment.rb index aaaf70514b7..dd01d788237 100644 --- a/core/app/models/spree/shipment.rb +++ b/core/app/models/spree/shipment.rb @@ -219,6 +219,14 @@ def refresh_rates shipping_rates end + def select_shipping_method(shipping_method) + estimator = Spree::Config.stock.estimator_class.new + rates = estimator.shipping_rates(to_package, false) + rate = rates.detect { |r| r.shipping_method_id == shipping_method.id } + rate.selected = true + self.shipping_rates = [rate] + end + def selected_shipping_rate shipping_rates.detect(&:selected?) end From 3db4c425ccdf0b3e2ecd42965acf9cbad26febf4 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 14 Sep 2017 00:25:49 -0700 Subject: [PATCH 4/9] Select also backend rates --- api/app/controllers/spree/api/shipments_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/app/controllers/spree/api/shipments_controller.rb b/api/app/controllers/spree/api/shipments_controller.rb index fc294c7b892..2bf3fb76c71 100644 --- a/api/app/controllers/spree/api/shipments_controller.rb +++ b/api/app/controllers/spree/api/shipments_controller.rb @@ -25,7 +25,7 @@ def mine def estimated_rates authorize! :update, @shipment estimator = Spree::Config.stock.estimator_class.new - @shipping_rates = estimator.shipping_rates(@shipment.to_package) + @shipping_rates = estimator.shipping_rates(@shipment.to_package, false) end def create From e5cbbfdb15d0edaecfb1305cf8a79a53859090a8 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 14 Sep 2017 13:45:35 -0700 Subject: [PATCH 5/9] More work on estimated rates --- .../shipments/estimated_rates.json.jbuilder | 3 +- .../spree/api/shipments_controller_spec.rb | 43 ++++++++++++------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/api/app/views/spree/api/shipments/estimated_rates.json.jbuilder b/api/app/views/spree/api/shipments/estimated_rates.json.jbuilder index 87a064617fe..74eecd98aaf 100644 --- a/api/app/views/spree/api/shipments/estimated_rates.json.jbuilder +++ b/api/app/views/spree/api/shipments/estimated_rates.json.jbuilder @@ -1,3 +1,4 @@ json.shipping_rates @shipping_rates do |shipping_rate| - json.partial!("spree/api/shipping_rates/shipping_rate", shipping_rate: shipping_rate) + json.(shipping_rate, :name, :cost, :shipping_method_id, :shipping_method_code) + json.display_cost(shipping_rate.display_cost.to_s) end diff --git a/api/spec/requests/spree/api/shipments_controller_spec.rb b/api/spec/requests/spree/api/shipments_controller_spec.rb index f1d2abcd14f..ea870c82f28 100644 --- a/api/spec/requests/spree/api/shipments_controller_spec.rb +++ b/api/spec/requests/spree/api/shipments_controller_spec.rb @@ -238,7 +238,8 @@ end describe "#estimated_rates" do - let(:shipping_method) { shipment.shipping_method } + let!(:user_shipping_method) { shipment.shipping_method } + let!(:admin_shipping_method) { create(:shipping_method, available_to_users: false, name: "Secret") } sign_in_as_admin! @@ -246,22 +247,34 @@ get spree.estimated_rates_api_shipment_path(shipment) end - it "returns the correct response" do + it "returns success" do subject - expect(response).to be_success - expect(json_response).to eq( - "shipping_rates"=> [ - { - "id" => nil, - "name" => shipping_method.name, - "cost" => "100.0", - "selected" => true, - "shipping_method_id" => shipping_method.id, - "shipping_method_code" => shipping_method.code, - "display_cost" => "$100.00" - } - ] + end + + it "returns rates available to user" do + subject + expect(json_response['shipping_rates']).to include( + { + "name" => user_shipping_method.name, + "cost" => "100.0", + "shipping_method_id" => user_shipping_method.id, + "shipping_method_code" => user_shipping_method.code, + "display_cost" => "$100.00" + } + ) + end + + it "returns rates available to admin" do + subject + expect(json_response['shipping_rates']).to include( + { + "name" => admin_shipping_method.name, + "cost" => "10.0", + "shipping_method_id" => admin_shipping_method.id, + "shipping_method_code" => admin_shipping_method.code, + "display_cost" => "$10.00" + } ) end end From f91e04d1e86dca5344127c29e866ca6d2ab69aef Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Thu, 14 Sep 2017 15:31:20 -0700 Subject: [PATCH 6/9] Don't lock order for estimated_rates --- api/app/controllers/spree/api/shipments_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/app/controllers/spree/api/shipments_controller.rb b/api/app/controllers/spree/api/shipments_controller.rb index 2bf3fb76c71..312d9b495e1 100644 --- a/api/app/controllers/spree/api/shipments_controller.rb +++ b/api/app/controllers/spree/api/shipments_controller.rb @@ -4,7 +4,7 @@ class ShipmentsController < Spree::Api::BaseController before_action :find_order_on_create, only: :create before_action :find_shipment, only: [:update, :ship, :ready, :add, :remove, :estimated_rates] before_action :load_transfer_params, only: [:transfer_to_location, :transfer_to_shipment] - around_action :lock_order, except: [:mine] + around_action :lock_order, except: [:mine, :estimated_rates] before_action :update_shipment, only: [:ship, :ready, :add, :remove] def mine From b723acccf3fd3a09292c3771320af38582de97b4 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Mon, 18 Sep 2017 09:59:06 -0700 Subject: [PATCH 7/9] Don't lock order on orders_controller show --- api/app/controllers/spree/api/orders_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/app/controllers/spree/api/orders_controller.rb b/api/app/controllers/spree/api/orders_controller.rb index 2134ed3ae9a..b52f9491444 100644 --- a/api/app/controllers/spree/api/orders_controller.rb +++ b/api/app/controllers/spree/api/orders_controller.rb @@ -10,7 +10,7 @@ class OrdersController < Spree::Api::BaseController skip_before_action :authenticate_user, only: :apply_coupon_code before_action :find_order, except: [:create, :mine, :current, :index] - around_action :lock_order, except: [:create, :mine, :current, :index] + around_action :lock_order, except: [:create, :mine, :current, :index, :show] # Dynamically defines our stores checkout steps to ensure we check authorization on each step. Spree::Order.checkout_steps.keys.each do |step| From 14ca07dbae257e8362428aa396ca5ef644bd70e3 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Mon, 8 Jan 2018 11:50:36 -0800 Subject: [PATCH 8/9] Use API endpoints for selecting shipping method --- .../spree/api/shipments_controller.rb | 10 +++++++++- api/config/routes.rb | 1 + .../spree/backend/models/shipment.js | 17 +++++++++++++++++ .../templates/orders/shipping_method.hbs | 8 ++++++-- .../backend/views/order/shipping_method.js | 19 +++++++++++-------- 5 files changed, 44 insertions(+), 11 deletions(-) diff --git a/api/app/controllers/spree/api/shipments_controller.rb b/api/app/controllers/spree/api/shipments_controller.rb index 312d9b495e1..5cdaaaf0609 100644 --- a/api/app/controllers/spree/api/shipments_controller.rb +++ b/api/app/controllers/spree/api/shipments_controller.rb @@ -2,7 +2,7 @@ module Spree module Api class ShipmentsController < Spree::Api::BaseController before_action :find_order_on_create, only: :create - before_action :find_shipment, only: [:update, :ship, :ready, :add, :remove, :estimated_rates] + before_action :find_shipment, only: [:update, :ship, :ready, :add, :remove, :estimated_rates, :select_shipping_method] before_action :load_transfer_params, only: [:transfer_to_location, :transfer_to_shipment] around_action :lock_order, except: [:mine, :estimated_rates] before_action :update_shipment, only: [:ship, :ready, :add, :remove] @@ -28,6 +28,14 @@ def estimated_rates @shipping_rates = estimator.shipping_rates(@shipment.to_package, false) end + def select_shipping_method + authorize! :update, @shipment + shipping_method = Spree::ShippingMethod.find(params.require(:shipping_method_id)) + @shipment.select_shipping_method(shipping_method) + @order.recalculate + respond_with(@shipment, default_template: :show) + end + def create authorize! :create, Shipment quantity = params[:quantity].to_i diff --git a/api/config/routes.rb b/api/config/routes.rb index d32b4fdb826..a92beae8351 100644 --- a/api/config/routes.rb +++ b/api/config/routes.rb @@ -83,6 +83,7 @@ member do get :estimated_rates + put :select_shipping_method put :ready put :ship diff --git a/backend/app/assets/javascripts/spree/backend/models/shipment.js b/backend/app/assets/javascripts/spree/backend/models/shipment.js index 0e8a2567f12..d6279c6b644 100644 --- a/backend/app/assets/javascripts/spree/backend/models/shipment.js +++ b/backend/app/assets/javascripts/spree/backend/models/shipment.js @@ -6,5 +6,22 @@ Spree.Models.Shipment = Backbone.Model.extend({ relations: { "selected_shipping_rate": Backbone.Model, "shipping_rates": Backbone.Collection, + }, + + estimatedRates: function() { + var ratesCollection = Backbone.Collection.extend({ + parse: function(resp){ return resp.shipping_rates } + }); + var rates = new ratesCollection(); + rates.fetch({ url: this.url() + "/estimated_rates" }) + return rates; + }, + + selectShippingMethodId: function(shippingMethodId, options) { + this.sync("update", this, _.extend({ + url: this.url() + "/select_shipping_method", + contentType: 'application/json', + data: JSON.stringify({ shipping_method_id: shippingMethodId }) + }, options)); } }) diff --git a/backend/app/assets/javascripts/spree/backend/templates/orders/shipping_method.hbs b/backend/app/assets/javascripts/spree/backend/templates/orders/shipping_method.hbs index 255a3c42cf3..4aa6cbd156b 100644 --- a/backend/app/assets/javascripts/spree/backend/templates/orders/shipping_method.hbs +++ b/backend/app/assets/javascripts/spree/backend/templates/orders/shipping_method.hbs @@ -3,13 +3,15 @@ {{#if editing}} +{{#if shipping_rates}}
- {{#each shipping_rates}} - + {{/each}}
+{{/if}} {{else}} @@ -22,7 +24,9 @@ {{#if editing}} + {{#if shipping_rates}} + {{/if}} {{else}} diff --git a/backend/app/assets/javascripts/spree/backend/views/order/shipping_method.js b/backend/app/assets/javascripts/spree/backend/views/order/shipping_method.js index ad44644b85f..cd6e72796a3 100644 --- a/backend/app/assets/javascripts/spree/backend/views/order/shipping_method.js +++ b/backend/app/assets/javascripts/spree/backend/views/order/shipping_method.js @@ -10,32 +10,35 @@ Spree.Views.Order.ShippingMethod = Backbone.View.extend({ }, initialize: function(options) { - this.shippingRateId = this.model.get('selected_shipping_rate').get('id') + this.shippingMethodId = this.model.get('selected_shipping_rate').get('shipping_method_id'); + this.shippingRates = new Backbone.Collection(); this.render(); }, onEdit: function(event) { this.editing = true; + this.shippingRates = this.model.estimatedRates(); + this.listenTo(this.shippingRates, "sync", this.render); this.render(); }, onSave: function(event) { this.editing = false; - this.model.save({ - selected_shipping_rate_id: this.$('select').val() - }, { - patch: true, + this.shippingMethodId = this.$('select').val(); + this.shippingRates = new Backbone.Collection(); + this.model.selectShippingMethodId(this.shippingMethodId, { success: function() { - // FIXME: should update page without reloading window.location.reload(); } }); + this.render(); return false; }, onCancel: function(event) { this.editing = false; + this.shippingRates = new Backbone.Collection(); this.render(); }, @@ -45,10 +48,10 @@ Spree.Views.Order.ShippingMethod = Backbone.View.extend({ order: this.model.collection.parent.toJSON(), shipment: this.model.toJSON(), selected_shipping_rate: this.model.get("selected_shipping_rate").toJSON(), - shipping_rates: this.model.get("shipping_rates").toJSON() + shipping_rates: this.shippingRates.toJSON() }); this.$el.html(html); - this.$('select').val(this.shippingRateId); + this.$('select').val(this.shippingMethodId); } }) From 3239735816a72aaf45426b64b7d6537762769499 Mon Sep 17 00:00:00 2001 From: John Hawthorn Date: Wed, 10 Jan 2018 12:08:57 -0800 Subject: [PATCH 9/9] Add spec for selecting admin only shipping method --- .../admin/orders/order_details_spec.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/backend/spec/features/admin/orders/order_details_spec.rb b/backend/spec/features/admin/orders/order_details_spec.rb index fdc9f2695d5..4ddaf8d0266 100644 --- a/backend/spec/features/admin/orders/order_details_spec.rb +++ b/backend/spec/features/admin/orders/order_details_spec.rb @@ -119,6 +119,24 @@ expect(page).to have_content("UPS Ground") end + it "can use admin-only shipping methods" do + create(:shipping_method, name: "Admin Free Shipping", cost: 0, available_to_users: false) + + visit spree.edit_admin_order_path(order) + + screenshot_and_open_image + + within("tr", text: "Shipping Method") do + click_icon :edit + select "Admin Free Shipping $0.00" + click_icon :check + end + + expect(page).not_to have_css('#selected_shipping_rate_id') + expect(page).to have_no_content("UPS Ground") + expect(page).to have_content("Admin Free Shipping") + end + it "will show the variant sku" do visit spree.edit_admin_order_path(order) sku = order.line_items.first.variant.sku