Added LineItem name to unavailable flash #1697
Conversation
We were seeing issues where users wouldn't know which of their products in their cart were out of stock and causing the issue. Adding the product name to the flash message helps them out.
| @@ -150,7 +150,8 @@ def ensure_order_not_completed | |||
|
|
|||
| def ensure_sufficient_stock_lines | |||
| if @order.insufficient_stock_lines.present? | |||
| flash[:error] = Spree.t(:inventory_error_flash_for_insufficient_quantity) | |||
| out_of_stock_items = @order.insufficient_stock_lines.map { |l| "\"#{l.name}\"" }.join(', ') | |||
There was a problem hiding this comment.
No need to convert name into a string, it is already one.
Also I would prefer to join the names with Rails' to_sentence instead of join with comma.
There was a problem hiding this comment.
Will do. I'm actually wrapping each name in " to distinguish it from the rest of the message.
|
The test failures seem related. Could you please have a look? Thanks |
Fixed specs by allowing mock model LineItem to return a name. Also, changed the variable name in the translation file to match existing spec.
|
It turns out the specs were originally written to look for the product name in the translation, https://github.com/solidusio/solidus/blob/master/frontend/spec/controllers/spree/checkout_controller_spec.rb#L414. I have updated the code here to adhere to those old specs. |
| @@ -150,7 +150,8 @@ def ensure_order_not_completed | |||
|
|
|||
| def ensure_sufficient_stock_lines | |||
| if @order.insufficient_stock_lines.present? | |||
| flash[:error] = Spree.t(:inventory_error_flash_for_insufficient_quantity) | |||
| out_of_stock_items = @order.insufficient_stock_lines.map { |l| "'#{l.name}'" }.to_sentence | |||
There was a problem hiding this comment.
Please put the apostrophe into the locale file. Then users can decide if they want to have it in their notice or not.
There was a problem hiding this comment.
This could be rewritten as
@order.insufficient_stock_lines.collect(&:name).to_sentence
core/config/locales/en.yml
Outdated
| @@ -1282,7 +1282,7 @@ en: | |||
| inventory: Inventory | |||
| inventory_adjustment: Inventory Adjustment | |||
| inventory_canceled: Inventory canceled | |||
| inventory_error_flash_for_insufficient_quantity: An item in your cart has become unavailable. | |||
| inventory_error_flash_for_insufficient_quantity: "'#{names}' has become unavailable." | |||
There was a problem hiding this comment.
Seems like quoting the outside of the sentence will read quite odd if multiple items are out of stock.
'blue shirt and red shirt' has become unavailable.
There was a problem hiding this comment.
I was thinking about that today as well. Initially, I had it wrapping each product name but @tvdeyen suggested moving the apostrophe to the translation file. I've also been slightly bothered by the has vs have not being changed properly. Thoughts?
There was a problem hiding this comment.
Valid points. I think we could just remove the apostrophe from the translation. Stores that want to keep it, can easily update their locale file. And "has become" could be "became" and everything should be fine, no?
WDYT @jhawthorn @ericsaupe
There was a problem hiding this comment.
became is great! I just couldn't think of a way to rephrase that. Thanks!
|
@tvdeyen anything else needed on this? |
|
Thank you @ericsaupe |
We were seeing issues where users wouldn't know which of their
products in their cart were out of stock and causing the issue.
Adding the product name to the flash message helps them out.