fix(loyalty): replace broad exception handlers with specific types and rename onboarding service
- Replace `except Exception` with specific exception types in google_wallet_service.py (requests.RequestException, ValueError, etc.) and apple_wallet_service.py (httpx.HTTPError, OSError, ssl.SSLError) - Rename loyalty_onboarding.py -> loyalty_onboarding_service.py to match NAM-002 naming convention (+ test file + imports) - Add PasswordChangeResponse Pydantic model to user_account API, removing raw dict return and noqa suppression Resolves 12 EXC-003 + 1 NAM-002 architecture warnings in loyalty module. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -60,7 +60,7 @@ def _get_feature_provider():
|
|||||||
|
|
||||||
def _get_onboarding_provider():
|
def _get_onboarding_provider():
|
||||||
"""Lazy import of onboarding provider to avoid circular imports."""
|
"""Lazy import of onboarding provider to avoid circular imports."""
|
||||||
from app.modules.loyalty.services.loyalty_onboarding import (
|
from app.modules.loyalty.services.loyalty_onboarding_service import (
|
||||||
loyalty_onboarding_provider,
|
loyalty_onboarding_provider,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
@@ -417,9 +417,9 @@ class AppleWalletService:
|
|||||||
size = 29 * scale
|
size = 29 * scale
|
||||||
|
|
||||||
if program.logo_url:
|
if program.logo_url:
|
||||||
try:
|
import httpx
|
||||||
import httpx
|
|
||||||
|
|
||||||
|
try:
|
||||||
resp = httpx.get(program.logo_url, timeout=10, follow_redirects=True)
|
resp = httpx.get(program.logo_url, timeout=10, follow_redirects=True)
|
||||||
resp.raise_for_status()
|
resp.raise_for_status()
|
||||||
img = Image.open(io.BytesIO(resp.content))
|
img = Image.open(io.BytesIO(resp.content))
|
||||||
@@ -428,7 +428,7 @@ class AppleWalletService:
|
|||||||
buf = io.BytesIO()
|
buf = io.BytesIO()
|
||||||
img.save(buf, format="PNG")
|
img.save(buf, format="PNG")
|
||||||
return buf.getvalue()
|
return buf.getvalue()
|
||||||
except Exception:
|
except (httpx.HTTPError, OSError, ValueError):
|
||||||
logger.warning("Failed to fetch logo for icon, using fallback")
|
logger.warning("Failed to fetch logo for icon, using fallback")
|
||||||
|
|
||||||
# Fallback: colored square with initial
|
# Fallback: colored square with initial
|
||||||
@@ -463,9 +463,9 @@ class AppleWalletService:
|
|||||||
width, height = 160 * scale, 50 * scale
|
width, height = 160 * scale, 50 * scale
|
||||||
|
|
||||||
if program.logo_url:
|
if program.logo_url:
|
||||||
try:
|
import httpx
|
||||||
import httpx
|
|
||||||
|
|
||||||
|
try:
|
||||||
resp = httpx.get(program.logo_url, timeout=10, follow_redirects=True)
|
resp = httpx.get(program.logo_url, timeout=10, follow_redirects=True)
|
||||||
resp.raise_for_status()
|
resp.raise_for_status()
|
||||||
img = Image.open(io.BytesIO(resp.content))
|
img = Image.open(io.BytesIO(resp.content))
|
||||||
@@ -480,7 +480,7 @@ class AppleWalletService:
|
|||||||
buf = io.BytesIO()
|
buf = io.BytesIO()
|
||||||
canvas.save(buf, format="PNG")
|
canvas.save(buf, format="PNG")
|
||||||
return buf.getvalue()
|
return buf.getvalue()
|
||||||
except Exception:
|
except (httpx.HTTPError, OSError, ValueError):
|
||||||
logger.warning("Failed to fetch logo for pass logo, using fallback")
|
logger.warning("Failed to fetch logo for pass logo, using fallback")
|
||||||
|
|
||||||
# Fallback: colored rectangle with initial
|
# Fallback: colored rectangle with initial
|
||||||
@@ -719,7 +719,7 @@ class AppleWalletService:
|
|||||||
response.status_code,
|
response.status_code,
|
||||||
response.text,
|
response.text,
|
||||||
)
|
)
|
||||||
except Exception as exc: # noqa: BLE001
|
except (httpx.HTTPError, ssl.SSLError, OSError) as exc:
|
||||||
logger.error("APNs push error for token %s...: %s", push_token[:8], exc)
|
logger.error("APNs push error for token %s...: %s", push_token[:8], exc)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ import time
|
|||||||
from datetime import UTC, datetime, timedelta
|
from datetime import UTC, datetime, timedelta
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
|
import requests
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
|
|
||||||
from app.core.config import settings
|
from app.core.config import settings
|
||||||
@@ -141,7 +142,7 @@ class GoogleWalletService:
|
|||||||
|
|
||||||
except json.JSONDecodeError as exc:
|
except json.JSONDecodeError as exc:
|
||||||
result["errors"].append(f"Invalid JSON in service account file: {exc}")
|
result["errors"].append(f"Invalid JSON in service account file: {exc}")
|
||||||
except Exception as exc: # noqa: BLE001
|
except (OSError, ValueError) as exc:
|
||||||
result["errors"].append(f"Failed to load credentials: {exc}")
|
result["errors"].append(f"Failed to load credentials: {exc}")
|
||||||
|
|
||||||
return result
|
return result
|
||||||
@@ -182,9 +183,9 @@ class GoogleWalletService:
|
|||||||
settings.loyalty_google_service_account_json,
|
settings.loyalty_google_service_account_json,
|
||||||
)
|
)
|
||||||
return self._signer
|
return self._signer
|
||||||
except Exception as exc: # noqa: BLE001
|
except (ValueError, OSError, KeyError) as exc:
|
||||||
logger.error("Failed to create RSA signer: %s", exc)
|
logger.error("Failed to create RSA signer: %s", exc)
|
||||||
raise WalletIntegrationException("google", str(exc))
|
raise WalletIntegrationException("google", str(exc)) from exc
|
||||||
|
|
||||||
def _get_http_client(self):
|
def _get_http_client(self):
|
||||||
"""Get authenticated HTTP client."""
|
"""Get authenticated HTTP client."""
|
||||||
@@ -197,9 +198,9 @@ class GoogleWalletService:
|
|||||||
credentials = self._get_credentials()
|
credentials = self._get_credentials()
|
||||||
self._http_client = AuthorizedSession(credentials)
|
self._http_client = AuthorizedSession(credentials)
|
||||||
return self._http_client
|
return self._http_client
|
||||||
except Exception as exc: # noqa: BLE001
|
except (ValueError, TypeError, AttributeError) as exc:
|
||||||
logger.error("Failed to create Google HTTP client: %s", exc)
|
logger.error("Failed to create Google HTTP client: %s", exc)
|
||||||
raise WalletIntegrationException("google", str(exc))
|
raise WalletIntegrationException("google", str(exc)) from exc
|
||||||
|
|
||||||
# =========================================================================
|
# =========================================================================
|
||||||
# LoyaltyClass Operations (Program-level)
|
# LoyaltyClass Operations (Program-level)
|
||||||
@@ -283,9 +284,9 @@ class GoogleWalletService:
|
|||||||
)
|
)
|
||||||
except WalletIntegrationException:
|
except WalletIntegrationException:
|
||||||
raise
|
raise
|
||||||
except Exception as exc: # noqa: BLE001
|
except (requests.RequestException, ValueError, AttributeError) as exc:
|
||||||
logger.error("Failed to create Google Wallet class: %s", exc)
|
logger.error("Failed to create Google Wallet class: %s", exc)
|
||||||
raise WalletIntegrationException("google", str(exc))
|
raise WalletIntegrationException("google", str(exc)) from exc
|
||||||
|
|
||||||
@_retry_on_failure
|
@_retry_on_failure
|
||||||
def update_class(self, db: Session, program: LoyaltyProgram) -> None:
|
def update_class(self, db: Session, program: LoyaltyProgram) -> None:
|
||||||
@@ -317,7 +318,7 @@ class GoogleWalletService:
|
|||||||
program.google_class_id,
|
program.google_class_id,
|
||||||
response.status_code,
|
response.status_code,
|
||||||
)
|
)
|
||||||
except Exception as exc: # noqa: BLE001
|
except (requests.RequestException, ValueError, AttributeError) as exc:
|
||||||
logger.error("Failed to update Google Wallet class: %s", exc)
|
logger.error("Failed to update Google Wallet class: %s", exc)
|
||||||
|
|
||||||
# =========================================================================
|
# =========================================================================
|
||||||
@@ -375,9 +376,9 @@ class GoogleWalletService:
|
|||||||
)
|
)
|
||||||
except WalletIntegrationException:
|
except WalletIntegrationException:
|
||||||
raise
|
raise
|
||||||
except Exception as exc: # noqa: BLE001
|
except (requests.RequestException, ValueError, AttributeError) as exc:
|
||||||
logger.error("Failed to create Google Wallet object: %s", exc)
|
logger.error("Failed to create Google Wallet object: %s", exc)
|
||||||
raise WalletIntegrationException("google", str(exc))
|
raise WalletIntegrationException("google", str(exc)) from exc
|
||||||
|
|
||||||
@_retry_on_failure
|
@_retry_on_failure
|
||||||
def update_object(self, db: Session, card: LoyaltyCard) -> None:
|
def update_object(self, db: Session, card: LoyaltyCard) -> None:
|
||||||
@@ -405,7 +406,7 @@ class GoogleWalletService:
|
|||||||
card.google_object_id,
|
card.google_object_id,
|
||||||
response.status_code,
|
response.status_code,
|
||||||
)
|
)
|
||||||
except Exception as exc: # noqa: BLE001
|
except (requests.RequestException, ValueError, AttributeError) as exc:
|
||||||
logger.error("Failed to update Google Wallet object: %s", exc)
|
logger.error("Failed to update Google Wallet object: %s", exc)
|
||||||
|
|
||||||
def _build_object_data(
|
def _build_object_data(
|
||||||
@@ -506,11 +507,11 @@ class GoogleWalletService:
|
|||||||
db.commit()
|
db.commit()
|
||||||
|
|
||||||
return f"https://pay.google.com/gp/v/save/{token}"
|
return f"https://pay.google.com/gp/v/save/{token}"
|
||||||
except Exception as exc: # noqa: BLE001
|
except (AttributeError, ValueError, KeyError) as exc:
|
||||||
logger.error(
|
logger.error(
|
||||||
"Failed to generate Google Wallet save URL: %s", exc
|
"Failed to generate Google Wallet save URL: %s", exc
|
||||||
)
|
)
|
||||||
raise WalletIntegrationException("google", str(exc))
|
raise WalletIntegrationException("google", str(exc)) from exc
|
||||||
|
|
||||||
# =========================================================================
|
# =========================================================================
|
||||||
# Class Approval
|
# Class Approval
|
||||||
@@ -544,7 +545,7 @@ class GoogleWalletService:
|
|||||||
"program_name": data.get("programName"),
|
"program_name": data.get("programName"),
|
||||||
}
|
}
|
||||||
return None
|
return None
|
||||||
except Exception as exc: # noqa: BLE001
|
except (requests.RequestException, ValueError, AttributeError) as exc:
|
||||||
logger.error(
|
logger.error(
|
||||||
"Failed to get Google Wallet class status: %s", exc
|
"Failed to get Google Wallet class status: %s", exc
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
# app/modules/loyalty/services/loyalty_onboarding.py
|
# app/modules/loyalty/services/loyalty_onboarding_service.py
|
||||||
"""
|
"""
|
||||||
Onboarding provider for the loyalty module.
|
Onboarding provider for the loyalty module.
|
||||||
|
|
||||||
@@ -1,4 +1,4 @@
|
|||||||
# app/modules/loyalty/tests/unit/test_loyalty_onboarding.py
|
# app/modules/loyalty/tests/unit/test_loyalty_onboarding_service.py
|
||||||
"""Unit tests for LoyaltyOnboardingProvider."""
|
"""Unit tests for LoyaltyOnboardingProvider."""
|
||||||
|
|
||||||
import uuid
|
import uuid
|
||||||
@@ -6,7 +6,9 @@ import uuid
|
|||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
from app.modules.loyalty.models.loyalty_program import LoyaltyProgram, LoyaltyType
|
from app.modules.loyalty.models.loyalty_program import LoyaltyProgram, LoyaltyType
|
||||||
from app.modules.loyalty.services.loyalty_onboarding import LoyaltyOnboardingProvider
|
from app.modules.loyalty.services.loyalty_onboarding_service import (
|
||||||
|
LoyaltyOnboardingProvider,
|
||||||
|
)
|
||||||
from app.modules.tenancy.models import Merchant, Store, User
|
from app.modules.tenancy.models import Merchant, Store, User
|
||||||
|
|
||||||
|
|
||||||
@@ -15,6 +15,7 @@ from sqlalchemy.orm import Session
|
|||||||
from app.core.database import get_db
|
from app.core.database import get_db
|
||||||
from app.modules.tenancy.schemas.auth import UserContext
|
from app.modules.tenancy.schemas.auth import UserContext
|
||||||
from app.modules.tenancy.schemas.user_account import (
|
from app.modules.tenancy.schemas.user_account import (
|
||||||
|
PasswordChangeResponse,
|
||||||
UserAccountResponse,
|
UserAccountResponse,
|
||||||
UserAccountUpdate,
|
UserAccountUpdate,
|
||||||
UserPasswordChange,
|
UserPasswordChange,
|
||||||
@@ -49,7 +50,7 @@ def create_account_router(auth_dependency: Callable) -> APIRouter:
|
|||||||
db.commit()
|
db.commit()
|
||||||
return result
|
return result
|
||||||
|
|
||||||
@router.put("/password")
|
@router.put("/password", response_model=PasswordChangeResponse)
|
||||||
async def change_my_password(
|
async def change_my_password(
|
||||||
password_data: UserPasswordChange,
|
password_data: UserPasswordChange,
|
||||||
current_user: UserContext = Depends(auth_dependency),
|
current_user: UserContext = Depends(auth_dependency),
|
||||||
@@ -60,7 +61,7 @@ def create_account_router(auth_dependency: Callable) -> APIRouter:
|
|||||||
db, current_user.id, password_data.model_dump()
|
db, current_user.id, password_data.model_dump()
|
||||||
)
|
)
|
||||||
db.commit()
|
db.commit()
|
||||||
return {"message": "Password changed successfully"} # noqa: API001
|
return PasswordChangeResponse(message="Password changed successfully")
|
||||||
|
|
||||||
return router
|
return router
|
||||||
|
|
||||||
|
|||||||
@@ -56,3 +56,9 @@ class UserPasswordChange(BaseModel):
|
|||||||
if not any(char.isalpha() for char in v):
|
if not any(char.isalpha() for char in v):
|
||||||
raise ValueError("Password must contain at least one letter")
|
raise ValueError("Password must contain at least one letter")
|
||||||
return v
|
return v
|
||||||
|
|
||||||
|
|
||||||
|
class PasswordChangeResponse(BaseModel):
|
||||||
|
"""Response for a successful password change."""
|
||||||
|
|
||||||
|
message: str
|
||||||
|
|||||||
Reference in New Issue
Block a user