Skip to content
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

Amend backorder after cancellations #12945

Merged
merged 3 commits into from
Oct 29, 2024

Conversation

mkllnk
Copy link
Member

@mkllnk mkllnk commented Oct 24, 2024

ℹ️ Funded Feature. Please track ALL ASSOCIATED WORK under the associated tracking code: #11678 DFC Orders

What? Why?

The new job class blends code from the BackorderJob and the
CompleteBackorderJob for the specific case of adjusting quantities after
an order has been cancelled.

I would like to write a more general class which can be used for any
order amendmends but this was the quickest solution to cater for
currently running pilots.

What should we test?

  • For @RaggedStaff:
  • Checkout -> backorder is created.
  • Cancel order -> backorder should be adjusted. If there was only one order then this results in an empty order, I think. I hope that doesn't cause trouble down the line. I could add a check and set it to cancelled?

Release notes

Changelog Category (reviewers may add a label for the release notes):

  • User facing changes
  • API changes (V0, V1, DFC or Webhook)
  • Technical changes only
  • Feature toggled

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

The new job class blends code from the BackorderJob and the
CompleteBackorderJob for the specific case of adjusting quantities after
an order has been cancelled.

I would like to write a more general class which can be used for any
order amendmends but this was the quickest solution to cater for
currently running pilots.
@mkllnk mkllnk added the api changes These pull requests change the API and can break integrations label Oct 24, 2024
@mkllnk mkllnk self-assigned this Oct 24, 2024
@RaggedStaff RaggedStaff added the pr-staged-uk staging.openfoodnetwork.org.uk label Oct 24, 2024
@mkllnk mkllnk marked this pull request as ready for review October 25, 2024 02:33
@RaggedStaff
Copy link
Collaborator

RaggedStaff commented Oct 25, 2024

Ok, I've tested this & it seems to work (Shopify draft orders are correctly amended when an OFN order is cancelled)! 🎉

Cancel order -> backorder should be adjusted. If there was only one order then this results in an empty order, I think. I hope that doesn't cause trouble down the line. I could add a check and set it to cancelled?

This is a bit concerning... I think we do need to cancel the Order. Currently, when I remove all OFN Orders, the last item is left on the Shopify Order (this is a known issue - Shopify does not permit Orders without Line Items). The back Order then completes on OC close & submits that final product for delivery. 🫤

Copy link
Collaborator

@rioug rioug left a comment

Choose a reason for hiding this comment

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

A bit hard to follow all the transformations, but it makes sense in the end. Nice one !

Copy link
Member

@dacook dacook left a comment

Choose a reason for hiding this comment

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

I have to admit I don't completely follow it, but I can see that it's been clearly specced and confirmed with manual testing already, and it needs to move forward.

Also I think it will be important to refactor in the not-too-distant future to ensure integrity (so we don't have duplicated code get out of sync) and make it easier for developers to use and understand it.

Comment on lines +91 to +92
.to change { beans.reload.on_hand }.from(9).to(15)
.and change { chia_seed.reload.on_hand }.from(7).to(12)
Copy link
Member

Choose a reason for hiding this comment

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

If the retail order and the backorder are both cancelled, why would the on_hand be changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

This block is cancelling the OFN order. So when you cancel the order the local OFN stock is returned. The backorder hasn't been touch yet. That's done in the next block.

When the backorder is updated, there are two different methods. If the product is on demand then the local stock may be reduced again because we are cancelling part of the backorder at the same time. But when it's stock controlled, we don't touch the OFN stock. The backorder is only affected by the line item quantities while the stock is synced with the remote catalog.

@dacook dacook removed the pr-staged-uk staging.openfoodnetwork.org.uk label Oct 28, 2024
@mkllnk mkllnk merged commit b8822ee into openfoodfoundation:master Oct 29, 2024
56 checks passed
@mkllnk mkllnk deleted the dfc-amend-after-cancel branch October 29, 2024 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api changes These pull requests change the API and can break integrations
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[DFC Orders] Cancelled Order does not reduce DFC Order quantities immediately
4 participants