Stop setting line items with quantity of zero #5275
Conversation
Right now, when empty values are passed as quantity in the API call that creates line items, the line item is populated with quantity 0. This is not the expected behavior, as it makes no sense to have a line item with 0 as quantity. Still, if someone wants to force the value to 0, this is still possible but 0 needs to be explicitly passed as quantity value. A spec has been added for this scenario as well.
github-actions
bot
added
changelog:solidus_api
RyanofWoods
force-pushed
the
ryanofwoods/stop-setting-line-items-with-quantity-of-zero
branch
from
f996063
to
ae87c41
Compare
Its spec was not testing single-level nested_attributes, which in the codebase occurs in the Api::LineItemsController [1]. [1] https://github.com/solidusio/solidus/blob/df68c0c748ded6a595d28c37a9460126c6d12ecb/api/app/controllers/spree/api/line_items_controller.rb#L56-L62
This method allows the deletion of LineItems where the quantity is set to 0 or less. This is possible because LineItem allows a quantity of 0 on save, but soon this will be disallowed, so we have to manually destroy these LineItems and remove them from the params before saving.
RyanofWoods
force-pushed
the
ryanofwoods/stop-setting-line-items-with-quantity-of-zero
branch
from
ae87c41
to
0d88daf
Compare
| else | ||
| params[:line_items_attributes].reject! do |_index, attributes| | ||
| line_items_to_destroy_ids << attributes[:id] if attributes[:quantity].to_i <= 0 | ||
| end |
There was a problem hiding this comment.
In which cases do we have this difference in passing the attributes?
There was a problem hiding this comment.
Do you mean whether it's nested or not? Like so:
# Case 1:
line_items_attributes: {
id: 1,
quantity: 1
}
# Case 2:
line_items_attributes: {
"01" => {
id: 1,
quantity: 1
},
"02" => {
id: 3,
quantity: 0
}
}I am supporting single nesting for Api::LineItemsController, though it seems that Rails supports these different levels by default - though I couldn't find any explicit documentation about it 🤔
There was a problem hiding this comment.
Hey @RyanofWoods, I'm not able to detect any code in the Api::LineItemsController or its specs that seem to support this way of passing attributes. Can you please help me adding more details about your discovery?
There was a problem hiding this comment.
Case 1 is needed for Spree::Api::LineItemsController:
solidus/api/app/controllers/spree/api/line_items_controller.rb
Lines 56 to 62 in 834347e
Case 2 was needed for how the OrderContents spec is setup.
Both these formats are supported by the nested_attributes in Rails.
Summary
In order to order to be able to update LineItem's validations to stop allowing a quantity of zero, we need to remove code that saves LineItems with a quantity of 0 (or less which gets normalized to 0).
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed: