fix(loyalty): fix runtime bugs in storefront routes, point expiration, and enforce settings
- Add total_points_voided column to LoyaltyCard with migration (loyalty_002) - Fix storefront self_enroll to use correct service method signature and schema fields - Fix get_my_card/get_my_transactions to use get_card_by_customer_and_merchant - Fix transaction history field reference (balance_after -> points_balance_after) - Fix point_expiration task: wrong field names and manual balance update -> card.expire_points() - Register storefront_router in definition.py and export all routers from __init__.py - Enforce MerchantLoyaltySettings in storefront enrollment, points, and stamp void operations - Fix test fixture using non-existent balance_after column - Suppress intentional architecture validator warnings in templates Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -8,7 +8,7 @@ from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
revision = "dev_tools_001"
|
||||
down_revision = "loyalty_001"
|
||||
down_revision = "loyalty_002"
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
|
||||
@@ -31,6 +31,13 @@ def _get_platform_router():
|
||||
return platform_router
|
||||
|
||||
|
||||
def _get_storefront_router():
|
||||
"""Lazy import of storefront router to avoid circular imports."""
|
||||
from app.modules.loyalty.routes.api.storefront import storefront_router
|
||||
|
||||
return storefront_router
|
||||
|
||||
|
||||
# Loyalty module definition
|
||||
loyalty_module = ModuleDefinition(
|
||||
code="loyalty",
|
||||
@@ -197,6 +204,7 @@ def get_loyalty_module_with_routers() -> ModuleDefinition:
|
||||
loyalty_module.admin_router = _get_admin_router()
|
||||
loyalty_module.store_router = _get_store_router()
|
||||
loyalty_module.platform_router = _get_platform_router()
|
||||
loyalty_module.storefront_router = _get_storefront_router()
|
||||
return loyalty_module
|
||||
|
||||
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
"""add total_points_voided to loyalty_cards
|
||||
|
||||
Revision ID: loyalty_002
|
||||
Revises: loyalty_001
|
||||
Create Date: 2026-02-08
|
||||
"""
|
||||
from alembic import op
|
||||
import sqlalchemy as sa
|
||||
|
||||
revision = "loyalty_002"
|
||||
down_revision = "loyalty_001"
|
||||
branch_labels = None
|
||||
depends_on = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.add_column(
|
||||
"loyalty_cards",
|
||||
sa.Column(
|
||||
"total_points_voided",
|
||||
sa.Integer(),
|
||||
nullable=False,
|
||||
server_default="0",
|
||||
comment="Lifetime points voided (returns + expirations)",
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
op.drop_column("loyalty_cards", "total_points_voided")
|
||||
@@ -160,6 +160,12 @@ class LoyaltyCard(Base, TimestampMixin):
|
||||
nullable=False,
|
||||
comment="Lifetime points redeemed",
|
||||
)
|
||||
total_points_voided = Column(
|
||||
Integer,
|
||||
default=0,
|
||||
nullable=False,
|
||||
comment="Lifetime points voided (returns + expirations)",
|
||||
)
|
||||
|
||||
# =========================================================================
|
||||
# Wallet Integration
|
||||
@@ -326,6 +332,7 @@ class LoyaltyCard(Base, TimestampMixin):
|
||||
points: Number of points to void
|
||||
"""
|
||||
self.points_balance = max(0, self.points_balance - points)
|
||||
self.total_points_voided += points
|
||||
self.last_activity_at = datetime.now(UTC)
|
||||
|
||||
def expire_points(self, points: int) -> None:
|
||||
@@ -336,6 +343,7 @@ class LoyaltyCard(Base, TimestampMixin):
|
||||
points: Number of points to expire
|
||||
"""
|
||||
self.points_balance = max(0, self.points_balance - points)
|
||||
self.total_points_voided += points
|
||||
|
||||
# =========================================================================
|
||||
# Properties
|
||||
|
||||
@@ -5,7 +5,13 @@ Loyalty module API routes.
|
||||
Provides REST API endpoints for:
|
||||
- Admin: Platform-wide loyalty program management
|
||||
- Store: Store loyalty operations (stamps, points, cards)
|
||||
- Public: Customer enrollment and wallet passes
|
||||
- Platform: Platform administration
|
||||
- Storefront: Customer enrollment and wallet passes
|
||||
"""
|
||||
|
||||
__all__: list[str] = []
|
||||
from app.modules.loyalty.routes.api.admin import admin_router
|
||||
from app.modules.loyalty.routes.api.store import store_router
|
||||
from app.modules.loyalty.routes.api.platform import platform_router
|
||||
from app.modules.loyalty.routes.api.storefront import storefront_router
|
||||
|
||||
__all__ = ["admin_router", "store_router", "platform_router", "storefront_router"]
|
||||
|
||||
@@ -13,7 +13,7 @@ Uses store from middleware context (StoreContextMiddleware).
|
||||
|
||||
import logging
|
||||
|
||||
from fastapi import APIRouter, Depends, Query, Request
|
||||
from fastapi import APIRouter, Depends, HTTPException, Query, Request
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.api.deps import get_current_customer_api
|
||||
@@ -29,7 +29,7 @@ from app.modules.loyalty.schemas import (
|
||||
)
|
||||
from app.modules.tenancy.exceptions import StoreNotFoundException
|
||||
|
||||
router = APIRouter()
|
||||
storefront_router = APIRouter()
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
@@ -38,7 +38,7 @@ logger = logging.getLogger(__name__)
|
||||
# =============================================================================
|
||||
|
||||
|
||||
@router.get("/loyalty/program")
|
||||
@storefront_router.get("/loyalty/program")
|
||||
def get_program_info(
|
||||
request: Request,
|
||||
db: Session = Depends(get_db),
|
||||
@@ -63,7 +63,7 @@ def get_program_info(
|
||||
return response
|
||||
|
||||
|
||||
@router.post("/loyalty/enroll")
|
||||
@storefront_router.post("/loyalty/enroll")
|
||||
def self_enroll(
|
||||
request: Request,
|
||||
data: CardEnrollRequest,
|
||||
@@ -77,15 +77,31 @@ def self_enroll(
|
||||
if not store:
|
||||
raise StoreNotFoundException("context", identifier_type="subdomain")
|
||||
|
||||
logger.info(f"Self-enrollment for {data.customer_email} at store {store.subdomain}")
|
||||
# Check if self-enrollment is allowed
|
||||
settings = program_service.get_merchant_settings(db, store.merchant_id)
|
||||
if settings and not settings.allow_self_enrollment:
|
||||
raise HTTPException(403, "Self-enrollment is not available")
|
||||
|
||||
card = card_service.enroll_customer(
|
||||
db,
|
||||
store_id=store.id,
|
||||
customer_email=data.customer_email,
|
||||
customer_phone=data.customer_phone,
|
||||
customer_name=data.customer_name,
|
||||
)
|
||||
# Resolve customer_id
|
||||
customer_id = data.customer_id
|
||||
if not customer_id and data.email:
|
||||
from app.modules.customers.models.customer import Customer
|
||||
|
||||
customer = (
|
||||
db.query(Customer)
|
||||
.filter(Customer.email == data.email, Customer.store_id == store.id)
|
||||
.first()
|
||||
)
|
||||
if not customer:
|
||||
raise HTTPException(400, "Customer not found with provided email")
|
||||
customer_id = customer.id
|
||||
|
||||
if not customer_id:
|
||||
raise HTTPException(400, "Either customer_id or email is required")
|
||||
|
||||
logger.info(f"Self-enrollment for customer {customer_id} at store {store.subdomain}")
|
||||
|
||||
card = card_service.enroll_customer_for_store(db, customer_id, store.id)
|
||||
|
||||
return CardResponse.model_validate(card)
|
||||
|
||||
@@ -95,7 +111,7 @@ def self_enroll(
|
||||
# =============================================================================
|
||||
|
||||
|
||||
@router.get("/loyalty/card")
|
||||
@storefront_router.get("/loyalty/card")
|
||||
def get_my_card(
|
||||
request: Request,
|
||||
customer: CustomerContext = Depends(get_current_customer_api),
|
||||
@@ -116,11 +132,11 @@ def get_my_card(
|
||||
if not program:
|
||||
return {"card": None, "program": None, "locations": []}
|
||||
|
||||
# Look up card by customer email
|
||||
card = card_service.get_card_by_customer_email(
|
||||
# Look up card by customer ID and merchant
|
||||
card = card_service.get_card_by_customer_and_merchant(
|
||||
db,
|
||||
customer_id=customer.id,
|
||||
merchant_id=program.merchant_id,
|
||||
customer_email=customer.email,
|
||||
)
|
||||
|
||||
if not card:
|
||||
@@ -146,7 +162,7 @@ def get_my_card(
|
||||
}
|
||||
|
||||
|
||||
@router.get("/loyalty/transactions")
|
||||
@storefront_router.get("/loyalty/transactions")
|
||||
def get_my_transactions(
|
||||
request: Request,
|
||||
skip: int = Query(0, ge=0),
|
||||
@@ -169,17 +185,16 @@ def get_my_transactions(
|
||||
return {"transactions": [], "total": 0}
|
||||
|
||||
# Get card
|
||||
card = card_service.get_card_by_customer_email(
|
||||
card = card_service.get_card_by_customer_and_merchant(
|
||||
db,
|
||||
customer_id=customer.id,
|
||||
merchant_id=program.merchant_id,
|
||||
customer_email=customer.email,
|
||||
)
|
||||
|
||||
if not card:
|
||||
return {"transactions": [], "total": 0}
|
||||
|
||||
# Get transactions
|
||||
from sqlalchemy import func
|
||||
from app.modules.loyalty.models import LoyaltyTransaction
|
||||
from app.modules.tenancy.models import Store as StoreModel
|
||||
|
||||
@@ -200,7 +215,8 @@ def get_my_transactions(
|
||||
"transaction_type": tx.transaction_type.value if hasattr(tx.transaction_type, 'value') else str(tx.transaction_type),
|
||||
"points_delta": tx.points_delta,
|
||||
"stamps_delta": tx.stamps_delta,
|
||||
"balance_after": tx.balance_after,
|
||||
"points_balance_after": tx.points_balance_after,
|
||||
"stamps_balance_after": tx.stamps_balance_after,
|
||||
"transaction_at": tx.transaction_at.isoformat() if tx.transaction_at else None,
|
||||
"notes": tx.notes,
|
||||
"store_name": None,
|
||||
|
||||
@@ -17,6 +17,7 @@ Handles points operations including:
|
||||
import logging
|
||||
from datetime import UTC, datetime
|
||||
|
||||
from fastapi import HTTPException
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
from app.modules.loyalty.exceptions import (
|
||||
@@ -94,6 +95,12 @@ class PointsService:
|
||||
logger.warning(f"Points attempted on stamps-only program {program.id}")
|
||||
raise LoyaltyCardInactiveException(card.id)
|
||||
|
||||
# Check if order reference is required
|
||||
from app.modules.loyalty.services.program_service import program_service
|
||||
settings = program_service.get_merchant_settings(db, card.merchant_id)
|
||||
if settings and settings.require_order_reference and not order_reference:
|
||||
raise HTTPException(400, "Order reference required")
|
||||
|
||||
# Check minimum purchase amount
|
||||
if program.minimum_purchase_cents > 0 and purchase_amount_cents < program.minimum_purchase_cents:
|
||||
return {
|
||||
@@ -353,6 +360,12 @@ class PointsService:
|
||||
|
||||
program = card.program
|
||||
|
||||
# Check if void transactions are allowed
|
||||
from app.modules.loyalty.services.program_service import program_service
|
||||
settings = program_service.get_merchant_settings(db, card.merchant_id)
|
||||
if settings and not settings.allow_void_transactions:
|
||||
raise LoyaltyCardInactiveException(card.id)
|
||||
|
||||
# Verify staff PIN if required
|
||||
verified_pin = None
|
||||
if program.require_staff_pin:
|
||||
|
||||
@@ -337,6 +337,12 @@ class StampService:
|
||||
|
||||
program = card.program
|
||||
|
||||
# Check if void transactions are allowed
|
||||
from app.modules.loyalty.services.program_service import program_service
|
||||
settings = program_service.get_merchant_settings(db, card.merchant_id)
|
||||
if settings and not settings.allow_void_transactions:
|
||||
raise LoyaltyCardInactiveException(card.id)
|
||||
|
||||
# Verify staff PIN if required
|
||||
verified_pin = None
|
||||
if program.require_staff_pin:
|
||||
|
||||
@@ -164,17 +164,16 @@ def _expire_points_for_program(db: Session, program: LoyaltyProgram) -> tuple[in
|
||||
store_id=None, # System action, no store
|
||||
transaction_type=TransactionType.POINTS_EXPIRED.value,
|
||||
points_delta=-expired_points,
|
||||
balance_after=0,
|
||||
points_balance_after=0,
|
||||
stamps_delta=0,
|
||||
stamps_balance_after=card.stamps_balance,
|
||||
stamps_balance_after=card.stamp_count,
|
||||
notes=f"Points expired after {program.points_expiration_days} days of inactivity",
|
||||
transaction_at=datetime.now(UTC),
|
||||
)
|
||||
db.add(transaction)
|
||||
|
||||
# Update card balance
|
||||
card.points_balance = 0
|
||||
card.total_points_voided = (card.total_points_voided or 0) + expired_points
|
||||
# Update card balance and voided tracking
|
||||
card.expire_points(expired_points)
|
||||
# Note: We don't update last_activity_at for expiration
|
||||
|
||||
cards_processed += 1
|
||||
|
||||
@@ -75,7 +75,7 @@
|
||||
<label class="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-2">
|
||||
Max Failed Attempts
|
||||
</label>
|
||||
<input type="number" min="3" max="10"
|
||||
<input type="number" min="3" max="10" {# noqa: FE-008 #}
|
||||
x-model.number="settings.staff_pin_lockout_attempts"
|
||||
class="w-full px-4 py-2 text-sm border border-gray-300 dark:border-gray-600 rounded-lg focus:border-purple-400 focus:outline-none dark:bg-gray-700 dark:text-gray-300">
|
||||
<p class="mt-1 text-xs text-gray-500 dark:text-gray-400">Number of wrong attempts before lockout (3-10)</p>
|
||||
|
||||
@@ -111,7 +111,7 @@
|
||||
</div>
|
||||
</form>
|
||||
|
||||
<!-- Success Modal -->
|
||||
<!-- Success Modal --> {# noqa: FE-004 #}
|
||||
<div x-show="enrolledCard" class="fixed inset-0 z-50 flex items-center justify-center bg-black/50">
|
||||
<div class="bg-white dark:bg-gray-800 rounded-lg shadow-xl max-w-md w-full mx-4 p-6">
|
||||
<div class="text-center">
|
||||
|
||||
@@ -24,7 +24,7 @@
|
||||
<div class="grid gap-6 md:grid-cols-2">
|
||||
<div>
|
||||
<label class="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-2">Points per EUR spent</label>
|
||||
<input type="number" min="1" max="100" x-model.number="settings.points_per_euro"
|
||||
<input type="number" min="1" max="100" x-model.number="settings.points_per_euro" {# noqa: FE-008 #}
|
||||
class="w-full px-4 py-2 text-sm border border-gray-300 dark:border-gray-600 rounded-lg focus:border-purple-400 focus:outline-none dark:bg-gray-700 dark:text-gray-300">
|
||||
<p class="mt-1 text-xs text-gray-500">1 EUR = <span x-text="settings.points_per_euro || 1"></span> point(s)</p>
|
||||
</div>
|
||||
|
||||
@@ -146,7 +146,7 @@
|
||||
<label class="block text-xs text-gray-500 dark:text-gray-400 mb-1">Purchase Amount</label>
|
||||
<div class="relative">
|
||||
<span class="absolute inset-y-0 left-0 flex items-center pl-3 text-gray-500">EUR</span>
|
||||
<input type="number" step="0.01" min="0"
|
||||
<input type="number" step="0.01" min="0" {# noqa: FE-008 #}
|
||||
x-model.number="earnAmount"
|
||||
class="w-full pl-12 pr-4 py-2 text-sm border border-gray-300 dark:border-gray-600 rounded-lg focus:border-green-400 focus:outline-none dark:bg-gray-700 dark:text-gray-300">
|
||||
</div>
|
||||
|
||||
2
tests/fixtures/loyalty_fixtures.py
vendored
2
tests/fixtures/loyalty_fixtures.py
vendored
@@ -139,7 +139,7 @@ def test_loyalty_transaction(db, test_loyalty_card, test_store):
|
||||
store_id=test_store.id,
|
||||
transaction_type=TransactionType.POINTS_EARNED.value,
|
||||
points_delta=50,
|
||||
balance_after=150,
|
||||
points_balance_after=150,
|
||||
stamps_delta=0,
|
||||
stamps_balance_after=0,
|
||||
notes="Test purchase",
|
||||
|
||||
Reference in New Issue
Block a user