Skip to content

Conversation

@jeffstieler
Copy link
Contributor

All Submissions:

Changes proposed in this Pull Request:

Closes #30618.

This PR seeks to resolve the errant stock adjustment on restocked items in partially refunded orders by preserving the _reduced_stock and _restock_refunded_items order item metadata value so wc_maybe_adjust_line_item_product_stock() correctly identifies no $diff between the ordered quantity and the amount deducted and restocked.

I've added unit test and E2E test coverage, but I'm still unsure of all of the potential ramifications of keeping the _reduced_stock metadata around (perhaps extensions are looking for it?).

How to test the changes in this Pull Request:

  1. Create an order for 2 products with set stock levels (products with manage stock enabled and an inventory quantity)
  2. Refund one of the products on the order and select the "restock refunded items" option.
  3. Notice the note is added confirming the stock has been restored.
  4. Update the order (click "update").
  5. Verify there is not a new note indicating the stock level has been reduced again.
  6. Verify the product inventory levels have not changed (beyond the restocking) by checking the product edit screen.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Enter a summary of all changes on this Pull Request. This will appear in the changelog if accepted.

Fix - errant stock adjustment on restocked items when saving partially refunded orders.

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@jeffstieler jeffstieler force-pushed the fix/30618-restocked-line-item-reduction branch from 0c18184 to 41e2ce4 Compare November 15, 2021 14:55
@peterfabian
Copy link
Contributor

Hey, is this one ready for review? Just checking if we should assign a reviewer or it's WIP. Thanks!

@jeffstieler
Copy link
Contributor Author

Hey, is this one ready for review?

Yes it is, I'll assign a reviewer.

@jeffstieler jeffstieler requested review from a team and barryhughes and removed request for a team November 19, 2021 13:08
@jeffstieler jeffstieler force-pushed the fix/30618-restocked-line-item-reduction branch from 41e2ce4 to 4b10190 Compare November 19, 2021 13:40
@barryhughes
Copy link
Member

Can't imagine it's related to this change (so perhaps worth rebasing on latest trunk) but noting there is fail via the test coverage check:

 1) Automattic\WooCommerce\Tests\Internal\ProductAttributesLookup\FiltererTest::test_filtering_excludes_hidden_products with data set #0 (true, array('Red'))
Failed asserting that two arrays are equal.

(Re-running checks did not clear it.)

barryhughes
barryhughes previously approved these changes Nov 19, 2021
Copy link
Member

@barryhughes barryhughes left a comment

Choose a reason for hiding this comment

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

This is great, tried a few permutations of the testing instructions and it works well.

@jeffstieler: hitting approve but will let you merge (there is a failing check—most likely unrelated—but letting you take a look first, just in case).

Prevents fully restocked items on partially refunded orders from having stock reduced on subsequent order updates.
@jeffstieler
Copy link
Contributor Author

Rebasing off trunk didn't fix the code coverage job unfortunately, but I feel confident enough merging this since the same configuration (sans coverage) is passing.

@jeffstieler jeffstieler merged commit 209566b into trunk Nov 19, 2021
@jeffstieler jeffstieler deleted the fix/30618-restocked-line-item-reduction branch November 19, 2021 21:24
@jeffstieler jeffstieler added the release: add changelog Mark all PRs that have not had their changelog entries added. [auto] label Nov 19, 2021
@github-actions github-actions bot added this to the 6.1.0 milestone Nov 19, 2021
@github-actions
Copy link
Contributor

Hi @jeffstieler, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the status: needs changelog label
  • Add the status: needs testing instructions label

@jonathansadowski jonathansadowski added changelog added and removed release: add changelog Mark all PRs that have not had their changelog entries added. [auto] labels Dec 17, 2021
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.

Refunding orders results in incorrect stock adjustments

6 participants