-
Notifications
You must be signed in to change notification settings - Fork 10.7k
Add Cost of Goods Sold fields in product editors in admin #54399
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
Conversation
b077262 to
a4916a0
Compare
a4916a0 to
2784c6f
Compare
For now it's not sortable, and for variations it displays the default cost value.
ba23772 to
bd43bc1
Compare
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
|
Size Change: +215 B (0%) Total Size: 5.39 MB |
in "Remove costs" bulk action confirmation dialog.
aee23fa to
ae6bb26
Compare
|
Hi @coreymckrill @message-dimke , Apart from reviewing the code changes, please make sure to review the testing instructions and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. You can follow this guide to find out what good testing instructions should look like: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Konamiman!
Thanks for your work on this. I followed the instructions and this tests well. I left some feedback but nothing too serious. I also have a few questions/comments:
-
There's nothing in the database for these, and get_cogs_value for the corresponding product object will return null; but as per #54458 this is equivalent to zero, and an explicit zero is clearer than an empty value.
I read the linked PR but it's still unclear to me why 0 is clearer than an empty value. I mean, if (for some reason) something costs me 0 to make why is that the same as "I haven't even looked at this"? I'm missing a bunch of context around COGS so apologies if this has been discussed before.
-
Additional information (like the range of costs defined for existing variations, similarly to how the price column shows a range of variation prices) might be added in an additional pull request too.
I guess it's fine if this comes in a later PR but the wording here makes me think it might not come at all. If that's the case, I believe we need to find a way to convey that the cost for variations might not be accurate, as otherwise this would just be confusing: you go and set prices for variations and then the table shows something else.
-
I tried to test the new COGS functions in a WP-CLI shell but I was greeted by this error:
wp> $p = wc_get_product ( ... ); wp> $p->get_cogs_value(); Notice: Function WC_Product::get_cogs_value was called <strong>incorrectly</strong>. The Cost of Goods sold feature is disabled, thus the method called will do nothing and will return dummy data.The same code as a snippet tied to a hook worked correctly, but I believe once the feature is enabled it should also work on the shell. Why is that not the case?
-
COGS values don't seem to play nicely with different decimal separators. Changing the decimal separator in WC > Settings > General from
.to,correctly changes things for the display and saving of prices, but not for COGS. Entering2,5as a COGS value will result in2being saved, which is incorrect.
Similarly, COGS values are always rounded (2.0becomes2) as opposed to normal product prices.
plugins/woocommerce/client/legacy/js/admin/meta-boxes-product-variation.js
Outdated
Show resolved
Hide resolved
plugins/woocommerce/client/legacy/js/admin/meta-boxes-product-variation.js
Outdated
Show resolved
Hide resolved
plugins/woocommerce/client/legacy/js/admin/meta-boxes-product-variation.js
Outdated
Show resolved
Hide resolved
plugins/woocommerce/client/legacy/js/admin/meta-boxes-product-variation.js
Outdated
Show resolved
Hide resolved
plugins/woocommerce/client/legacy/js/admin/meta-boxes-product-variation.js
Outdated
Show resolved
Hide resolved
plugins/woocommerce/client/legacy/js/admin/meta-boxes-product-variation.js
Outdated
Show resolved
Hide resolved
For all practical purposes (including cost calculations for orders) WooCommerce will consider that the cost of any product that hasn't got a cost value explicitly set is equal to zero, so I thought that it's important to make that zero explicitly stated. The fact that zero values aren't actually stored is an implementation detail (which could cease to be true in the future). Note that for variations zero and empty have indeed different meanings, and that's why in that case empty values are preserved in the UI.
This is indeed planned (although possibly not for the MVP). Displaying the default variation cost seemed like the most reasonable approach until a proper range display is implemented, but I'm open to discussing alternatives.
This is a bug in the features controller that will get fixed in #55303.
Good catch. Fixed in |
jorgeatorres
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for making the adjustments @Konamiman!
I'm not merging yet in case @woocommerce/ventures wants to review, per @jconroy's comment above.
message-dimke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks perfect! Thank you, @Konamiman!
I went through all the 10 steps described and clicked here and there on my own - not a single glitch. Awesome!
(for variations this time)
Changes proposed in this Pull Request:
This pull request adds the ability to edit the Cost of Goods Sold values for products in the product editor of the classic admin UI. It also adds a (yet incomplete) "Cost" column to the products list table in the admin area.
Simple and variable product editors
The "General" tab of the product editor gets a new cost column that gets tooltip'ed differently depending on whether the product is variable or not:
This field will show a value of zero for products that don't have an explicit value set. There's nothing in the database for these, and
get_cogs_valuefor the corresponding product object will returnnull; but as per #54458 this is equivalent to zero, and an explicit zero is clearer than an empty value.Variation editor
Variations get their own cost field. For variations that don't have an explicit cost value set, a placeholder indicating the default value configured for the parent is displayed:
Quick and bulk edit
The cost can be edited using the standard quick and bulk editors too. For variable products the value that gets modified is the default value that gets applied to variations:
Bulk actions for variations
One new bulk action is added for variations to remove the custom cost value of all variations (thus effectively reverting to the default value). Confirmation is requested before proceeding:
💡 An additional added improvement is that now the action selector dropdown will go back to the default "Bulk actions" text after the action has run (or the prompt/confirmation has been cancelled). This is useful when having to retry an action after having supplied invalid data.
Cost column
A new "Cost" column is added to the products list table:
Two new filters are added to customize how this column is rendered, similar to the existing filter for prices:
woocommerce_get_cogs_htmlandwoocommerce_empty_cogs_htmlFor now this column is not sortable, as this would require the addition of a new column in the
wc_product_meta_lookupdatabase table. This will be implemented in a separate pull request.Also, for variable products this column only shows the default cost value for variations. Additional information (like the range of costs defined for existing variations, similarly to how the price column shows a range of variation prices) might be added in an additional pull request too.
How to test the changes in this Pull Request:
Enable the Cost of Goods Sold feature if you haven't done that yet (WooCommerce - Settings - Advanced - Features, check the box for "Cost of Goods Sold" and save).
Create a few products, simple and variable with some variations.
Go to the products table ("Products" in the left menu). Open the regular editor for some of the products, simple and variable. The cost value that you see in the "General" tab should be zero. Verify that you can modify the value and save it, and when you reload the editor the value is still there.
Try the quick editor ("Quick edit" link in the product rows in the products table). Verify that the cost shown in the "Cost" field is initially 0, and that you can change it and save, and the value gets persisted too.
Try the bulk editor (mark the checkbox for a few products in the products table, select "Edit" in the "Bulk actions" dropdown, then click "Apply"). Verify that the cost is modified for the selected products only if the cost field is set to "Change to:", and that an empty value sets a cost of zero.
Open the editor for a variation ("Variations" tab when editing a variable product). Verify that the "Cost" field shows a placeholder with the cost value set for the parent product in the "General" tab (or zero, if none has been set). Verify that you can set a value and save it, and that you can then delete the value and save it too (note that an empty value is not equivalent to zero in this case).
Try the "Remove custom costs" bulk action for variations (edit a variable object, go to the "Variations" tab, click the "Bulk actions" dropdown). Try accepting and cancelling the confirmation prompt (it should remove the custom cost of all the variations in the first case, and no costs should be changed in the second case).
Verify that the "Cost" column in the products table shows the proper values for each product (zero is indicated with a hyphen).
Try the cost column HTML filters with the following snippet:
Changelog entry
Changelog Entry Details
Significance
Type
Message
Changelog Entry Comment
Comment