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

DFC Orders: Calculate wholesale price for imported retail variants #13033

Merged
merged 4 commits into from
Dec 16, 2024

Conversation

mkllnk
Copy link
Member

@mkllnk mkllnk commented Dec 12, 2024

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

What? Why?

In OFN, enterprises add fees on top of producer prices to cover their expenses. But when a DFC product is imported with its recommended retail price then the resulting price in the OFN shop is too high. In the current implementation, retail variants are not ordered in individually, they are ordered as bulk items with a cheaper wholesale price. Considering this backordering logic, we need to set the price of an imported retail variant to the wholesale cost price, calculated from the wholesale variant.

What should we test?

  • @RaggedStaff is testing.
  • Import DFC catalog.
  • Retail variant prices should be adjusted.
  • Wholesale variant prices should not be changed.
  • No prices are changed in the absence of transformations.

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

@mkllnk mkllnk added the user facing changes Thes pull requests affect the user experience label Dec 12, 2024
@mkllnk mkllnk self-assigned this Dec 12, 2024
@mkllnk mkllnk marked this pull request as ready for review December 12, 2024 05:47
@mkllnk mkllnk added the pr-staged-uk staging.openfoodnetwork.org.uk label Dec 12, 2024
@RaggedStaff
Copy link
Collaborator

Variant pricing looks good for me. 👍🏻

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.

Looks Good 👍

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.

Great to see refactoring as you go, nice work!

There's one assumption I wasn't sure about, but I trust that you are ;)


wholesale_variant_price = transformation.offer.price

return unless wholesale_variant_price
Copy link
Member

Choose a reason for hiding this comment

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

unless price is present? a number? positive?

I guess for all intents and purposes it doesn't matter, if it's truthy, we can assume in this context that it's valid? I'm not familiar with the price data structure so I can only assume.

But if we're going to do a division on it later, it might we worth checking it a bit more thoroughly.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I was aware of that assumption. Thanks for checking though. It's either a number or nil.

@dacook
Copy link
Member

dacook commented Dec 16, 2024

Reviews and testing done, ready to merge!

@dacook dacook merged commit e689844 into openfoodfoundation:master Dec 16, 2024
56 checks passed
@dacook dacook removed the pr-staged-uk staging.openfoodnetwork.org.uk label Dec 16, 2024
@mkllnk mkllnk deleted the dfc-pricing branch December 17, 2024 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user facing changes Thes pull requests affect the user experience
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[DFC Orders] Retail Variant Pricing
4 participants