Update ShipmentMailer to use Cartons

Conflicts:
	core/app/mailers/spree/shipment_mailer.rb
	core/app/models/spree/shipment.rb
	core/spec/mailers/shipment_mailer_spec.rb

CHANGELOG.md

@@ -1,3 +1,15 @@
+*   Replace ShipmentMailer with CartonMailer
+
+    IMPORTANT: Appliction and extension code targeting ShipmentMailer needs to
+    be updated to target CartonMailer instead.
+
+    Issue https://github.com/bonobos/spree/pull/299
+
+*   Add Carton concept to Spree
+
+    Cartons represent containers of inventory units that have been shipped. See
+    carton.rb for details.
+
 *   Remove Promotion::Actions::CreateLineItems
 
     They were broken in a couple ways.

core/app/mailers/spree/carton_mailer.rb

@@ -0,0 +1,12 @@
+module Spree
+  class CartonMailer < BaseMailer
+    # Send an email to customers to notify that an individual carton has been
+    # shipped.
+    def shipped_email(carton_id, resend: false)
+      @carton = Spree::Carton.find(carton_id)
+      subject = (resend ? "[#{Spree.t(:resend).upcase}] " : '')
+      subject += "#{Spree::Config[:site_name]} #{Spree.t('shipment_mailer.shipped_email.subject')} ##{@carton.order_numbers.join(', ')}"
+      mail(to: @carton.order_emails, from: from_address, subject: subject)
+    end
+  end
+end

core/app/models/spree/carton.rb

@@ -1,4 +1,6 @@
 class Spree::Carton < ActiveRecord::Base
+  include Spree::ShippingManifest
+
   belongs_to :address, class_name: 'Spree::Address', inverse_of: :cartons
   belongs_to :stock_location, class_name: 'Spree::StockLocation', inverse_of: :cartons
   belongs_to :shipping_method, class_name: 'Spree::ShippingMethod', inverse_of: :cartons
@@ -25,4 +27,12 @@ def to_param
   def tracking_url
     @tracking_url ||= shipping_method.build_tracking_url(tracking)
   end
+
+  def order_numbers
+    orders.map(&:number)
+  end
+
+  def order_emails
+    orders.map(&:email).uniq
+  end
 end

core/app/models/spree/order_shipping.rb

@@ -68,14 +68,10 @@ def ship(inventory_units:, stock_location:, address:, shipping_method:,
         # having Shipment#ship! call OrderShipping#ship_shipment. We only really
         # need this `update_columns` for the specs, until we make that change.
         shipment.update_columns(state: 'shipped', shipped_at: Time.now)
-
-        if stock_location.fulfillable? # e.g. digital gift cards that aren't actually shipped
-          # TODO: send inventory units instead of shipment
-          Spree::ShipmentMailer.shipped_email(shipment).deliver
-        end
       end
     end
 
+    send_shipment_email(carton) if stock_location.fulfillable? # e.g. digital gift cards that aren't actually shipped
     fulfill_order_stock_locations(stock_location)
     update_order_state
 
@@ -95,4 +91,8 @@ def update_order_state
       updated_at: Time.now,
     )
   end
+
+  def send_shipment_email(carton)
+    Spree::CartonMailer.shipped_email(carton.id).deliver
+  end
 end

core/app/models/spree/shipment.rb

@@ -2,8 +2,11 @@
 
 module Spree
   class Shipment < Spree::Base
-    belongs_to :address, class_name: 'Spree::Address', inverse_of: :shipments
     belongs_to :order, class_name: 'Spree::Order', touch: true, inverse_of: :shipments
+    include Spree::ShippingManifest
+
+    belongs_to :order, class_name: 'Spree::Order', touch: true, inverse_of: :shipments
+    belongs_to :address, class_name: 'Spree::Address', inverse_of: :shipments
     belongs_to :stock_location, class_name: 'Spree::StockLocation'
 
     has_many :adjustments, as: :adjustable, dependent: :delete_all
@@ -158,24 +161,6 @@ def line_items
       inventory_units.includes(:line_item).map(&:line_item).uniq
     end
 
-    ManifestItem = Struct.new(:line_item, :variant, :quantity, :states)
-
-    def manifest
-      # Grouping by the ID means that we don't have to call out to the association accessor
-      # This makes the grouping by faster because it results in less SQL cache hits.
-      inventory_units.group_by(&:variant_id).map do |variant_id, units|
-        units.group_by(&:line_item_id).map do |line_item_id, units|
-
-          states = {}
-          units.group_by(&:state).each { |state, iu| states[state] = iu.count }
-
-          line_item = units.first.line_item
-          variant = units.first.variant
-          ManifestItem.new(line_item, variant, units.length, states)
-        end
-      end.flatten
-    end
-
     def process_order_payments
       pending_payments =  order.pending_payments
                             .sort_by(&:uncaptured_amount).reverse
@@ -202,6 +187,10 @@ def process_order_payments
       end
     end
 
+    def line_items
+      inventory_units.includes(:line_item).map(&:line_item).uniq
+    end
+
     def ready_or_pending?
       self.ready? || self.pending?
     end

core/app/models/spree/shipping_manifest.rb

@@ -0,0 +1,19 @@
+module Spree::ShippingManifest
+  ManifestItem = Struct.new(:line_item, :variant, :quantity, :states)
+
+  def manifest
+    # Grouping by the ID means that we don't have to call out to the association accessor
+    # This makes the grouping by faster because it results in less SQL cache hits.
+    inventory_units.group_by(&:variant_id).map do |variant_id, units|
+      units.group_by(&:line_item_id).map do |line_item_id, units|
+
+        states = {}
+        units.group_by(&:state).each { |state, iu| states[state] = iu.count }
+
+        line_item = units.first.line_item
+        variant = units.first.variant
+        ManifestItem.new(line_item, variant, units.length, states)
+      end
+    end.flatten
+  end
+end

core/app/views/spree/shipment_mailer/shipped_email.text.erb → core/app/views/spree/carton_mailer/shipped_email.text.erb

@@ -5,12 +5,12 @@
 ============================================================
 <%= Spree.t('shipment_mailer.shipped_email.shipment_summary') %>
 ============================================================
-<% @shipment.manifest.each do |item| %>
+<% @carton.manifest.each do |item| %>
   <%= item.variant.sku %> <%= item.variant.product.name %> <%= item.variant.options_text %>
 <% end %>
 ============================================================
 
-<%= Spree.t('shipment_mailer.shipped_email.track_information', :tracking => @shipment.tracking)     if @shipment.tracking %>
-<%= Spree.t('shipment_mailer.shipped_email.track_link',        :url      => @shipment.tracking_url) if @shipment.tracking_url %>
+<%= Spree.t('shipment_mailer.shipped_email.track_information', :tracking => @carton.tracking)     if @carton.tracking %>
+<%= Spree.t('shipment_mailer.shipped_email.track_link',        :url      => @carton.tracking_url) if @carton.tracking_url %>
 
 <%= Spree.t('shipment_mailer.shipped_email.thanks') %>

core/spec/mailers/carton_mailer_spec.rb

@@ -0,0 +1,50 @@
+require 'spec_helper'
+require 'email_spec'
+
+describe Spree::CartonMailer do
+  include EmailSpec::Helpers
+  include EmailSpec::Matchers
+
+  let(:carton) { create(:carton) }
+
+  context ":from not set explicitly" do
+    it "falls back to spree config" do
+      message = Spree::CartonMailer.shipped_email(carton.id)
+      message.from.should == [Spree::Config[:mails_from]]
+    end
+  end
+
+  # Regression test for #2196
+  it "doesn't include out of stock in the email body" do
+    shipment_email = Spree::CartonMailer.shipped_email(carton.id)
+    shipment_email.body.should_not include(%Q{Out of Stock})
+  end
+
+  context "with resend option" do
+    subject do
+      Spree::CartonMailer.shipped_email(carton.id, resend: true).subject
+    end
+    it { is_expected.to match /^\[RESEND\] / }
+  end
+
+  context "emails must be translatable" do
+    context "shipped_email" do
+      context "pt-BR locale" do
+        before do
+          pt_br_shipped_email = { :spree => { :shipment_mailer => { :shipped_email => { :dear_customer => 'Caro Cliente,' } } } }
+          I18n.backend.store_translations :'pt-BR', pt_br_shipped_email
+          I18n.locale = :'pt-BR'
+        end
+
+        after do
+          I18n.locale = I18n.default_locale
+        end
+
+        specify do
+          shipped_email = Spree::CartonMailer.shipped_email(carton.id)
+          shipped_email.body.should include("Caro Cliente,")
+        end
+      end
+    end
+  end
+end

core/spec/models/spree/carton_spec.rb

@@ -42,4 +42,31 @@
     it { is_expected.to eq carton.number }
   end
 
+  describe "#order_numbers" do
+    subject { carton.order_numbers }
+    let(:order) { carton.orders.first }
+
+    it "returns a list of the order numbers it is associated to" do
+      expect(subject).to eq [order.number]
+    end
+  end
+
+  describe "#order_emails" do
+    subject { carton.order_emails }
+
+    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) }
+    let(:second_order) { create(:order_ready_to_ship, line_items_count: 1) }
+    let(:email) { 'something@something.com' }
+
+    before do
+      first_order.update_attributes!(email: email)
+      second_order.update_attributes!(email: email)
+    end
+
+    it "returns a unique list of the order emails it is associated to" do
+      expect(subject).to eq [email]
+    end
+  end
+
 end