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

Calculate stock from DFC wholesale variants #13049

Merged
merged 5 commits into from
Jan 8, 2025

Conversation

mkllnk
Copy link
Member

@mkllnk mkllnk commented Dec 20, 2024

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

What? Why?

When importing DFC products, two products can be coupled as retail variant and wholesale variant of the same product. We aim to sell the retail variant through an OFN shop and then place backorders for the wholesale variant to make use of economies of scale.

We already calculated the price of a retail variant from its wholesale variant:

Now we also calculate the stock level from the wholesale variant.

What should we test?

  • Import a DFC catalog at Admin -> Product Import: https://env-0105831.jcloud-ver-jpe.ik-server.com/api/dfc/Enterprises/test-hodmedod/SuppliedProducts
  • Observe the product prices and stock levels.
  • Add retail products to an order cycle.
  • Change stock levels in Shopify.
  • Add a retail variant to your OFN cart.
  • The stock level should get updated correctly.

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 20, 2024
@mkllnk mkllnk self-assigned this Dec 20, 2024
@mkllnk mkllnk marked this pull request as ready for review December 20, 2024 04:38
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 work, easy to follow the commits and see how the code evolves.

Just one little hiccup at the end:


return unless offer && wholesale_offer&.stockLimitation.present?

offer.stockLimiation = wholesale_offer.stockLimitation.to_i * transformation.factor
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, that spelling doesn't look right:

Suggested change
offer.stockLimiation = wholesale_offer.stockLimitation.to_i * transformation.factor
offer.stockLimitation = wholesale_offer.stockLimitation.to_i * transformation.factor

The test below isn't testing for this. Is it something that can be tested?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well spotted!

While the standard allows a stock limitation on catalog items and offers, the FDC data stores the stock only on the catalog item and not the offer. So this code path didn't run. I added a test case for that now.

@mkllnk mkllnk requested a review from dacook January 2, 2025 05:56
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.

👏

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 👍

@rioug
Copy link
Collaborator

rioug commented Jan 5, 2025

@mkllnk I'll let you handle the conflict

@mkllnk mkllnk force-pushed the dfc-wholesale-stock branch from a167547 to 0bd6fe6 Compare January 7, 2025 00:03
@mkllnk mkllnk added no-staging-UK A tag which does not trigger deployments, indicating a server is being used pr-staged-uk staging.openfoodnetwork.org.uk and removed no-staging-UK A tag which does not trigger deployments, indicating a server is being used labels Jan 7, 2025
@dacook dacook self-assigned this Jan 7, 2025
@dacook
Copy link
Member

dacook commented Jan 7, 2025

Initial issues:

1. Session error causes 500 page

With no explanation of what's wrong. Eg: https://staging.openfoodnetwork.org.uk/admin/dfc_product_imports?enterprise_id=203921&catalog_url=https%3A%2F%2Fenv-0105831.jcloud-ver-jpe.ik-server.com%2Fapi%2Fdfc%2FEnterprises%2Ftest-hodmedod%2FSuppliedProducts&commit=Import

Bugsnag reveals Rack::OAuth2::Client::Error - invalid_grant :: Session not active

2. OIDC settings page says it is active, when it is not.

https://staging.openfoodnetwork.org.uk/admin/oidc_settings

Maybe when we load the page it should perform a check to display current status.
Workaround: disconnect and reconnect.


I will continue trying to test in the morning. I got a bit lost but I think I found my way. I'm confused why the Wholesale product has dropped from 4 to 2 after making an order but maybe I missed something.

@mkllnk
Copy link
Member Author

mkllnk commented Jan 7, 2025

The first issue you found is on our list to fix:

Currently, @rioug is assigned. But who has time first?

@rioug
Copy link
Collaborator

rioug commented Jan 7, 2025

Currently, @rioug is assigned. But who has time first?

Go for it !

@dacook
Copy link
Member

dacook commented Jan 7, 2025

Testing

  • Import a DFC catalog at Admin -> Product Import: https://env-0105831.jcloud-ver-jpe.ik-server.com/api/dfc/Enterprises/test-hodmedod/SuppliedProducts
  • Observe the product prices and stock levels in OFN.
    Case 60x40mL has 4 in stock
    Screenshot 2025-01-08 at 9 43 18 am
  • Add retail products to an order cycle.
  • Change stock levels in Shopify
    Case 60x40mL now has 5 in stock
    Screenshot 2025-01-08 at 9 43 44 am
  • Add a retail variant to your OFN cart.
    Checked out 2x 40mL bottles
    Screenshot 2025-01-08 at 9 48 10 am
  • The stock level should get updated correctly.
    OFN now shows 5 cases available. 5 cases = 30 bottles. 2 are sold, so 28 remaining bottles available.
    Screenshot 2025-01-08 at 9 49 50 am

✅ Ok I think that's correct! Look right to you @mkllnk ?

Other notes

I also played around with updating values in OFN bulk edit products screen, and found that these changes aren't reflected back in Shopify.
A re-import resets the wholesale product stock.

Price

30.20 / 6 = 5.0333333
This makes pricing fully transparent, and the hub would cover their processing costs with enterprise fees in OFN 👍

Product amounts

Product amounts show as weight instead of volume. I can see that Shopify only supports weight (assumed gross weight for shipping, not the net product weight), so that's the best we can do I guess. Unless we were to use a metafield or some other shopify plugin.
Screenshot 2025-01-08 at 9 55 06 am

@mkllnk
Copy link
Member Author

mkllnk commented Jan 8, 2025

Ok I think that's correct! Look right to you @mkllnk ?

Yes, looking good. Thank you! I'll merge then.

@mkllnk mkllnk merged commit 1df3a6b into openfoodfoundation:master Jan 8, 2025
54 of 55 checks passed
@mkllnk mkllnk deleted the dfc-wholesale-stock branch January 8, 2025 01:32
@mkllnk mkllnk removed the pr-staged-uk staging.openfoodnetwork.org.uk label Jan 8, 2025
@RaggedStaff
Copy link
Collaborator

I also played around with updating values in OFN bulk edit products screen, and found that these changes aren't reflected back in Shopify.
A re-import resets the wholesale product stock.

I think that's probably how we want it for now... Shopify is managing the wholesale variant stock, OFN is managing the retail variant stock Syncromising stock updates across multiple locations would be a lot more complex to manage.

That said... we do have local stock of the retail variant, so a check to make sure that doesn't reset on refresh might be handy?

Also worth checking how cancellations are handled: stock should be returned to the retail variant, and (where possible) the back order reduced (& local stock readjusted down).

@dacook
Copy link
Member

dacook commented Jan 9, 2025

Also worth checking how cancellations are handled: stock should be returned to the retail variant, and (where possible) the back order reduced (& local stock readjusted down).

Good idea. I cancelled an order:
Screenshot 2025-01-10 at 9 26 21 am

I didn't check the stock beforehand, but the other day there were 28 bottles in stock.
Now cases are 0, and there's only the 2 cancelled bottles. Is this because the backorder was automatically adjusted down to only order what's needed?
Screenshot 2025-01-10 at 9 29 34 am

I found the backorder, which is empty: https://admin.shopify.com/store/test-hodmedod/draft_orders?selectedView=all
Screenshot 2025-01-10 at 9 33 47 am

@mkllnk
Copy link
Member Author

mkllnk commented Jan 10, 2025

I'm not sure I have the full picture here. But yes, when orders are cancelled, the backorder is adjusted. And if we then don't need to order anything then the backorder ends up being empty.

@mkllnk
Copy link
Member Author

mkllnk commented Jan 10, 2025

The test shop is used in various ways. So I don't think that you can refer to stock levels from a few days ago.

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] Compute stock of retail variants from wholesale variants
4 participants