Skip to content

Remove set_backorders logic from external products closes #29696 #29766

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.

Sign up for GitHub

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

Jump to bottom
Merged
Konamiman merged 1 commit into from
Apr 27, 2021
Merged

Remove set_backorders logic from external products closes #29696 #29766

Konamiman merged 1 commit into from
Apr 27, 2021

Conversation

@roykho
Copy link
Contributor

@roykho roykho commented Apr 26, 2021

All Submissions:

Changes proposed in this Pull Request:

We're making this change because external products does not support backorders.

Closes #29696

How to test the changes in this Pull Request:

  1. Create an external product.
  2. Go to the product list and check the bulk edit checkbox for the external product.
  3. On the Bulk actions dropdown select Edit then Apply
  4. Go to Backorders dropdown and select any value.
  5. Click on Update
  6. Ensure there are no errors in your logs and the product updated without issues.

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

Fix - Bulk edit on external products causes an error when changing the Backorders setting.

@roykho roykho requested review from a team and Konamiman and removed request for a team April 26, 2021 15:25
Konamiman
Konamiman approved these changes Apr 27, 2021
View reviewed changes
Copy link
Contributor

@Konamiman Konamiman left a comment

Choose a reason for hiding this comment

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

Hi @roykho. I tested the fix and it works fine. However I'm thinking that perhaps a full fix would include hiding the "Backorders?" field from the quick edit form since that's effectively a no-op for external products and it might be confusing to see it there.

Anyway, I'm not sure if that would be worth the effort and in that case, if that would be in scope for this PR. So I'm going to approve this and leave you the decision about whether to implement that field hiding now, later, or not at all.

@roykho
Copy link
Contributor Author

roykho commented Apr 27, 2021 •
edited
Loading

@Konamiman - It is already hidden for single external products.

@Konamiman
Copy link
Contributor

Konamiman commented Apr 27, 2021

@roykho Sorry I said it wrongly. It's indeed hidden in "Quick edit", but not in "Bulk edit":

image

@roykho
Copy link
Contributor Author

roykho commented Apr 27, 2021

@Konamiman - So you can't hide that because it is editing in "bulk" which means it has to be there for all other products. Imagine, you're bulk editing mixed products that are external and simple. You can't hide that just because you have one external products in the list.

@Konamiman
Copy link
Contributor

Konamiman commented Apr 27, 2021

@roykho D'OH. Of course, I hadn't counted on that. 🤦‍♂️ Ready to merge then.

@Konamiman Konamiman merged commit 8095f79 into trunk Apr 27, 2021
@Konamiman Konamiman deleted the fix/29696 branch April 27, 2021 14:02
@woocommercebot woocommercebot added release: add changelog Mark all PRs that have not had their changelog entries added. [auto] release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels Apr 27, 2021
@github-actions github-actions bot added this to the 5.4.0 milestone Apr 27, 2021
@ObliviousHarmony ObliviousHarmony added changelog added and removed release: add changelog Mark all PRs that have not had their changelog entries added. [auto] labels May 14, 2021
@juliaamosova juliaamosova added testing instructions added and removed release: add testing instructions PRs that have not had testing instructions added to the wiki. [auto] labels May 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Konamiman Konamiman Konamiman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

5.4.0

6 participants

@roykho @Konamiman @ObliviousHarmony @juliaamosova @woocommercebot