Skip to content

Conversation

@Konamiman
Copy link
Contributor

@Konamiman Konamiman commented Jan 13, 2025

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:

image

image

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_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.

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:

image

image

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:

image

image

image

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:

image

image

💡 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:

image

Two new filters are added to customize how this column is rendered, similar to the existing filter for prices: woocommerce_get_cogs_html and woocommerce_empty_cogs_html

For now this column is not sortable, as this would require the addition of a new column in the wc_product_meta_lookup database 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:

  1. 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).

  2. Create a few products, simple and variable with some variations.

  3. 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.

  4. 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.

  5. 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.

  6. 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).

  7. 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).

  8. Verify that the "Cost" column in the products table shows the proper values for each product (zero is indicated with a hyphen).

  9. Try the cost column HTML filters with the following snippet:

add_filter('woocommerce_empty_cogs_html', function($html, $product)  {
    return "<p>No cost for " . $product->get_id() . "</p>";
}, 2, 10);

add_filter('woocommerce_get_cogs_html', function($html, $value, $product)  {
    return
        0.0 === $value ?
        $html :
        '<p>Cost for ' . $product->get_id() . ': ' . $value . '</p>';
}, 3, 10);
  1. Finally, disable the Cost of Goods Sold feature and verify that none of the added UI elements that you just tested is visible anymore, but you can still edit the rest of the product fields (using regular editor, quick edit, bulk edit, variation editor and variation bulk action).

Changelog entry

  • Automatically create a changelog entry from the details below.
  • This Pull Request does not require a changelog entry. (Comment required below)
Changelog Entry Details

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Changelog Entry Comment

Comment

@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Jan 13, 2025
@Konamiman Konamiman force-pushed the cogs/admin-product-cogs-edition branch 2 times, most recently from b077262 to a4916a0 Compare January 16, 2025 12:51
@Konamiman Konamiman force-pushed the cogs/admin-product-cogs-edition branch from a4916a0 to 2784c6f Compare January 17, 2025 09:29
@Konamiman Konamiman force-pushed the cogs/admin-product-cogs-edition branch from ba23772 to bd43bc1 Compare January 17, 2025 12:01
@Konamiman Konamiman marked this pull request as ready for review January 17, 2025 12:42
@github-actions
Copy link
Contributor

github-actions bot commented Jan 17, 2025

Test using WordPress Playground

The changes in this pull request can be previewed and tested using a WordPress Playground instance.
WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

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.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 17, 2025

Size Change: +215 B (0%)

Total Size: 5.39 MB

compressed-size-action

@Konamiman Konamiman force-pushed the cogs/admin-product-cogs-edition branch from aee23fa to ae6bb26 Compare January 21, 2025 12:02
@Konamiman Konamiman requested review from a team and jorgeatorres and removed request for a team January 21, 2025 14:48
@github-actions
Copy link
Contributor

github-actions bot commented Jan 21, 2025

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:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

Copy link
Member

@jorgeatorres jorgeatorres left a 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. Entering 2,5 as a COGS value will result in 2 being saved, which is incorrect.
    Similarly, COGS values are always rounded (2.0 becomes 2) as opposed to normal product prices.

@Konamiman
Copy link
Contributor Author

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"?

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.

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.

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.

Notice: Function WC_Product::get_cogs_value was called incorrectly. The Cost of Goods sold feature is disabled, thus the method called will do nothing and will return dummy data.

This is a bug in the features controller that will get fixed in #55303.

Changing the decimal separator in WC > Settings > General from . to , correctly changes things for the display and saving of prices, but not for COGS. Entering 2,5 as a COGS value will result in 2 being saved, which is incorrect.

Good catch. Fixed in b0c4f79 (#54399).

Copy link
Member

@jorgeatorres jorgeatorres left a 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.

Copy link
Contributor

@message-dimke message-dimke left a 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!

@Konamiman Konamiman merged commit 149dc76 into trunk Feb 19, 2025
40 checks passed
@Konamiman Konamiman deleted the cogs/admin-product-cogs-edition branch February 19, 2025 09:45
@github-actions github-actions bot added this to the 9.8.0 milestone Feb 19, 2025
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Feb 19, 2025
@veljkho veljkho added status: analysis complete Indicates if a PR has been analysed by Solaris needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs: external testing Indicates if the PR requires further testing conducted by testers external to the development team. plugin: woocommerce Issues related to the WooCommerce Core plugin. status: analysis complete Indicates if a PR has been analysed by Solaris

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants