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

Max quantity for bulk buy implementation is too complicated #334

Open
Tracked by #330
mkllnk opened this issue Oct 29, 2018 · 3 comments
Open
Tracked by #330

Max quantity for bulk buy implementation is too complicated #334

mkllnk opened this issue Oct 29, 2018 · 3 comments

Comments

@mkllnk
Copy link
Member

mkllnk commented Oct 29, 2018

Description

Reviewing openfoodfoundation/openfoodnetwork#2880 (comment) I noticed that our bulk buy implementation is overly complicated and confusing. The idea of bulk buy is that the shop orders in bulk, which is cheaper. For example 50 kg bags of potatoes have better value (per kg) than 5 kg bags. The big bag gets split up for customers, depending how much they want. Customers order the desired quantity of a product, for example 3 kg of potatoes. But they can also indicate that they would take up to 5 kg of potatoes if it's necessary to make bulk buy work.

In the current code we modified the Spree::LineItem model and added another field called max_quantity. It is optional and can be activated on product level (field group_buy). In order to make this work, we had to override a lot of Spree logic to pass the max_quantity through any stock logic.

Expected Behavior

The implementation is small and easy to understand. It is separate from Spree's logic and behaves like an optional add-on.

Actual Behavior

We modified a lot of Spree logic which makes upgrades difficult.

Steps to Reproduce

Some of the affected code:

  • app/controllers/cart_controller.rb
  • app/models/spree/line_item_decorator.rb
  • app/models/spree/order_decorator.rb
  • app/serializers/api/admin/line_item_serializer.rb
  • app/serializers/api/line_item_serializer.rb
  • app/services/cart_service.rb
  • app/views/spree/admin/orders/bulk_management.html.haml
  • app/views/spree/products/_add_to_cart_quantity_fields.html.haml
  • db/migrate/20120802031147_add_group_buy_fields.rb
  • several reports and specs

Context

We are working on the Spree upgrade and decided that we should not include this in the current upgrade. This can be done separately and it shouldn't delay the upgrade. It's tech debt to be paid after the Spree upgrade.

Possible Fix

Implement a new model called QuantityRange which belongs to a LineItem. It would also allow us to set a minimum quantity if that feature is requested.

@kirstenalarsen
Copy link

A note that I would expect this to be completely redone when we implement the new network structure as the bulk buy variant will become the base of other combination / split variants. So definitely the way it is done now can be done away with. Could this be a case of a good first implementation of new network structure (alongside the customer prices)? We need to keep this functionality as we have people using it, but it we need to pull out out / redo to enable spree upgrade then we should think through and create the new architecture that we ultimately want? @myriamboure @luisramos0 @sauloperez @HugsDaniel

@luisramos0
Copy link

This is not required for the spree upgrade.
anyway, I am looking forward getting into "the network" story :-D

@myriamboure
Copy link

Count on us to tell you the story whenever you want @luisramos0 we love telling those kind of stories :-) Agree with you on principle @kirstenalarsen even thought I think we will need to implement first some product chain basic features before we touch this so it might be something that happen a bit later in the process... without the backbone of new product chain not sure we can redo this, but we can think about it ! If not touched by Spree upgrade no hurry :-)

@lin-d-hop lin-d-hop transferred this issue from openfoodfoundation/openfoodnetwork Nov 18, 2022
@RachL RachL added the Parked label Feb 28, 2024
@github-project-automation github-project-automation bot moved this to Candidates in Wishlist Board Feb 28, 2024
@RachL RachL moved this from Candidates to Stuff from main repo for review in Wishlist Board Feb 28, 2024
@RachL RachL removed the Parked label Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Stuff from main repo for review
Development

No branches or pull requests

5 participants