fix: validate products before creating order to follow architecture rules
Restructured create_letzshop_order to follow the "validate first, then write" architecture pattern: Phase 1 (Read-only validation): - Check if order already exists - Parse all inventory units and collect GTINs - Batch query all products by GTIN (single query instead of N queries) - Validate all products exist - raise ValidationException BEFORE any writes Phase 2 (Database writes): - Only after all validation passes, create customer, order, and items This ensures if validation fails, no database modifications happen, so the endpoint/task simply doesn't commit - no rollback needed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -342,6 +342,10 @@ class OrderService:
|
||||
"""
|
||||
Create an order from Letzshop shipment data.
|
||||
|
||||
Validates all products exist BEFORE creating any database records.
|
||||
This ensures we don't leave the session in an inconsistent state
|
||||
if validation fails.
|
||||
|
||||
Args:
|
||||
db: Database session
|
||||
vendor_id: Vendor ID
|
||||
@@ -354,6 +358,67 @@ class OrderService:
|
||||
ValidationException: If product not found by GTIN
|
||||
"""
|
||||
order_data = shipment_data.get("order", {})
|
||||
|
||||
# Generate order number using Letzshop order number
|
||||
letzshop_order_number = order_data.get("number", "")
|
||||
order_number = f"LS-{vendor_id}-{letzshop_order_number}"
|
||||
|
||||
# Check if order already exists (read-only, safe to do first)
|
||||
existing = (
|
||||
db.query(Order)
|
||||
.filter(Order.order_number == order_number)
|
||||
.first()
|
||||
)
|
||||
if existing:
|
||||
return existing
|
||||
|
||||
# =====================================================================
|
||||
# PHASE 1: Parse and validate all data BEFORE any database writes
|
||||
# =====================================================================
|
||||
|
||||
# Parse inventory units
|
||||
inventory_units = shipment_data.get("inventoryUnits", [])
|
||||
if isinstance(inventory_units, dict):
|
||||
inventory_units = inventory_units.get("nodes", [])
|
||||
|
||||
# Collect all GTINs and validate products exist
|
||||
gtins = set()
|
||||
for unit in inventory_units:
|
||||
variant = unit.get("variant", {}) or {}
|
||||
trade_id = variant.get("tradeId", {}) or {}
|
||||
gtin = trade_id.get("number")
|
||||
if gtin:
|
||||
gtins.add(gtin)
|
||||
|
||||
# Batch query all products by GTIN
|
||||
products_by_gtin: dict[str, Product] = {}
|
||||
if gtins:
|
||||
products = (
|
||||
db.query(Product)
|
||||
.filter(
|
||||
and_(
|
||||
Product.vendor_id == vendor_id,
|
||||
Product.gtin.in_(gtins),
|
||||
)
|
||||
)
|
||||
.all()
|
||||
)
|
||||
products_by_gtin = {p.gtin: p for p in products if p.gtin}
|
||||
|
||||
# Validate all products exist BEFORE creating anything
|
||||
missing_gtins = gtins - set(products_by_gtin.keys())
|
||||
if missing_gtins:
|
||||
missing_gtin = next(iter(missing_gtins))
|
||||
logger.error(
|
||||
f"Product not found for GTIN {missing_gtin} in vendor {vendor_id}. "
|
||||
f"Order: {order_number}"
|
||||
)
|
||||
raise ValidationException(
|
||||
f"Product not found for GTIN {missing_gtin}. "
|
||||
f"Please ensure the product catalog is in sync."
|
||||
)
|
||||
|
||||
# Parse address data
|
||||
ship_address = order_data.get("shipAddress", {}) or {}
|
||||
bill_address = order_data.get("billAddress", {}) or {}
|
||||
ship_country = ship_address.get("country", {}) or {}
|
||||
@@ -364,16 +429,6 @@ class OrderService:
|
||||
ship_first_name = ship_address.get("firstName", "") or ""
|
||||
ship_last_name = ship_address.get("lastName", "") or ""
|
||||
|
||||
# Find or create customer (inactive)
|
||||
customer = self.find_or_create_customer(
|
||||
db=db,
|
||||
vendor_id=vendor_id,
|
||||
email=customer_email,
|
||||
first_name=ship_first_name,
|
||||
last_name=ship_last_name,
|
||||
is_active=False,
|
||||
)
|
||||
|
||||
# Parse order date
|
||||
order_date = datetime.now(UTC)
|
||||
completed_at_str = order_data.get("completedAt")
|
||||
@@ -402,18 +457,19 @@ class OrderService:
|
||||
}
|
||||
status = status_mapping.get(letzshop_state, "pending")
|
||||
|
||||
# Generate order number using Letzshop order number
|
||||
letzshop_order_number = order_data.get("number", "")
|
||||
order_number = f"LS-{vendor_id}-{letzshop_order_number}"
|
||||
# =====================================================================
|
||||
# PHASE 2: All validation passed - now create database records
|
||||
# =====================================================================
|
||||
|
||||
# Check if order already exists
|
||||
existing = (
|
||||
db.query(Order)
|
||||
.filter(Order.order_number == order_number)
|
||||
.first()
|
||||
# Find or create customer (inactive)
|
||||
customer = self.find_or_create_customer(
|
||||
db=db,
|
||||
vendor_id=vendor_id,
|
||||
email=customer_email,
|
||||
first_name=ship_first_name,
|
||||
last_name=ship_last_name,
|
||||
is_active=False,
|
||||
)
|
||||
if existing:
|
||||
return existing
|
||||
|
||||
# Create order
|
||||
order = Order(
|
||||
@@ -464,10 +520,6 @@ class OrderService:
|
||||
db.flush()
|
||||
|
||||
# Create order items from inventory units
|
||||
inventory_units = shipment_data.get("inventoryUnits", [])
|
||||
if isinstance(inventory_units, dict):
|
||||
inventory_units = inventory_units.get("nodes", [])
|
||||
|
||||
for unit in inventory_units:
|
||||
variant = unit.get("variant", {}) or {}
|
||||
product_info = variant.get("product", {}) or {}
|
||||
@@ -477,30 +529,8 @@ class OrderService:
|
||||
gtin = trade_id.get("number")
|
||||
gtin_type = trade_id.get("parser")
|
||||
|
||||
# Find product by GTIN
|
||||
product = None
|
||||
if gtin:
|
||||
product = (
|
||||
db.query(Product)
|
||||
.filter(
|
||||
and_(
|
||||
Product.vendor_id == vendor_id,
|
||||
Product.gtin == gtin,
|
||||
)
|
||||
)
|
||||
.first()
|
||||
)
|
||||
|
||||
if not product:
|
||||
# This should be an error per the design requirements
|
||||
logger.error(
|
||||
f"Product not found for GTIN {gtin} in vendor {vendor_id}. "
|
||||
f"Order: {order_number}"
|
||||
)
|
||||
raise ValidationException(
|
||||
f"Product not found for GTIN {gtin}. "
|
||||
f"Please ensure the product catalog is in sync."
|
||||
)
|
||||
# Get product from pre-validated map
|
||||
product = products_by_gtin.get(gtin)
|
||||
|
||||
# Get product name
|
||||
product_name = (
|
||||
@@ -523,7 +553,7 @@ class OrderService:
|
||||
|
||||
order_item = OrderItem(
|
||||
order_id=order.id,
|
||||
product_id=product.id,
|
||||
product_id=product.id if product else None,
|
||||
product_name=product_name,
|
||||
product_sku=variant.get("sku"),
|
||||
gtin=gtin,
|
||||
|
||||
Reference in New Issue
Block a user