3528 Scope Customer Totals report also by variant (not just product) and use variant SKU #3646
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kristinalim
force-pushed
the
fix/3528-product_sku_in_report
branch
2 times, most recently
from
68f343e to
34eed18
Compare
Probably a copy&paste error. The PackingReport had exactly the same spec.
This conversion is done by Transpec 3.4.0 with the following command:
transpec spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb
* 8 conversions
from: == expected
to: eq(expected)
* 8 conversions
from: obj.should
to: expect(obj).to
* 1 conversion
from: obj.stub(:message)
to: allow(obj).to receive(:message)
For more details: https://github.com/yujinakayama/transpec#supported-conversions
kristinalim
force-pushed
the
fix/3528-product_sku_in_report
branch
2 times, most recently
from
4c62a3c to
a64b078
Compare
kristinalim
changed the title
This groups the Customer Totals report by variant too (among others, including by product), and then changes the report to use the variant SKU not the product SKU. This adds a spec for customer totals grouping and details, which replaces the test specific to the SKU column.
Line items belong to a single order.
kristinalim
force-pushed
the
fix/3528-product_sku_in_report
branch
from
a64b078 to
807cdb4
Compare
|
I updated the description of this commit: 46e7090, and also the title and description of this PR, to reflect that the grouping is by variant in addition to (not "instead of") by product. 🙂 No code changes. In an earlier version, I just replaced the grouping by product to grouping by variant. The current version uses both because both are important. |
|
I hate myself. I literally spent 20 minutes on "I don't see SKU in the report, am I on the correct report?" Only to discover that SKU is called "product code" in the export 🤦♀️ Anyway this is good to go :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Scope Customer Totals report also by variant (not just product) and use variant SKU.
(This includes the work of @mkllnk in his fork. Thank you, @mkllnk!)
What? Why?
Closes #3528
The report groups the line item data by product, among other attributes. We need to display the variant SKU, not the product SKU, so we need to group these by variant
insteadalso.What should we test?
Release notes
Changelog Category: Fixed