Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List order customer returns only once #4196

Conversation

spaghetticode
Copy link
Member

Customer returns are associated to orders via return items. This means that, if an order has multiple line items and a single customer return for all of them, then the customer return will be listed twice. Listing customer returns multiple times adds noise without any significant benefit, and can be misleading when calling order.customer_returns.size and similars.

Given this is an ActiveRecord feature, it doesn't make much sense to add a spec in the codebase for covering it, considering also that the spec would require the creation of many different records, but for the curious ones it can be verified with:

order = create(:order_with_line_items, line_items_count: 2)

customer_return = build(:customer_return)

return_items = (0..1).each do |n|
  customer_return.return_items << create(:return_item, inventory_unit: order.inventory_units[n])
end

customer_return.save!

customer_return.order.customer_returns.size

Checklist:

Customer returns are associated to orders via return items. This
means that, if a order has multiple line items and a single
customer return for all of them, then the customer return will
be listed twice. Listing customer returns multiple times adds
noise without any significant benefit, and can be misleading when
calling `order.customer_returns.size` and similars.
@spaghetticode spaghetticode self-assigned this Oct 26, 2021
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, the spec could be useful to avoid future regressions on the same thing but I'm fine as is as well. Thank you, Andrea!

@waiting-for-dev waiting-for-dev merged commit 28213ce into solidusio:master Nov 8, 2021
@waiting-for-dev waiting-for-dev deleted the spaghetticode/uniq-customer-returns branch November 8, 2021 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants