From c3d831587449d897c3070141415ba601175b6f2f Mon Sep 17 00:00:00 2001 From: Daniele Palombo Date: Fri, 26 Oct 2018 17:21:30 +0200 Subject: [PATCH 1/3] Rename Spree::Shipment#manifest to shipping_manifest_items `manifest` method returns Spree::ShippingManifest#items, this commit is going to rename it to `shipping_manifest_items`. I also refactored it to use the memoize shipping_manifest method. --- .../spree/api/shipments/_small.json.jbuilder | 2 +- .../views/spree/api/shipments/show.json.jbuilder | 2 +- .../orders/confirm/_shipment_manifest.html.erb | 2 +- .../features/admin/orders/order_details_spec.rb | 2 +- core/app/models/spree/shipment.rb | 15 +++++++++++---- core/spec/models/spree/shipment_spec.rb | 15 ++++++++++++--- .../app/views/spree/checkout/_delivery.html.erb | 2 +- 7 files changed, 28 insertions(+), 12 deletions(-) diff --git a/api/app/views/spree/api/shipments/_small.json.jbuilder b/api/app/views/spree/api/shipments/_small.json.jbuilder index 591affb8d52..57cc17bafed 100644 --- a/api/app/views/spree/api/shipments/_small.json.jbuilder +++ b/api/app/views/spree/api/shipments/_small.json.jbuilder @@ -23,7 +23,7 @@ json.cache! [I18n.locale, shipment] do json.(shipping_category, :id, :name) end end - json.manifest(shipment.manifest) do |manifest_item| + json.manifest(shipment.shipping_manifest_items) do |manifest_item| json.variant_id manifest_item.variant.id json.quantity(manifest_item.quantity) json.states(manifest_item.states) diff --git a/api/app/views/spree/api/shipments/show.json.jbuilder b/api/app/views/spree/api/shipments/show.json.jbuilder index a7a328c2228..2413ca26aa3 100644 --- a/api/app/views/spree/api/shipments/show.json.jbuilder +++ b/api/app/views/spree/api/shipments/show.json.jbuilder @@ -23,7 +23,7 @@ json.cache! [I18n.locale, @shipment] do json.(shipping_category, :id, :name) end end - json.manifest(@shipment.manifest) do |manifest_item| + json.manifest(@shipment.shipping_manifest_items) do |manifest_item| json.variant do json.partial!("spree/api/variants/small", variant: manifest_item.variant) end diff --git a/backend/app/views/spree/admin/orders/confirm/_shipment_manifest.html.erb b/backend/app/views/spree/admin/orders/confirm/_shipment_manifest.html.erb index c00856d23b9..bbb5595cc70 100644 --- a/backend/app/views/spree/admin/orders/confirm/_shipment_manifest.html.erb +++ b/backend/app/views/spree/admin/orders/confirm/_shipment_manifest.html.erb @@ -1,4 +1,4 @@ -<% shipment.manifest.each do |item| %> +<% shipment.shipping_manifest_items.each do |item| %> <%= render 'spree/admin/shared/image', diff --git a/backend/spec/features/admin/orders/order_details_spec.rb b/backend/spec/features/admin/orders/order_details_spec.rb index 92c55419724..5d0c3f36afe 100644 --- a/backend/spec/features/admin/orders/order_details_spec.rb +++ b/backend/spec/features/admin/orders/order_details_spec.rb @@ -332,7 +332,7 @@ visit spree.edit_admin_order_path(order) expect(order.shipments.count).to eq(1) - expect(order.shipments.first.manifest.count).to eq(2) + expect(order.shipments.first.shipping_manifest_items.count).to eq(2) within('tr', text: line_item.sku) { click_icon 'arrows-h' } complete_split_to(stock_location2) diff --git a/core/app/models/spree/shipment.rb b/core/app/models/spree/shipment.rb index 8e8a9caa230..7557c947321 100644 --- a/core/app/models/spree/shipment.rb +++ b/core/app/models/spree/shipment.rb @@ -106,11 +106,11 @@ def add_shipping_method(shipping_method, selected = false) deprecate :add_shipping_method, deprecator: Spree::Deprecation def after_cancel - manifest.each { |item| manifest_restock(item) } + shipping_manifest_items.each { |item| manifest_restock(item) } end def after_resume - manifest.each { |item| manifest_unstock(item) } + shipping_manifest_items.each { |item| manifest_unstock(item) } end def backordered? @@ -230,10 +230,13 @@ def selected_shipping_rate shipping_rates.detect(&:selected?) end - def manifest - @manifest ||= Spree::ShippingManifest.new(inventory_units: inventory_units).items + def shipping_manifest_items + @shipping_manifest_items ||= shipping_manifest.items end + alias manifest shipping_manifest_items + deprecate manifest: :shipping_manifest_items, deprecator: Spree::Deprecation + def selected_shipping_rate_id selected_shipping_rate.try(:id) end @@ -395,6 +398,10 @@ def address private + def shipping_manifest + @shipping_manifest ||= Spree::ShippingManifest.new(inventory_units: inventory_units) + end + def after_ship order.shipping.ship_shipment(self, suppress_mailer: suppress_mailer) end diff --git a/core/spec/models/spree/shipment_spec.rb b/core/spec/models/spree/shipment_spec.rb index 6057955ffd5..afd96f5d402 100644 --- a/core/spec/models/spree/shipment_spec.rb +++ b/core/spec/models/spree/shipment_spec.rb @@ -187,21 +187,30 @@ expect(shipment.total).to eq(8) end - context "manifest" do + describe "#manifest_items" do let(:order) { create(:order) } let(:variant) { create(:variant) } let!(:line_item) { order.contents.add variant } let!(:shipment) { order.create_proposed_shipments.first } it "returns variant expected" do - expect(shipment.manifest.first.variant).to eq variant + expect(shipment.shipping_manifest_items.first.variant).to eq variant end context "variant was removed" do before { variant.discard } it "still returns variant expected" do - expect(shipment.manifest.first.variant).to eq variant + expect(shipment.shipping_manifest_items.first.variant).to eq variant + end + end + end + + context "manifest" do + it "is deprecated" do + Spree::Deprecation.silence do + expect(Spree::Deprecation).to(receive(:warn)) + Spree::Shipment.new.manifest end end end diff --git a/frontend/app/views/spree/checkout/_delivery.html.erb b/frontend/app/views/spree/checkout/_delivery.html.erb index 741c81658ef..68c364cd6b2 100644 --- a/frontend/app/views/spree/checkout/_delivery.html.erb +++ b/frontend/app/views/spree/checkout/_delivery.html.erb @@ -24,7 +24,7 @@ <%= t('spree.price') %> - <% ship_form.object.manifest.each do |item| %> + <% ship_form.object.shipping_manifest_items.each do |item| %> <%= render 'spree/shared/image', From fdc4551c340bd71e7d1e642febc2fdb1fcd7fc77 Mon Sep 17 00:00:00 2001 From: Daniele Palombo Date: Fri, 26 Oct 2018 17:29:19 +0200 Subject: [PATCH 2/3] Rename Spree::Carton#manifest to shipping_manifest_items `manifest` method returns Spree::ShippingManifest#items, this commit is going to rename it to `shipping_manifest_items`. I also refactored it to use the memoize shipping_manifest method. --- core/app/mailers/spree/carton_mailer.rb | 2 +- core/app/models/spree/carton.rb | 13 +++++++++++-- .../spree/carton_mailer/shipped_email.html.erb | 2 +- .../spree/carton_mailer/shipped_email.text.erb | 2 +- core/spec/models/spree/carton_spec.rb | 11 ++++++++++- 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/core/app/mailers/spree/carton_mailer.rb b/core/app/mailers/spree/carton_mailer.rb index c5885d192b0..5946a1b9c1e 100644 --- a/core/app/mailers/spree/carton_mailer.rb +++ b/core/app/mailers/spree/carton_mailer.rb @@ -18,7 +18,7 @@ class CartonMailer < BaseMailer def shipped_email(options, _deprecated_options = {}) @order = options.fetch(:order) @carton = options.fetch(:carton) - @manifest = @carton.manifest_for_order(@order) + @shipping_manifest_for_order = @carton.manifest_for_order(@order) options = { resend: false }.merge(options) @store = @order.store subject = (options[:resend] ? "[#{t('spree.resend').upcase}] " : '') diff --git a/core/app/models/spree/carton.rb b/core/app/models/spree/carton.rb index 444d7080dff..06223020642 100644 --- a/core/app/models/spree/carton.rb +++ b/core/app/models/spree/carton.rb @@ -45,10 +45,13 @@ def display_shipped_at shipped_at.to_s(:rfc822) end - def manifest - @manifest ||= Spree::ShippingManifest.new(inventory_units: inventory_units).items + def shipping_manifest_items + @shipping_manifest_items ||= shipping_manifest.items end + alias manifest shipping_manifest_items + deprecate manifest: :shipping_manifest_items, deprecator: Spree::Deprecation + def manifest_for_order(order) Spree::ShippingManifest.new(inventory_units: inventory_units).for_order(order).items end @@ -56,4 +59,10 @@ def manifest_for_order(order) def any_exchanges? inventory_units.any?(&:original_return_item) end + + private + + def shipping_manifest + @shipping_manifest ||= Spree::ShippingManifest.new(inventory_units: inventory_units) + end end diff --git a/core/app/views/spree/carton_mailer/shipped_email.html.erb b/core/app/views/spree/carton_mailer/shipped_email.html.erb index bbbaab01460..1a27616f849 100644 --- a/core/app/views/spree/carton_mailer/shipped_email.html.erb +++ b/core/app/views/spree/carton_mailer/shipped_email.html.erb @@ -11,7 +11,7 @@ <%= t('spree.shipment_mailer.shipped_email.shipment_summary') %>

- <% @manifest.each do |item| %> + <% @shipping_manifest_for_order.each do |item| %> diff --git a/core/app/views/spree/carton_mailer/shipped_email.text.erb b/core/app/views/spree/carton_mailer/shipped_email.text.erb index e7aa8441db9..34a572a7930 100644 --- a/core/app/views/spree/carton_mailer/shipped_email.text.erb +++ b/core/app/views/spree/carton_mailer/shipped_email.text.erb @@ -5,7 +5,7 @@ ============================================================ <%= t('spree.shipment_mailer.shipped_email.shipment_summary') %> ============================================================ -<% @manifest.each do |item| %> +<% @shipping_manifest_for_order.each do |item| %> <%= item.variant.sku %> <%= item.variant.product.name %> <%= item.variant.options_text %> <% end %> ============================================================ diff --git a/core/spec/models/spree/carton_spec.rb b/core/spec/models/spree/carton_spec.rb index 572183e3a1e..a5bc75db2d8 100644 --- a/core/spec/models/spree/carton_spec.rb +++ b/core/spec/models/spree/carton_spec.rb @@ -83,7 +83,16 @@ end describe "#manifest" do - subject { carton.manifest } + it "is deprecated" do + Spree::Deprecation.silence do + expect(Spree::Deprecation).to(receive(:warn)) + Spree::Shipment.new.manifest + end + end + end + + describe "#shipping_manifest_items" do + subject { carton.shipping_manifest_items } let(:carton) { create(:carton, inventory_units: [first_order.inventory_units, second_order.inventory_units].flatten) } let(:first_order) { create(:order_ready_to_ship, line_items_count: 1) } From e43dd8727ae7075ea0f0343b99aca10048e82c23 Mon Sep 17 00:00:00 2001 From: Daniele Palombo Date: Fri, 26 Oct 2018 17:31:31 +0200 Subject: [PATCH 3/3] Rename `manifest_for_order` to `shipping_manifest_for_order` This commit is going to rename the method `manifest_for_order` to `shipping_manifest_for_order` I also refactored it to use the memoized `shipping_manifest` method. --- core/app/models/spree/carton.rb | 7 +++++-- core/spec/models/spree/carton_spec.rb | 9 +++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/core/app/models/spree/carton.rb b/core/app/models/spree/carton.rb index 06223020642..39893579432 100644 --- a/core/app/models/spree/carton.rb +++ b/core/app/models/spree/carton.rb @@ -52,10 +52,13 @@ def shipping_manifest_items alias manifest shipping_manifest_items deprecate manifest: :shipping_manifest_items, deprecator: Spree::Deprecation - def manifest_for_order(order) - Spree::ShippingManifest.new(inventory_units: inventory_units).for_order(order).items + def shipping_manifest_for_order(order) + shipping_manifest.for_order(order).items end + alias manifest_for_order shipping_manifest_for_order + deprecate manifest_for_order: :shipping_manifest_for_order, deprecator: Spree::Deprecation + def any_exchanges? inventory_units.any?(&:original_return_item) end diff --git a/core/spec/models/spree/carton_spec.rb b/core/spec/models/spree/carton_spec.rb index a5bc75db2d8..9ff1424961b 100644 --- a/core/spec/models/spree/carton_spec.rb +++ b/core/spec/models/spree/carton_spec.rb @@ -106,6 +106,15 @@ end describe "#manifest_for_order" do + it "is deprecated" do + Spree::Deprecation.silence do + expect(Spree::Deprecation).to(receive(:warn)) + Spree::Carton.new.manifest_for_order(create(:order)) + end + end + end + + describe "#shipping_manifest_for_order" do subject { carton.manifest_for_order(first_order) } let(:carton) { create(:carton, inventory_units: [first_order.inventory_units, second_order.inventory_units].flatten) }
<%= item.variant.sku %> <%= item.variant.product.name %>