gistfile1.rb

# The fundamantal design opinion is that a Stock Return has many Returned Items 
# Each Returned Item effectively has one of 3 actions associated with it return, exchange OR replace
# Any of these action can involve a refund, although the most common is that a return involves a refund
# You may wish to refund a replacement Or exchange to maintain customer relations
#
# Once all Returned Items and their actions have been set then you can commence to attempt to process the Stock Return
# Processing the Stock Return will automate the Order Adjustments, Addiitonal Shipments and any Refunds / Payments
# It should be exstensible so that developers can easily implement their own behaviour here
#
# It could be that a developer can extend this to Create New Orders instead of Additional Shipments under certain scenarios:
# When the total of the exchanged items exceed the balance on the Order, it might be preferable to require a new payment, hence new Order

class StockReturn    
  # return_cost
  # handling_cost
  # timestamps
  has_many :returned_items
  has_many :exchange_items
  
  before_transistion_to :complete, :handle_return
  
  # Validate processable?
  def processable?
    Spree::StockReturnHandler.factory(self).processable?
  end

  def handle_return
     Spree::StockReturnHandler.factory(self).process!
  end
end

# A Returned Item has 3 actions: return, replace or exchange
# You can refund (using refund_amount not null and > 0) with ANY of the actions above
#
# Attributes:
# - reason: provided by customer (wrong_size, not_suitable, not_as_described, arrived_late, other, etc)
# - reason_other : string
# - refund_amount: allow null, null is unknown and therefore unprocessable, must be set to 0 if no refund provided
# NOTE: No qty for line item, each one has a separate record
#
# Potentially:
# - condition: provided by receiver (faulty, damaged, good)
# - restock: boolean (null is unknown and therefore unprocessable)
class ReturnedItem
  enum action: [:return, :replace, :exchange]
  enum reason: REASON_CODES

  belongs_to :stock_return
  belongs_to :line_item
	
  # If certain Reason Codes provided then we can auto set refund amount
  before_create :auto_set_refund_amount
  
  def refund?
    refund_amount.present? && refund_amount > 0
  end
end

class ExchangeItem
  # price (defaults to line_item current price but if set here is overriden)
  belongs_to :stock_return
  belongs_to :line_item
end

class StockReturnHandler
  def initialize(stock_return)
    @stock_return
  end

  def processable?
    # These methods add errors to the Stock Return object
    refund_due_within_processable_boundary?
    exchange_items_shipable?
    replacement_items_shipable?
    @stock_return.valid?
  end
	
  def perform
     return false unless processable?
	  
     adjust_order_for_return_cost
     adjust_order_for_handling_fee
     create_shipments_for_exchanged_and_replacement_items
     process_balance #(refund or charge additional)
     
     true
  end
end

Some feedback / questions:

To initialize: #factory should be #new.

class StockReturn      
  # Validate processable?
  def processable?
    Spree::StockReturnHandler.new(self).processable?
  end

  def handle_return
     Spree::StockReturnHandler.new(self).perform!
  end
end

To comply with sidekiq & resque: #process! should be #perform!

class StockReturnHandler
  def perform!
    # do stuff
  end
end

class ReturnedItem should be ReturnedInventoryUnit to go along with the InventoryUnit model that describes items shipped.

class ExchangeItem should be ExchangedInventoryUnit

No need for class ReturnItemReasonCode it can just be an enum column on ReturnedInventoryUnit rather than creating a whole class. http://edgeapi.rubyonrails.org/classes/ActiveRecord/Enum.html

ReturnedItem return and exchange booleans would always be opposite of each other correct? It's either a return or an exchange? This may make more sense as a enum or just a boolean clearly saying which it is. Then replace is whether or not you want to replace the item? I think maybe some more details on the difference between return, exchange, replace flags would be helpful or if it is just selecting one of those flags and not the others a single enum columns makes more sense.

The reason why I wanted to call a factory is that a developer can override this to return an instance of whatever handler they want. I guess we could offload this to an initializer and update the class in the handle_return method.

I'm not so bothered about a Code Review as I'm really very rusty, but more about you feedback on the concept behind the implementation.

I'm unclear on this example:
It could be that a developer can extend this to Create New orders instead of Shipments under certain scenarios for example when the total of the exchanged items exceed the balance on the Order.

If the exchanged items exceed the balance on the order you should just need to create a new payment record not an entire order.

Enum probably works best as they are discreet:

Return means the stock will go back into circulation unless faulty and normally the customer will receive a refund (unless damaged and their fault)

Replace means that we must ship a replacement (handled in the processing, only if in stock, else we would issue a refund and communicate this to the Customer

Exchange means that we must expect at least one ExchangedItem that is in stock and shippable, else we would issue a refund and communicate this to the Customer

The overall concept is good just want to also point out code things so the developers are aware.

So just would like some clarity on the balance things I just previously mentioned?

Also rereading the text I got my answer to the 3 return, exchange OR replace flags. To be clear:
Return: The customer is sending you back the item to be refunded.
Exchange: The customer is sending you back the item to be sent a new one.
Replace: You are sending the customer a new one without requiring their return of the original.

Not quite. All of the above require a Stock Return so be initiated so we have to have received an item by return:

Replace: The item was likely to be faulty or damaged on delivery, send a replacement
Return: The item has been returned and the customer is requested a refund (TBC if not damaged etc etc)
Exchange: The item has been returned and the customer would like a different item(s) instead

This doesn't deal with Replacing Shipments that got lost in transi, Stock Returns are only concerned with returned items.

In this case the best option would be add an undelivered status to the shipment but this falls outside the scope of Returns as far as I cam concerned.

Not too sure I follow with the renaming to InventoryUnits?

Personally I think I prefer the names ReturnedItem and ExchangeItem as they map more closely to LineItem. Once a StockReturn is successfully processed then I guess it will make any StockMovements using the Returned and Exchange Items?

In our implementation, Spree will not make any stock movements because our WMS is master for stock and will send updates to Spree once these goods are checked in to a Pickable location in the WH.

You said you would like the ReturnedItems to not have a quantity, and be a single record per unit quantity. That is what InventoryUnits are used for in terms of mapping LineItems to a Shipment. Following that naming convention it would make sense it using the naming as ReturnedInventoryUnit and ExchangedInventoryUnit as the way to track that your shipped InventoryUnit came back.

https://github.com/spree/spree/blob/master/core/app/models/spree/inventory_unit.rb