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

Update basket stock records when strategy changes #4180

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 14 additions & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,19 @@
# development
django-debug-toolbar>=2.2,<4.3
django-extensions>=2.2,<3.3
django-extra-views>=0.14
django-haystack>=3.2.1
django-phonenumber-field>=7.1
django_tables2>=2.6
django-treebeard>=4.7
django-widget-tweaks>=1.5
babel>=2.12.1
factory_boy>=3.3.0
phonenumbers>=8.13
psycopg2-binary>=2.8,<2.10
purl>=1.6
sorl_thumbnail>=12
easy_thumbnails>=2.8

# Sandbox
Whoosh>=2.7,<2.8
Expand All @@ -12,7 +24,8 @@ django-redis>=4.12,<5.4
pysolr>=3.9,<3.10
redis>=3.5,<5.1
requests>=2.25,<3
uWSGI>=2.0.19
# Does not build on Windows
# uWSGI>=2.0.19
whitenoise>=5.2,<6.6

# Linting
Expand Down
15 changes: 12 additions & 3 deletions src/oscar/apps/basket/abstract_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,17 +325,23 @@ def reset_offer_applications(self):
self.offer_applications = OfferApplications()
self._lines = None

def merge_line(self, line, add_quantities=True):
def merge_line(self, line, add_quantities=True, strategy=None):
"""
For transferring a line from another basket to this one.

This is used with the "Saved" basket functionality.
"""
# TODO: If the login causes the strategy to be None, the line won't save.
# IntegrityError at /checkout/
# (1048, "Column 'stockrecord_id' cannot be null")
# We'll need to display a message if that's the case.
try:
existing_line = self.lines.get(line_reference=line.line_reference)
except ObjectDoesNotExist:
# Line does not already exist - reassign its basket
line.basket = self
if strategy:
line.stockrecord = strategy.fetch_for_product(line.product).stockrecord
line.save()
else:
# Line already exists - assume the max quantity is correct and
Expand All @@ -344,14 +350,17 @@ def merge_line(self, line, add_quantities=True):
existing_line.quantity += line.quantity
else:
existing_line.quantity = max(existing_line.quantity, line.quantity)

if strategy:
existing_line.stockrecord = strategy.fetch_for_product(existing_line.product).stockrecord
existing_line.save()
line.delete()
finally:
self._lines = None

merge_line.alters_data = True

def merge(self, basket, add_quantities=True):
def merge(self, basket, add_quantities=True, strategy=None):
"""
Merges another basket with this one.

Expand All @@ -361,7 +370,7 @@ def merge(self, basket, add_quantities=True):
# Use basket.lines.all instead of all_lines as this function is called
# before a strategy has been assigned.
for line_to_merge in basket.lines.all():
self.merge_line(line_to_merge, add_quantities)
self.merge_line(line_to_merge, add_quantities=add_quantities, strategy=strategy)
basket.status = self.MERGED
basket.date_merged = now()
basket._lines = None
Expand Down
8 changes: 4 additions & 4 deletions src/oscar/apps/basket/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def get_basket(self, request):
old_baskets = list(manager.filter(owner=request.user))
basket = old_baskets[0]
for other_basket in old_baskets[1:]:
self.merge_baskets(basket, other_basket)
self.merge_baskets(basket, other_basket, strategy=request.strategy)
# count number of items that have been merged
num_items_merged += other_basket.num_items

Expand All @@ -166,7 +166,7 @@ def get_basket(self, request):
if cookie_basket:
# count number of items in the basket
num_items_merged += cookie_basket.num_items
self.merge_baskets(basket, cookie_basket)
self.merge_baskets(basket, cookie_basket, strategy=request.strategy)
request.cookies_to_delete.append(cookie_key)

elif cookie_basket:
Expand Down Expand Up @@ -197,13 +197,13 @@ def get_basket(self, request):

return basket

def merge_baskets(self, master, slave):
def merge_baskets(self, master, slave, strategy=None):
"""
Merge one basket into another.

This is its own method to allow it to be overridden
"""
master.merge(slave, add_quantities=False)
master.merge(slave, add_quantities=False, strategy=strategy)

# pylint: disable=unused-argument
def get_cookie_basket(self, cookie_key, request, manager):
Expand Down
41 changes: 40 additions & 1 deletion tests/integration/partner/test_strategy.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@

from django.test import TestCase

from oscar.apps.basket.models import Basket
from oscar.apps.basket.models import Line
from oscar.apps.catalogue import models
from oscar.apps.partner import strategy
from oscar.test import factories


class TestDefaultStrategy(TestCase):
strategy = None

def setUp(self):
self.strategy = strategy.Default()

Expand Down Expand Up @@ -58,6 +61,42 @@ def test_availability_does_not_require_price(self):
self.assertTrue(info.availability.is_available_to_buy)


class UK2(strategy.UK):
def select_stockrecord(self, product):
try:
return product.stockrecords.all()[1]
except IndexError:
pass


class TestStrategyChange(TestCase):
basket = None
product = None
strategy = None

def setUp(self):
self.product = factories.create_product()
factories.create_stockrecord(product=self.product, partner_sku="1", price=1)
factories.create_stockrecord(product=self.product, partner_sku="2", price=2)
self.strategy = strategy.Default()
self.basket = Basket.objects.create()
self.basket.strategy = strategy.UK()
self.basket.add_product(self.product, 1)

def test_basket_price_changes_when_strategy_does(self):
self.assertEqual(self.basket.all_lines()[0].stockrecord.partner_sku, "1")
self.basket.strategy = UK2()
self.basket.freeze()
self.assertEqual(self.basket.total_excl_tax, 2)

def test_basket_stockrecord_does_not_change_when_strategy_does(self):
self.assertEqual(self.basket.all_lines()[0].stockrecord.partner_sku, "1")
self.basket.strategy = UK2()
self.basket.freeze()
self.assertEqual(self.basket.all_lines()[0].stockrecord.partner_sku, "1")
self.assertEqual(self.basket.total_excl_tax, 2)


class TestDefaultStrategyForParentProductWhoseVariantsHaveNoStockRecords(TestCase):
def setUp(self):
self.strategy = strategy.Default()
Expand Down Expand Up @@ -127,4 +166,4 @@ def test_pricing_policy_unavailable_if_no_price_excl_tax(self):
product = factories.ProductFactory(stockrecords=[])
factories.StockRecordFactory(price=None, product=product)
info = strategy.US().fetch_for_product(product)
self.assertFalse(info.price.exists)
self.assertFalse(info.price.exists)