fix: renewal status check, int() safety, daily charge atomicity
- renewal.py: block renew/renewal-options for PENDING/DISABLED subscriptions (extend_subscription doesn't transition these to ACTIVE — user would pay for nothing) - autopay.py: wrap 2x bare int() card_id parsing in try/except - devices.py: wrap 2x bare int() device_count parsing in try/except - daily_subscription_service: atomic daily charge — subtract_user_balance, create_transaction, update_daily_charge_time all use commit=False, single db.commit() after all three succeed. Prevents re-charge on partial failure. - subscription.py: update_daily_charge_time accepts commit=False kwarg
This commit is contained in:
@@ -14,7 +14,7 @@ from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from app.config import settings
|
||||
from app.database.crud.tariff import get_tariff_by_id
|
||||
from app.database.models import PaymentMethod, User
|
||||
from app.database.models import PaymentMethod, SubscriptionStatus, User
|
||||
from app.services.pricing_engine import pricing_engine
|
||||
from app.services.subscription_renewal_service import (
|
||||
SubscriptionRenewalChargeError,
|
||||
@@ -47,6 +47,11 @@ async def get_renewal_options(
|
||||
if not subscription:
|
||||
return []
|
||||
|
||||
_non_renewable = {SubscriptionStatus.DISABLED.value, SubscriptionStatus.PENDING.value}
|
||||
_actual_status = getattr(subscription, 'actual_status', subscription.status)
|
||||
if _actual_status in _non_renewable:
|
||||
return []
|
||||
|
||||
# Determine available periods
|
||||
if subscription.tariff_id and subscription.tariff and subscription.tariff.period_prices:
|
||||
periods = sorted(int(k) for k in subscription.tariff.period_prices.keys())
|
||||
@@ -103,6 +108,15 @@ async def renew_subscription(
|
||||
status_code=status.HTTP_404_NOT_FOUND,
|
||||
detail='No subscription found',
|
||||
)
|
||||
|
||||
_non_renewable = {SubscriptionStatus.DISABLED.value, SubscriptionStatus.PENDING.value}
|
||||
_actual_status = getattr(subscription, 'actual_status', subscription.status)
|
||||
if _actual_status in _non_renewable:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail=f'Cannot renew subscription with status: {_actual_status}',
|
||||
)
|
||||
|
||||
if subscription.tariff_id and subscription.tariff and subscription.tariff.period_prices:
|
||||
available_periods = [int(p) for p in subscription.tariff.period_prices.keys()]
|
||||
else:
|
||||
|
||||
@@ -1981,6 +1981,8 @@ async def update_daily_charge_time(
|
||||
db: AsyncSession,
|
||||
subscription: Subscription,
|
||||
charge_time: datetime = None,
|
||||
*,
|
||||
commit: bool = True,
|
||||
) -> Subscription:
|
||||
"""Обновляет время последнего суточного списания и продлевает подписку на 1 день."""
|
||||
now = charge_time or datetime.now(UTC)
|
||||
@@ -1992,7 +1994,10 @@ async def update_daily_charge_time(
|
||||
subscription.end_date = new_end_date
|
||||
logger.info('📅 Продлена подписка до', subscription_id=subscription.id, new_end_date=new_end_date)
|
||||
|
||||
await db.commit()
|
||||
if commit:
|
||||
await db.commit()
|
||||
else:
|
||||
await db.flush()
|
||||
await db.refresh(subscription)
|
||||
|
||||
return subscription
|
||||
|
||||
@@ -188,7 +188,11 @@ async def handle_saved_cards_list(callback: types.CallbackQuery, db_user: User,
|
||||
|
||||
async def handle_unlink_card(callback: types.CallbackQuery, db_user: User, db: AsyncSession):
|
||||
texts = get_texts(db_user.language)
|
||||
card_id = int(callback.data.split('_')[-1])
|
||||
try:
|
||||
card_id = int(callback.data.split('_')[-1])
|
||||
except (ValueError, IndexError):
|
||||
await callback.answer(texts.t('INVALID_REQUEST', 'Invalid request'), show_alert=True)
|
||||
return
|
||||
|
||||
cards = await get_active_payment_methods_by_user(db, db_user.id)
|
||||
card = next((c for c in cards if c.id == card_id), None)
|
||||
@@ -224,7 +228,11 @@ async def handle_unlink_card(callback: types.CallbackQuery, db_user: User, db: A
|
||||
|
||||
async def handle_confirm_unlink(callback: types.CallbackQuery, db_user: User, db: AsyncSession):
|
||||
texts = get_texts(db_user.language)
|
||||
card_id = int(callback.data.split('_')[-1])
|
||||
try:
|
||||
card_id = int(callback.data.split('_')[-1])
|
||||
except (ValueError, IndexError):
|
||||
await callback.answer(texts.t('INVALID_REQUEST', 'Invalid request'), show_alert=True)
|
||||
return
|
||||
|
||||
success = await deactivate_payment_method(db, card_id, db_user.id)
|
||||
|
||||
|
||||
@@ -260,8 +260,12 @@ async def handle_change_devices(
|
||||
async def confirm_change_devices(
|
||||
callback: types.CallbackQuery, db_user: User, db: AsyncSession, state: FSMContext = None
|
||||
):
|
||||
new_devices_count = int(callback.data.split('_')[2])
|
||||
texts = get_texts(db_user.language)
|
||||
try:
|
||||
new_devices_count = int(callback.data.split('_')[2])
|
||||
except (ValueError, IndexError):
|
||||
await callback.answer(texts.t('INVALID_REQUEST', 'Invalid request'), show_alert=True)
|
||||
return
|
||||
subscription, sub_id = await _resolve_subscription(callback, db_user, db, state)
|
||||
if subscription is None:
|
||||
return
|
||||
@@ -524,11 +528,14 @@ async def execute_change_devices(
|
||||
callback: types.CallbackQuery, db_user: User, db: AsyncSession, state: FSMContext = None
|
||||
):
|
||||
callback_parts = callback.data.split('_')
|
||||
new_devices_count = int(callback_parts[3])
|
||||
texts = get_texts(db_user.language)
|
||||
try:
|
||||
new_devices_count = int(callback_parts[3])
|
||||
except (ValueError, IndexError):
|
||||
await callback.answer(texts.t('INVALID_REQUEST', 'Invalid request'), show_alert=True)
|
||||
return
|
||||
|
||||
db_user = await lock_user_for_pricing(db, db_user.id)
|
||||
|
||||
texts = get_texts(db_user.language)
|
||||
# Re-resolve after lock since db_user was refreshed
|
||||
subscription, _ = await _resolve_subscription(callback, db_user, db, state)
|
||||
if not subscription:
|
||||
|
||||
@@ -93,7 +93,6 @@ class DailySubscriptionService:
|
||||
exc_info=True,
|
||||
)
|
||||
stats['errors'] += 1
|
||||
await db.commit()
|
||||
except Exception as e:
|
||||
logger.error('Ошибка при обработке подписок', error=e, exc_info=True)
|
||||
await db.rollback()
|
||||
@@ -163,19 +162,22 @@ class DailySubscriptionService:
|
||||
description = f'Суточная оплата тарифа «{tariff.name}»'
|
||||
|
||||
try:
|
||||
# commit=False для атомарности: баланс, транзакция и charge_time коммитятся вместе
|
||||
deducted = await subtract_user_balance(
|
||||
db,
|
||||
user,
|
||||
daily_price,
|
||||
description,
|
||||
mark_as_paid_subscription=True,
|
||||
commit=False,
|
||||
)
|
||||
|
||||
if not deducted:
|
||||
await db.rollback()
|
||||
logger.warning('Не удалось списать средства для подписки', subscription_id=subscription.id)
|
||||
return 'error'
|
||||
|
||||
# Создаём транзакцию
|
||||
# Создаём транзакцию (без коммита — часть атомарной операции)
|
||||
transaction = await create_transaction(
|
||||
db=db,
|
||||
user_id=user.id,
|
||||
@@ -183,11 +185,16 @@ class DailySubscriptionService:
|
||||
amount_kopeks=daily_price,
|
||||
description=description,
|
||||
payment_method=PaymentMethod.BALANCE,
|
||||
commit=False,
|
||||
)
|
||||
|
||||
# Обновляем время последнего списания и продлеваем подписку
|
||||
# Обновляем время последнего списания и продлеваем подписку (без коммита)
|
||||
old_end_date = subscription.end_date
|
||||
subscription = await update_daily_charge_time(db, subscription)
|
||||
subscription = await update_daily_charge_time(db, subscription, commit=False)
|
||||
|
||||
# Атомарный коммит: баланс + транзакция + charge_time
|
||||
await db.commit()
|
||||
await db.refresh(user)
|
||||
|
||||
user_id_display = user.telegram_id or user.email or f'#{user.id}'
|
||||
logger.info(
|
||||
@@ -284,6 +291,7 @@ class DailySubscriptionService:
|
||||
return 'charged'
|
||||
|
||||
except Exception as e:
|
||||
await db.rollback()
|
||||
logger.error(
|
||||
'Ошибка при списании средств для подписки', subscription_id=subscription.id, error=e, exc_info=True
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user