Making simple coordinator insufficient stock to include a message #3577
Conversation
softr8
changed the title
| raise Spree::Order::InsufficientStock | ||
| item_names = leftover.variants.map(&:name).to_sentence | ||
| message = I18n.t('spree.inventory_error_flash_for_insufficient_quantity', names: item_names) | ||
| raise Spree::Order::InsufficientStock.new(message) |
There was a problem hiding this comment.
What about enhancing this spec
to check also for the message?
| raise Spree::Order::InsufficientStock | ||
| item_names = leftover.variants.map(&:name).to_sentence | ||
| message = I18n.t('spree.inventory_error_flash_for_insufficient_quantity', names: item_names) | ||
| raise Spree::Order::InsufficientStock.new(message) |
There was a problem hiding this comment.
I like the idea but I think we can take advantage of creating a custom exception that contains the unavailable items and let the consumer decide what to do with those.
What I have in mind is changing the exception definition to something like:
class InsufficientStock < StandardError
attr_reader :items
def initialize(message=nil, items: [])
super(message)
@items = items
end
endand push names in it with:
raise Spree::Order::InsufficientStock.new(items: leftovers)At that point, we can consume the exception data in different ways depending on the context, even using rescue_from since its handler methods accept an argument that will be the exception containing the items.
rescue_from Spree::Order::InsufficientStock, with: :something
def something(exception)
# exception.items is available here!
endWhat do you think?
There was a problem hiding this comment.
I like it! will refactor my code to take this path
softr8
force-pushed
the
improving/insufficient-stock-exception
branch
from
3678847 to
cecb3c1
Compare
|
What do you think about this new approach @kennyadsl |
core/app/models/spree/order.rb
Outdated
| class InsufficientStock < StandardError | ||
| attr_reader :items | ||
|
|
||
| def initialize(message = nil, items: []) |
There was a problem hiding this comment.
Should the default value of items: be an array or a hash?
There was a problem hiding this comment.
True! it returns a hash of items
softr8
force-pushed
the
improving/insufficient-stock-exception
branch
from
cecb3c1 to
ace5ace
Compare
|
Rerunning specs, it's a flaky one! |
It is hard to know what items were the ones that were unavailable, this change makes the exception to include all the items that were unable to be fullfilled so they can be rescued and then can be manipulated to build a better error message
softr8
force-pushed
the
improving/insufficient-stock-exception
branch
from
ace5ace to
75f5e98
Compare
It is hard to know what items were the ones that were
unavailable, this change includes item names that
were unable to be fullfilled, then this message
can be used in the frontend easily.
What do you think about this?
I saw that the frontend does some complex code to achieve kind of the same here
Checklist: