fix(loyalty): route prefix, module migrations, and wallet barcode tests
- Fix 404 on /admin/loyalty/* and /vendor/loyalty/* by applying ROUTE_CONFIG custom_prefix when registering page routers in main.py (admin, vendor, and storefront registrations all updated) - Move loyalty alembic migrations from central alembic/versions/ into app/modules/loyalty/migrations/versions/ with proper naming convention - Add migrations_path="migrations" to loyalty module definition so the auto-discovery system finds them - Add unit tests for Apple/Google Wallet Code 128 barcode configuration (6 Apple tests, 4 Google tests) - Add integration tests for module migration auto-discovery (4 tests) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -166,6 +166,7 @@ loyalty_module = ModuleDefinition(
|
||||
schemas_path="app.modules.loyalty.schemas",
|
||||
exceptions_path="app.modules.loyalty.exceptions",
|
||||
tasks_path="app.modules.loyalty.tasks",
|
||||
migrations_path="migrations",
|
||||
# =========================================================================
|
||||
# Scheduled Tasks
|
||||
# =========================================================================
|
||||
|
||||
18
main.py
18
main.py
@@ -349,10 +349,11 @@ admin_page_routes = get_admin_page_routes()
|
||||
logger.info(f" Found {len(admin_page_routes)} admin page route modules")
|
||||
|
||||
for route_info in admin_page_routes:
|
||||
logger.info(f" Registering {route_info.module_code} admin pages")
|
||||
prefix = f"/admin{route_info.custom_prefix}" if route_info.custom_prefix else "/admin"
|
||||
logger.info(f" Registering {route_info.module_code} admin pages at {prefix}")
|
||||
app.include_router(
|
||||
route_info.router,
|
||||
prefix="/admin",
|
||||
prefix=prefix,
|
||||
tags=route_info.tags,
|
||||
include_in_schema=route_info.include_in_schema,
|
||||
)
|
||||
@@ -365,10 +366,11 @@ vendor_page_routes = get_vendor_page_routes()
|
||||
logger.info(f" Found {len(vendor_page_routes)} vendor page route modules")
|
||||
|
||||
for route_info in vendor_page_routes:
|
||||
logger.info(f" Registering {route_info.module_code} vendor pages (priority={route_info.priority})")
|
||||
prefix = f"/vendor{route_info.custom_prefix}" if route_info.custom_prefix else "/vendor"
|
||||
logger.info(f" Registering {route_info.module_code} vendor pages at {prefix} (priority={route_info.priority})")
|
||||
app.include_router(
|
||||
route_info.router,
|
||||
prefix="/vendor",
|
||||
prefix=prefix,
|
||||
tags=route_info.tags,
|
||||
include_in_schema=route_info.include_in_schema,
|
||||
)
|
||||
@@ -386,10 +388,11 @@ logger.info(f" Found {len(storefront_page_routes)} storefront page route module
|
||||
# Register at /storefront/* (direct access)
|
||||
logger.info(" Registering storefront routes at /storefront/*")
|
||||
for route_info in storefront_page_routes:
|
||||
logger.info(f" - {route_info.module_code} (priority={route_info.priority})")
|
||||
prefix = f"/storefront{route_info.custom_prefix}" if route_info.custom_prefix else "/storefront"
|
||||
logger.info(f" - {route_info.module_code} at {prefix} (priority={route_info.priority})")
|
||||
app.include_router(
|
||||
route_info.router,
|
||||
prefix="/storefront",
|
||||
prefix=prefix,
|
||||
tags=["storefront-pages"],
|
||||
include_in_schema=False,
|
||||
)
|
||||
@@ -397,9 +400,10 @@ for route_info in storefront_page_routes:
|
||||
# Register at /vendors/{code}/storefront/* (path-based development mode)
|
||||
logger.info(" Registering storefront routes at /vendors/{code}/storefront/*")
|
||||
for route_info in storefront_page_routes:
|
||||
prefix = f"/vendors/{{vendor_code}}/storefront{route_info.custom_prefix}" if route_info.custom_prefix else "/vendors/{vendor_code}/storefront"
|
||||
app.include_router(
|
||||
route_info.router,
|
||||
prefix="/vendors/{vendor_code}/storefront",
|
||||
prefix=prefix,
|
||||
tags=["storefront-pages"],
|
||||
include_in_schema=False,
|
||||
)
|
||||
|
||||
@@ -318,3 +318,148 @@ class TestPinService:
|
||||
pins = pin_service.list_pins_for_vendor(db, test_vendor.id)
|
||||
assert len(pins) >= 1
|
||||
assert any(p.id == test_staff_pin.id for p in pins)
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Apple Wallet Barcode Tests
|
||||
# =============================================================================
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
@pytest.mark.service
|
||||
class TestAppleWalletBarcode:
|
||||
"""Tests for Apple Wallet pass barcode configuration (Code 128)."""
|
||||
|
||||
def _build_pass(self, card):
|
||||
from app.modules.loyalty.services.apple_wallet_service import AppleWalletService
|
||||
|
||||
service = AppleWalletService()
|
||||
return service._build_pass_json(card, card.program)
|
||||
|
||||
def test_primary_barcode_is_code128(self, db, test_loyalty_card):
|
||||
"""Primary barcode format must be Code 128 for retail scanners."""
|
||||
pass_data = self._build_pass(test_loyalty_card)
|
||||
assert pass_data["barcode"]["format"] == "PKBarcodeFormatCode128"
|
||||
|
||||
def test_primary_barcode_uses_card_number_without_dashes(self, db, test_loyalty_card):
|
||||
"""Barcode message is card number with dashes stripped (digits only)."""
|
||||
pass_data = self._build_pass(test_loyalty_card)
|
||||
expected = test_loyalty_card.card_number.replace("-", "")
|
||||
assert pass_data["barcode"]["message"] == expected
|
||||
assert "-" not in pass_data["barcode"]["message"]
|
||||
|
||||
def test_primary_barcode_alttext_shows_formatted_number(self, db, test_loyalty_card):
|
||||
"""altText displays the human-readable card number with dashes."""
|
||||
pass_data = self._build_pass(test_loyalty_card)
|
||||
assert pass_data["barcode"]["altText"] == test_loyalty_card.card_number
|
||||
|
||||
def test_barcodes_array_code128_first_qr_second(self, db, test_loyalty_card):
|
||||
"""Barcodes array has Code 128 first (primary) and QR second (fallback)."""
|
||||
pass_data = self._build_pass(test_loyalty_card)
|
||||
barcodes = pass_data["barcodes"]
|
||||
assert len(barcodes) == 2
|
||||
assert barcodes[0]["format"] == "PKBarcodeFormatCode128"
|
||||
assert barcodes[1]["format"] == "PKBarcodeFormatQR"
|
||||
|
||||
def test_barcodes_array_code128_matches_primary(self, db, test_loyalty_card):
|
||||
"""First entry in barcodes array matches the primary barcode."""
|
||||
pass_data = self._build_pass(test_loyalty_card)
|
||||
expected = test_loyalty_card.card_number.replace("-", "")
|
||||
assert pass_data["barcodes"][0]["message"] == expected
|
||||
assert pass_data["barcodes"][0]["altText"] == test_loyalty_card.card_number
|
||||
|
||||
def test_barcodes_array_qr_uses_qr_code_data(self, db, test_loyalty_card):
|
||||
"""QR fallback uses the qr_code_data token, not the card number."""
|
||||
pass_data = self._build_pass(test_loyalty_card)
|
||||
assert pass_data["barcodes"][1]["message"] == test_loyalty_card.qr_code_data
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Google Wallet Barcode Tests
|
||||
# =============================================================================
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
@pytest.mark.service
|
||||
class TestGoogleWalletBarcode:
|
||||
"""Tests for Google Wallet object barcode configuration (CODE_128)."""
|
||||
|
||||
def _build_object(self, card):
|
||||
from app.modules.loyalty.services.google_wallet_service import GoogleWalletService
|
||||
|
||||
service = GoogleWalletService()
|
||||
return service._build_object_data(card, f"test_issuer.loyalty_card_{card.id}")
|
||||
|
||||
def test_barcode_type_is_code128(self, db, test_loyalty_card):
|
||||
"""Barcode type must be CODE_128 for retail scanners."""
|
||||
obj = self._build_object(test_loyalty_card)
|
||||
assert obj["barcode"]["type"] == "CODE_128"
|
||||
|
||||
def test_barcode_value_uses_card_number_without_dashes(self, db, test_loyalty_card):
|
||||
"""Barcode value is card number with dashes stripped."""
|
||||
obj = self._build_object(test_loyalty_card)
|
||||
expected = test_loyalty_card.card_number.replace("-", "")
|
||||
assert obj["barcode"]["value"] == expected
|
||||
assert "-" not in obj["barcode"]["value"]
|
||||
|
||||
def test_barcode_alternate_text_shows_formatted_number(self, db, test_loyalty_card):
|
||||
"""alternateText displays the human-readable card number."""
|
||||
obj = self._build_object(test_loyalty_card)
|
||||
assert obj["barcode"]["alternateText"] == test_loyalty_card.card_number
|
||||
|
||||
def test_barcode_has_all_required_fields(self, db, test_loyalty_card):
|
||||
"""Barcode object contains type, value, and alternateText."""
|
||||
obj = self._build_object(test_loyalty_card)
|
||||
barcode = obj["barcode"]
|
||||
assert "type" in barcode
|
||||
assert "value" in barcode
|
||||
assert "alternateText" in barcode
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Module Migration Discovery Tests
|
||||
# =============================================================================
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestLoyaltyMigrationDiscovery:
|
||||
"""Tests for loyalty module migration auto-discovery."""
|
||||
|
||||
def test_loyalty_migrations_discovered(self):
|
||||
"""Loyalty module migrations are found by the discovery system."""
|
||||
from app.modules.migrations import discover_module_migrations
|
||||
|
||||
paths = discover_module_migrations()
|
||||
loyalty_paths = [p for p in paths if "loyalty" in str(p)]
|
||||
assert len(loyalty_paths) == 1
|
||||
assert loyalty_paths[0].exists()
|
||||
|
||||
def test_loyalty_migrations_in_all_paths(self):
|
||||
"""Loyalty migrations are included in get_all_migration_paths."""
|
||||
from app.modules.migrations import get_all_migration_paths
|
||||
|
||||
paths = get_all_migration_paths()
|
||||
path_strs = [str(p) for p in paths]
|
||||
assert any("loyalty" in p for p in path_strs)
|
||||
# Core migrations should still be first
|
||||
assert "alembic" in str(paths[0])
|
||||
|
||||
def test_loyalty_migration_files_exist(self):
|
||||
"""Loyalty migration version files exist in the module directory."""
|
||||
from app.modules.migrations import discover_module_migrations
|
||||
|
||||
paths = discover_module_migrations()
|
||||
loyalty_path = [p for p in paths if "loyalty" in str(p)][0]
|
||||
migration_files = list(loyalty_path.glob("loyalty_*.py"))
|
||||
assert len(migration_files) >= 2
|
||||
|
||||
def test_loyalty_migrations_follow_naming_convention(self):
|
||||
"""Loyalty migration files follow the loyalty_ prefix convention."""
|
||||
from app.modules.migrations import discover_module_migrations
|
||||
|
||||
paths = discover_module_migrations()
|
||||
loyalty_path = [p for p in paths if "loyalty" in str(p)][0]
|
||||
for f in loyalty_path.glob("*.py"):
|
||||
if f.name == "__init__.py":
|
||||
continue
|
||||
assert f.name.startswith("loyalty_"), f"{f.name} should start with 'loyalty_'"
|
||||
|
||||
Reference in New Issue
Block a user