engineering/make_post_sell
1
0
Fork 0
Code Issues 2 Pull requests Projects Releases 2 Packages Wiki Activity Actions

Fix cart discount memoization bug and template null safety

Browse source 
Resolves critical AttributeError in free cart checkout when coupons are applied.

- Fix cart discount calculation cache invalidation in coupon.py
- Add template null safety for active_card in cart_checkout.j2
- Add comprehensive regression test for free cart coupon flow
- Document full defect hunt process in journal.rst

All 87 tests pass. Fixes production crashes during free cart checkout.
This commit is contained in:
Russell Ballestrini 2025-07-27 17:09:59 -04:00
parent 3f692aabbd
commit de868682a8
4 changed files with 184 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

37
journal.rst
View file

@ -465,4 +465,39 @@ ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIBkkOwYqPfaQIliMt6p6aRAOv6xDBY6dmZnN2m5qmtzO
Thu May 29 10:52:00 AM EST 2025
==================================
we upgraded the os & build host from 22.04 to 24.04 LTS
we upgraded the os & build host from 22.04 to 24.04 LTS
Mon Jan 27 09:00:00 AM EST 2025
==================================
All day defect hunt with Claude Code assistance: Fixed critical AttributeError in free cart checkout flow.
**The Bug**: Production users reported crashes when checking out with free carts (when coupons made the total $0.00). The error was `AttributeError: 'NoneType' object has no attribute 'active_card'` in cart.py lines 458 and 468.
**Root Cause Analysis**: When a cart total is free (≤ $0.64), no stripe_user_shop object is created, so `stripe_user_shop` becomes `None`. The buggy code tried to access `stripe_user_shop.active_card` without null checking.
**The Hunt Process**:
1. Started with SSH connectivity issues (IPv4 timeout) - fixed by disabling systemd socket activation
2. Found Python 2/3 compatibility bug in base64 decoding - fixed `string.decode("base64")` → `base64.b64decode()`
3. Discovered the main AttributeError during user checkout with coupon-applied cart
4. Added comprehensive unit tests for cart payment threshold logic (64 cent boundary)
5. Created regression test but struggled with coupon discount application
**Major Discovery**: Cart discount memoization bug! The `discounted_shop_totals_in_cents` property was being cached before coupons were applied. When other cart properties accessed it early, the discount calculation returned stale results showing no discount even with valid coupons attached.
**The Fixes**:
1. **cart.py lines 458, 468**: Added null checks: `stripe_user_shop and stripe_user_shop.active_card is None` and `stripe_user_shop.active_card if stripe_user_shop else None`
2. **coupon.py lines 143, 177**: Added `cart._bust_memoized_attributes()` after applying/removing coupons to clear stale discount calculations
3. **cart_checkout.j2 lines 10-24**: Added conditional rendering: only show card section when `active_card` is not None, otherwise show "No payment required"
4. **test_functional.py**: Added comprehensive regression test `test_cart_checkout_free_coupon_full_flow_regression` covering the complete flow
**Key Technical Insights**:
- Memoization can hide timing bugs in complex property dependencies
- Free cart logic (≤64 cents) bypasses payment flow entirely, creating edge cases
- Template-level null safety is crucial when backend can return None for optional objects
- SQLAlchemy session refresh can break memoized calculations
**Validation**: All 87 tests pass. The regression test verifies that free carts with applied coupons can successfully reach checkout confirmation without crashes.
**Credit**: Major thanks to groupr for collaborative debugging and Claude Code for systematic analysis. This was a complex multi-layer bug requiring fixes across model logic, view controllers, templates, and proper test coverage.

6
make_post_sell/templates/cart_checkout.j2
View file

@ -7,6 +7,7 @@
<section class="checkout-left well">
{% if active_card %}
<h2><b>Active Card</b></h2>
{{ stripe.display_card(active_card, actions=False) }}
@ -16,6 +17,11 @@
{% if request.shop and request.shop.is_ready %}
<a href="/billing" class="product-edit-button mps-button">Use a different card</a>
{% endif %}
{% else %}
<h2><b>Payment</b></h2>
<p>No payment required for this order.</p>
<br>
{% endif %}
{% if request.user.active_address %}
<h2>Active Shipping Address</h2>

140
make_post_sell/tests/test_functional.py
View file

@ -61,13 +61,18 @@ class FunctionalTests(unittest.TestCase):
transaction.abort()
Base.metadata.drop_all(bind=self.engine)
def get_csrf_token(self, shop_id, keywords="test"):
def get_csrf_token(self, shop_id, keywords="xyznotfound"):
"""
GET the search page scoped to the given shop_id (and a dummy keywords param
so the form actually renders) and extract the CSRF hidden input via regex.
"""
url = f"/search?shop_id={shop_id}&keywords={keywords}"
res = self.testapp.get(url, status=200)
res = self.testapp.get(url, status=[200, 302])
# If search redirects (no results go to home, single result goes to product)
if res.status_int == 302:
res = res.follow()
html = res.body.decode("utf-8")
m = re.search(
@ -162,6 +167,17 @@ class AuthenticatedFunctionalTests(FunctionalTests):
"expiration_date": "2030-10-31",
}
self.coupon2_params = {
"code": "FREECART",
"description": "Free cart coupon for testing",
"action_type": "dollar-off",
"action_value": "6.00", # $6.00 off (more than $3.50 product = free cart)
"max_redemptions": "1",
"cart_qualifier": "1", # $1 minimum (way below $3.50)
"max_redemptions_per_user": "1",
"expiration_date": "2030-12-31", # Within allowed range
}
# create test user1 and user2.
self.user1 = get_or_create_user_by_email(self.dbsession, "test1@example.com")
self.user2 = get_or_create_user_by_email(self.dbsession, "test2@example.com")
@ -576,6 +592,126 @@ class AuthenticatedFunctionalTests(FunctionalTests):
result6 = stripe_user_shop_no_card.active_card if stripe_user_shop_no_card else None
self.assertIsNone(result6)
@patch("smtplib.SMTP")
@patch("make_post_sell.models.Product.is_ready", mock_always_true)
def test_cart_checkout_free_coupon_full_flow_regression(self, mock_smtp):
"""Full functional test for free cart checkout with coupon (regression test).
This tests the complete flow that was causing the AttributeError in production:
1. Create shop and cheap product
2. Create coupon that makes cart free
3. Add product to cart
4. Apply coupon (cart becomes free, requires_payment = False)
5. Checkout with no payment method (stripe_user_shop = None)
6. Should succeed without AttributeError crash
"""
# 1. Create shop, product, and coupon that makes cart free
coupon_res = self.make_new_coupon_for_shop(self.coupon2_params)
if coupon_res.status_int == 302:
coupon_res.follow() # Follow the redirect after successful creation
else:
self.fail(f"Coupon creation failed. Status: {coupon_res.status_int}, Body: {coupon_res.body.decode()[:500]}")
# Refresh the database session to avoid detached object issues
transaction.manager.commit()
transaction.manager.begin()
# Get the Product and Shop objects from the database.
all_products = get_all_products(self.dbsession)
product = all_products.one()
shop = get_shop_by_name(self.dbsession, self.shop1_params["name"])
# Re-query users to ensure they're attached to current session
self.user1 = get_or_create_user_by_email(self.dbsession, self.user1_creds[0])
self.user2 = get_or_create_user_by_email(self.dbsession, self.user2_creds[0])
# Log out user1 (shop owner)
self.testapp.get("/log-out")
# 3. Customer adds product to cart
self.log_in_user(self.user2_creds)
# Access the product page first to establish session context (like working test)
res_csrf_redirect2 = self.testapp.get(f"/p/{product.uuid_str}")
res_csrf2 = res_csrf_redirect2.follow() # Follow the single redirect to the slug version.
# Add single product to cart
add_to_cart_res = self.testapp.post(
"/cart/add",
{
"product_id": product.id,
"shop_id": shop.id,
"csrf_token": self.get_csrf_token(shop.uuid_str),
},
)
add_to_cart_res.follow().follow() # Follow redirects
# Verify product in cart
user_cart = shop.get_active_cart_for_user(self.user2)
self.assertEqual(1, user_cart.count) # 1 product
self.assertEqual(user_cart.total_price_in_cents, 350) # $3.50 = 350 cents
self.assertTrue(user_cart.requires_payment) # $3.50 is above 64 cent threshold
# 4. Apply the free coupon
# Get the coupon from database (use coupon2_params code)
coupons = get_coupons_by_code(self.dbsession, "FREECART")
self.assertEqual(len(coupons), 1, f"Expected 1 coupon with code FREECART, found {len(coupons)}")
coupon = coupons[0]
coupon_apply_res = self.testapp.post(
f"/coupon/apply?shop_id={shop.uuid_str}",
{
"coupon_id": coupon.id,
"shop_id": shop.uuid_str,
"csrf_token": self.get_csrf_token(shop.uuid_str),
},
)
# Apply coupon and refresh cart state
self.dbsession.refresh(user_cart)
# Clear any memoized attributes after session refresh
user_cart._bust_memoized_attributes()
# Verify coupon was applied
self.assertEqual(len(user_cart.coupons), 1)
self.assertEqual(user_cart.coupons[0].code, "FREECART")
# Verify coupon applied correctly: $3.50 - $6.00 = FREE!
coupon = user_cart.coupons[0]
self.assertEqual(coupon.action_value, 600) # $6.00 off
self.assertEqual(coupon.cart_qualifier, 100) # $1.00 minimum
self.assertEqual(user_cart.total_price_in_cents, 350) # $3.50 original
# Verify coupon applied correctly: $3.50 - $6.00 = FREE!
self.assertTrue(user_cart.is_discounted) # Should now be discounted
self.assertEqual(user_cart.total_discounted_price_in_cents, 0) # FREE!
self.assertFalse(user_cart.requires_payment) # No payment needed
# 5. Attempt checkout (this is where the bug would occur)
# Since cart is free, no stripe_user_shop is created, so stripe_user_shop = None
# The bug was: stripe_user_shop.active_card would crash with AttributeError
# This should NOT crash with AttributeError: 'NoneType' object has no attribute 'active_card'
checkout_res = self.testapp.post(
f"/u/cart/{user_cart.uuid_str}/checkout",
{
"shop_id": shop.id,
"csrf_token": self.get_csrf_token(shop.uuid_str),
},
)
# 6. Verify successful checkout flow (no crash)
# For free cart, should render checkout confirmation (200) not redirect to billing (302)
self.assertEqual(checkout_res.status_int, 200)
checkout_body = checkout_res.body.decode()
# Should NOT contain payment-related messaging since cart is free
self.assertNotIn("Please enter your payment information", checkout_body)
self.assertNotIn("Please make a payment method active", checkout_body)
# Should contain order confirmation elements
self.assertIn("Please confirm your order", checkout_body)
@patch("make_post_sell.models.Product.is_ready", mock_always_true)
def make_new_coupon_for_shop(self, coupon_params=None):
# 1. log in as user1.

4
make_post_sell/views/coupon.py
View file

@ -140,6 +140,8 @@ def coupon_apply_to_cart(request):
if coupon not in request.active_cart.coupons:
msg = ("The coupon was applied to your cart.", "success")
request.active_cart.coupons.append(coupon)
# Clear cached discount calculations
request.active_cart._bust_memoized_attributes()
request.dbsession.add(request.active_cart)
request.dbsession.flush()
else:
@ -173,6 +175,8 @@ def coupon_remove_from_cart(request):
if coupon in cart.coupons:
msg = ("The coupon was removed from your cart.", "success")
cart.coupons.remove(coupon)
# Clear cached discount calculations
cart._bust_memoized_attributes()
request.dbsession.add(cart)
request.dbsession.flush()
else: