Skip to content

Commit

Permalink
fix: prevent errors on accessing unsaved basket properties
Browse files Browse the repository at this point in the history
  • Loading branch information
Julie Rymer committed Apr 23, 2024
1 parent 07acb50 commit 509d068
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 7 deletions.
17 changes: 10 additions & 7 deletions src/oscar/apps/basket/abstract_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ def basket_quantity(self, line):
The basket can contain multiple lines with the same product and
stockrecord, but different options. Those quantities are summed up.
"""
if self.id is None:
return 0

Check warning on line 207 in src/oscar/apps/basket/abstract_models.py

View check run for this annotation

Codecov / codecov/patch

src/oscar/apps/basket/abstract_models.py#L207

Added line #L207 was not covered by tests
matching_lines = self.lines.filter(stockrecord=line.stockrecord)
quantity = matching_lines.aggregate(Sum("quantity"))["quantity__sum"]
return quantity or 0
Expand Down Expand Up @@ -590,7 +592,7 @@ def time_since_creation(self, test_datetime=None):

@property
def contains_a_voucher(self):
if not self.id:
if self.id is None:
return False
return self.vouchers.exists()

Expand Down Expand Up @@ -636,17 +638,18 @@ def product_quantity(self, product):
The basket can contain multiple lines with the same product, but
different options and stockrecords. Those quantities are summed up.
"""
if self.id:
matching_lines = self.lines.filter(product=product)
quantity = matching_lines.aggregate(Sum("quantity"))["quantity__sum"]
return quantity or 0

return 0
if self.id is None:
return 0
matching_lines = self.lines.filter(product=product)
quantity = matching_lines.aggregate(Sum("quantity"))["quantity__sum"]
return quantity or 0

def line_quantity(self, product, stockrecord, options=None):
"""
Return the current quantity of a specific product and options
"""
if self.id is None:
return 0
ref = self._create_line_reference(product, stockrecord, options)
try:
return self.lines.get(line_reference=ref).quantity
Expand Down
12 changes: 12 additions & 0 deletions tests/integration/basket/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ def test_has_zero_items(self):
self.assertEqual(0, self.basket.num_items)

def test_doesnt_contain_vouchers(self):
# assert no exception on unsaved basket
self.assertFalse(Basket().contains_a_voucher)
self.assertFalse(self.basket.contains_a_voucher)

def test_can_be_edited(self):
Expand Down Expand Up @@ -165,8 +167,14 @@ def test_can_be_flushed(self):
self.assertEqual(self.basket.num_items, 0)

def test_returns_correct_product_quantity(self):
# assert no exception on unsaved basket
self.assertEqual(0, Basket().product_quantity(self.product))
self.assertEqual(10, self.basket.product_quantity(self.product))

def test_returns_correct_line_quantity_for_unsaved_basket(self):
# assert no exception on unsaved basket
self.assertEqual(0, Basket().line_quantity(self.product, self.record))

def test_returns_correct_line_quantity_for_existing_product_and_stockrecord(self):
self.assertEqual(10, self.basket.line_quantity(self.product, self.record))

Expand Down Expand Up @@ -259,6 +267,10 @@ def test_max_allowed_quantity(self):

def test_is_quantity_allowed(self):
with self.settings(OSCAR_MAX_BASKET_QUANTITY_THRESHOLD=20):
# assert no exception on unsaved basket
allowed, message = Basket().is_quantity_allowed(1)
self.assertTrue(allowed)
self.assertIsNone(message)
# 7 or below is possible
allowed, message = self.basket.is_quantity_allowed(qty=7)
self.assertTrue(allowed)
Expand Down

0 comments on commit 509d068

Please sign in to comment.