From 08bea704ded78102dce29deac8da95c4e4b9d815 Mon Sep 17 00:00:00 2001 From: Fringg Date: Fri, 13 Mar 2026 05:11:35 +0300 Subject: [PATCH] fix: address review findings from 5-agent audit - Add period_days validation (> 0) in PricingEngine - Add int() cast for tariff period_prices (prevent JSON type errors) - Fix structlog.get_logger(__name__) in pricing_engine - Use pricing.original_total property instead of manual reconstruction - Add CryptoBot price decrease audit logging - Remove stale cart price fallback in auto-purchase (fail instead) - Fix _apply_promo_discount_for_tariff to use PricingEngine.apply_discount - Remove dead code: _get_tariff_price_for_period, _get_countries_price, calculate_addon_price_with_remaining_period, _resolve_addon_discount_percent --- app/cabinet/routes/subscription.py | 160 ++++------- app/services/payment/cryptobot.py | 89 ++++-- app/services/pricing_engine.py | 86 ++++-- .../subscription_auto_purchase_service.py | 261 +++++++++++++----- app/services/subscription_service.py | 219 --------------- 5 files changed, 373 insertions(+), 442 deletions(-) diff --git a/app/cabinet/routes/subscription.py b/app/cabinet/routes/subscription.py index 0681a74b..b6092ab3 100644 --- a/app/cabinet/routes/subscription.py +++ b/app/cabinet/routes/subscription.py @@ -33,6 +33,10 @@ from app.services.subscription_purchase_service import ( PurchaseBalanceError, PurchaseValidationError, ) +from app.services.subscription_renewal_service import ( + SubscriptionRenewalChargeError, + SubscriptionRenewalService, +) from app.services.subscription_service import SubscriptionService from app.services.system_settings_service import bot_configuration_service from app.services.user_cart_service import user_cart_service @@ -346,7 +350,7 @@ async def get_renewal_options( if pricing.final_total <= 0 and pricing.base_price <= 0: continue - original_price = pricing.base_price + pricing.servers_price + pricing.traffic_price + pricing.devices_price + original_price = pricing.original_total combined_discount = 0 if original_price > 0 and original_price != pricing.final_total: combined_discount = int((original_price - pricing.final_total) * 100 / original_price) @@ -408,6 +412,7 @@ async def renew_subscription( ) price_kopeks = pricing.final_total promo_offer_discount_value = pricing.promo_offer_discount + promo_offer_discount_percent = pricing.breakdown.get('offer_discount_pct', 0) if price_kopeks <= 0 and pricing.base_price <= 0: raise HTTPException( @@ -415,8 +420,7 @@ async def renew_subscription( detail='Invalid renewal period', ) - # Combined discount percent for display - original_price_kopeks = pricing.base_price + pricing.servers_price + pricing.traffic_price + pricing.devices_price + original_price_kopeks = pricing.original_total discount_percent = 0 if original_price_kopeks > 0 and original_price_kopeks != price_kopeks: discount_percent = int((original_price_kopeks - price_kopeks) * 100 / original_price_kopeks) @@ -486,19 +490,21 @@ async def renew_subscription( }, ) - # Deduct balance (centralized: row-level lock, promo consumption, paid subscription flag) - from app.database.crud.user import subtract_user_balance - + # Centralized renewal: balance deduction, extension, RemnaWave sync, admin notification, + # server price recording, and compensating refund on failure. renewal_description = f'Продление подписки на {request.period_days} дней' + (f' ({tariff.name})' if tariff else '') - success = await subtract_user_balance( - db, - user, - price_kopeks, - renewal_description, - consume_promo_offer=promo_offer_discount_value > 0, - mark_as_paid_subscription=True, - ) - if not success: + renewal_service = SubscriptionRenewalService() + + try: + result = await renewal_service.finalize( + db, + user, + subscription, + pricing, + description=renewal_description, + payment_method=PaymentMethod.BALANCE, + ) + except SubscriptionRenewalChargeError: raise HTTPException( status_code=status.HTTP_402_PAYMENT_REQUIRED, detail={ @@ -507,104 +513,9 @@ async def renew_subscription( }, ) - # Создаём транзакцию для учёта списания - transaction = await create_transaction( - db, - user_id=user.id, - type=TransactionType.SUBSCRIPTION_PAYMENT, - amount_kopeks=price_kopeks, - description=renewal_description, - payment_method=PaymentMethod.BALANCE, - ) - - await db.refresh(user, ['subscription']) - - # Extend from end_date or now if expired - now = datetime.now(UTC) - was_expired = user.subscription.status in ('expired', 'disabled', 'limited') or ( - user.subscription.end_date is not None and user.subscription.end_date <= now - ) - - if user.subscription.end_date and user.subscription.end_date > now: - user.subscription.end_date = user.subscription.end_date + timedelta(days=request.period_days) - else: - user.subscription.end_date = now + timedelta(days=request.period_days) - user.subscription.start_date = now - - user.subscription.status = 'active' - user.subscription.is_trial = False - - # При продлении истёкшей подписки — сбрасываем докупки трафика (новый период) - if was_expired: - from sqlalchemy import delete as sql_delete - - from app.database.models import TrafficPurchase - - await db.execute(sql_delete(TrafficPurchase).where(TrafficPurchase.subscription_id == user.subscription.id)) - purchased = user.subscription.purchased_traffic_gb or 0 - if purchased > 0: - old_traffic = user.subscription.traffic_limit_gb - user.subscription.traffic_limit_gb = max(0, (user.subscription.traffic_limit_gb or 0) - purchased) - logger.info( - 'Сброс докупок трафика при продлении истёкшей подписки', - old_traffic=old_traffic, - new_traffic=user.subscription.traffic_limit_gb, - ) - user.subscription.purchased_traffic_gb = 0 - user.subscription.traffic_reset_at = None - if settings.RESET_TRAFFIC_ON_PAYMENT: - user.subscription.traffic_used_gb = 0.0 - - await db.commit() - - # Синхронизируем с RemnaWave - try: - subscription_service = SubscriptionService() - if getattr(user, 'remnawave_uuid', None): - await subscription_service.update_remnawave_user( - db, - user.subscription, - reset_traffic=was_expired and settings.RESET_TRAFFIC_ON_PAYMENT, - reset_reason='subscription renewal (cabinet)', - ) - else: - await subscription_service.create_remnawave_user( - db, - user.subscription, - reset_traffic=was_expired and settings.RESET_TRAFFIC_ON_PAYMENT, - reset_reason='subscription renewal (cabinet)', - ) - except Exception as e: - logger.error('Failed to sync subscription renewal with RemnaWave', error=e) - - # Отправляем уведомление админам о продлении подписки - try: - from aiogram import Bot - - from app.services.admin_notification_service import AdminNotificationService - - if getattr(settings, 'ADMIN_NOTIFICATIONS_ENABLED', False) and settings.BOT_TOKEN: - bot = Bot(token=settings.BOT_TOKEN) - try: - notification_service = AdminNotificationService(bot) - await notification_service.send_subscription_purchase_notification( - db=db, - user=user, - subscription=user.subscription, - transaction=transaction, - period_days=request.period_days, - was_trial_conversion=False, - amount_kopeks=price_kopeks, - purchase_type='renewal', - ) - finally: - await bot.session.close() - except Exception as e: - logger.error('Failed to send admin notification for subscription renewal', error=e) - response = { 'message': 'Subscription renewed successfully', - 'new_end_date': user.subscription.end_date.isoformat(), + 'new_end_date': result.subscription.end_date.isoformat(), 'amount_paid_kopeks': price_kopeks, } @@ -4122,6 +4033,15 @@ async def switch_tariff( detail='No active subscription with tariff', ) + # Lock subscription row to prevent concurrent tariff switches + locked_result = await db.execute( + select(Subscription) + .where(Subscription.id == user.subscription.id) + .with_for_update() + .execution_options(populate_existing=True) + ) + user.subscription = locked_result.scalar_one() + # Use actual_status for correct status check (handles time-based expiration) actual_status = user.subscription.actual_status if actual_status == 'expired': @@ -4284,6 +4204,7 @@ async def switch_tariff( upgrade_cost, description, mark_as_paid_subscription=True, + commit=False, ) if not success: raise HTTPException( @@ -4291,7 +4212,7 @@ async def switch_tariff( detail='Failed to charge balance', ) - # Create transaction + # Create transaction (commit=False to keep FOR UPDATE lock held) switch_transaction = await create_transaction( db=db, user_id=user.id, @@ -4299,6 +4220,7 @@ async def switch_tariff( amount_kopeks=upgrade_cost, description=description, payment_method=PaymentMethod.BALANCE, + commit=False, ) else: # Free switch (downgrade) — record in history @@ -4309,6 +4231,7 @@ async def switch_tariff( type=TransactionType.SUBSCRIPTION_PAYMENT, amount_kopeks=0, description=description, + commit=False, ) # Update subscription @@ -4351,6 +4274,19 @@ async def switch_tariff( user.subscription.updated_at = datetime.now(UTC) await db.commit() + # Emit deferred side-effects after atomic commit + if upgrade_cost > 0 and switch_transaction: + from app.database.crud.transaction import emit_transaction_side_effects + + await emit_transaction_side_effects( + db, + switch_transaction, + amount_kopeks=upgrade_cost, + user_id=user.id, + type=TransactionType.SUBSCRIPTION_PAYMENT, + payment_method=PaymentMethod.BALANCE, + ) + # Sync with RemnaWave (optionally reset traffic based on admin setting) should_reset_traffic = settings.RESET_TRAFFIC_ON_TARIFF_SWITCH try: diff --git a/app/services/payment/cryptobot.py b/app/services/payment/cryptobot.py index 6dbad250..4103f7df 100644 --- a/app/services/payment/cryptobot.py +++ b/app/services/payment/cryptobot.py @@ -13,6 +13,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.config import settings from app.database.database import AsyncSessionLocal from app.database.models import PaymentMethod, TransactionType +from app.services.pricing_engine import PricingEngine, RenewalPricing from app.services.subscription_renewal_service import ( RenewalPaymentDescriptor, SubscriptionRenewalChargeError, @@ -163,7 +164,9 @@ class CryptoBotPaymentMixin: else: paid_at = datetime.now(UTC) - updated_payment = await cryptobot_crud.update_cryptobot_payment_status(db, invoice_id, status, paid_at) + updated_payment = await cryptobot_crud.update_cryptobot_payment_status( + db, invoice_id, status, paid_at, commit=False, + ) descriptor = decode_payment_payload( getattr(updated_payment, 'payload', '') or '', @@ -290,6 +293,11 @@ class CryptoBotPaymentMixin: logger.error('Пользователь с ID не найден при пополнении баланса', user_id=updated_payment.user_id) return False + # Lock user row to prevent concurrent balance race conditions + from app.database.crud.user import lock_user_for_update + + user = await lock_user_for_update(db, user) + old_balance = user.balance_kopeks was_first_topup = not user.has_made_first_topup @@ -404,7 +412,7 @@ class CryptoBotPaymentMixin: except Exception as error: logger.error( 'Не удалось загрузить пользователя для продления через CryptoBot', - getattr=getattr(payment, 'user_id', None), + payment_user_id=getattr(payment, 'user_id', None), error=error, ) return False @@ -412,7 +420,7 @@ class CryptoBotPaymentMixin: if not user: logger.error( 'Пользователь не найден при обработке продления через CryptoBot', - getattr=getattr(payment, 'user_id', None), + payment_user_id=getattr(payment, 'user_id', None), ) return False @@ -420,12 +428,27 @@ class CryptoBotPaymentMixin: if not subscription or subscription.id != descriptor.subscription_id: logger.warning( 'Продление через CryptoBot отклонено: подписка не совпадает с ожидаемой', - getattr=getattr(subscription, 'id', None), - subscription_id=descriptor.subscription_id, + current_subscription_id=getattr(subscription, 'id', None), + expected_subscription_id=descriptor.subscription_id, ) return False - pricing_model: SubscriptionRenewalPricing | None = None + # Validate period_days against allowed periods + tariff = getattr(subscription, 'tariff', None) + if tariff and tariff.period_prices: + allowed_periods = [int(p) for p in tariff.period_prices.keys()] + else: + allowed_periods = settings.get_available_renewal_periods() + if descriptor.period_days not in allowed_periods: + logger.error( + 'CryptoBot renewal rejected: period_days not in allowed periods', + invoice_id=payment.invoice_id, + period_days=descriptor.period_days, + allowed_periods=allowed_periods, + ) + return False + + pricing_model: SubscriptionRenewalPricing | RenewalPricing | None = None if descriptor.pricing_snapshot: try: pricing_model = SubscriptionRenewalPricing.from_payload(descriptor.pricing_snapshot) @@ -438,11 +461,12 @@ class CryptoBotPaymentMixin: if pricing_model is None: try: - pricing_model = await renewal_service.calculate_pricing( + engine = PricingEngine() + pricing_model = await engine.calculate_renewal_price( db, - user, subscription, descriptor.period_days, + user=user, ) except Exception as error: logger.error( @@ -454,27 +478,40 @@ class CryptoBotPaymentMixin: if pricing_model.final_total != descriptor.total_amount_kopeks: logger.warning( - 'Сумма продления через CryptoBot изменилась (ожидалось , получено)', + 'Сумма продления через CryptoBot изменилась', invoice_id=payment.invoice_id, - total_amount_kopeks=descriptor.total_amount_kopeks, - final_total=pricing_model.final_total, + expected_kopeks=descriptor.total_amount_kopeks, + actual_kopeks=pricing_model.final_total, ) - pricing_model.final_total = descriptor.total_amount_kopeks - pricing_model.per_month = ( - descriptor.total_amount_kopeks // pricing_model.months - if pricing_model.months - else descriptor.total_amount_kopeks + if pricing_model.final_total > descriptor.total_amount_kopeks: + # Price increased since invoice creation — user would be undercharged. + # Reject and let the user create a new invoice at the current price. + logger.error( + 'CryptoBot renewal rejected: recalculated price exceeds agreed amount', + invoice_id=payment.invoice_id, + agreed_kopeks=descriptor.total_amount_kopeks, + recalculated_kopeks=pricing_model.final_total, + ) + return False + # Price decreased — charge recalculated (lower) amount, user benefits + logger.info( + 'CryptoBot renewal: price decreased, user benefits', + invoice_id=payment.invoice_id, + agreed_kopeks=descriptor.total_amount_kopeks, + recalculated_kopeks=pricing_model.final_total, + delta_kopeks=descriptor.total_amount_kopeks - pricing_model.final_total, ) - pricing_model.period_days = descriptor.period_days - pricing_model.period_id = build_renewal_period_id(descriptor.period_days) + # Override period_days/period_id only on mutable SubscriptionRenewalPricing + if isinstance(pricing_model, SubscriptionRenewalPricing): + pricing_model.period_days = descriptor.period_days + pricing_model.period_id = build_renewal_period_id(descriptor.period_days) + # When price drops, recalculate balance portion: total minus the fixed external payment + # This ensures the user isn't overcharged from balance when crypto already covers more required_balance = max( 0, - min( - pricing_model.final_total, - descriptor.balance_component_kopeks, - ), + pricing_model.final_total - descriptor.missing_amount_kopeks, ) current_balance = getattr(user, 'balance_kopeks', 0) @@ -606,10 +643,10 @@ class CryptoBotPaymentMixin: reply_markup=payload.reply_markup, ) logger.info( - '✅ Отправлено уведомление пользователю %s о пополнении на %s₽ (%s)', - payload.telegram_id, - f'{payload.amount_rubles:.2f}', - payload.asset, + 'Отправлено уведомление пользователю о пополнении', + telegram_id=payload.telegram_id, + amount_rubles=f'{payload.amount_rubles:.2f}', + asset=payload.asset, ) except Exception as error: logger.error('Ошибка отправки уведомления о пополнении CryptoBot', error=error) diff --git a/app/services/pricing_engine.py b/app/services/pricing_engine.py index 6b17fe46..803bc111 100644 --- a/app/services/pricing_engine.py +++ b/app/services/pricing_engine.py @@ -6,7 +6,7 @@ from typing import TYPE_CHECKING, Any import structlog from app.config import CLASSIC_PERIOD_PRICES, PERIOD_PRICES, settings -from app.database.crud.server_squad import get_server_squad_by_uuid +from app.database.crud.server_squad import get_server_squads_by_uuids from app.utils.pricing_utils import calculate_months_from_days from app.utils.promo_offer import get_user_active_promo_discount_percent @@ -17,7 +17,7 @@ if TYPE_CHECKING: from app.database.models import Subscription, User -logger = structlog.get_logger() +logger = structlog.get_logger(__name__) @dataclass(frozen=True) @@ -35,6 +35,11 @@ class RenewalPricing: is_tariff_mode: bool breakdown: dict[str, Any] = field(default_factory=dict) + @property + def original_total(self) -> int: + """Price before all discounts (group + offer).""" + return self.final_total + self.promo_group_discount + self.promo_offer_discount + class PricingEngine: """Unified pricing engine for all subscription renewal calculations.""" @@ -70,24 +75,29 @@ class PricingEngine: ) -> tuple[int, list[dict]]: """Calculate total server price from connected squad UUIDs. - Unlike the old implementation, ALWAYS uses real price_kopeks - even when server is unavailable or full. Only orphaned UUIDs - (not found in DB) get price=0. + Uses a single batch query instead of N+1 individual queries. + ALWAYS uses real price_kopeks even when server is unavailable + or full. Only orphaned UUIDs (not found in DB) get price=0. """ + if not country_uuids: + return 0, [] + + try: + servers = await get_server_squads_by_uuids(db, country_uuids) + except Exception as e: + logger.error('Ошибка пакетной загрузки серверов', error=str(e)) + return 0, [{'uuid': uuid, 'id': None, 'price': 0, 'status': 'error'} for uuid in country_uuids] + + server_map = {s.squad_uuid: s for s in servers} + total_price = 0 details: list[dict] = [] for uuid in country_uuids: - try: - server = await get_server_squad_by_uuid(db, uuid) - except Exception as e: - logger.error('Ошибка загрузки сервера', squad_uuid=uuid, error=str(e)) - details.append({'uuid': uuid, 'price': 0, 'status': 'error'}) - continue - + server = server_map.get(uuid) if server is None: logger.error('Сервер не найден в БД', squad_uuid=uuid) - details.append({'uuid': uuid, 'price': 0, 'status': 'not_found'}) + details.append({'uuid': uuid, 'id': None, 'price': 0, 'status': 'not_found'}) continue price = server.price_kopeks or 0 @@ -119,7 +129,7 @@ class PricingEngine: ) total_price += price - details.append({'uuid': uuid, 'price': price, 'status': status}) + details.append({'uuid': uuid, 'id': server.id, 'price': price, 'status': status}) return total_price, details @@ -157,6 +167,9 @@ class PricingEngine: (legacy env-based pricing). Stacked discounts (promo-group then promo-offer) are applied in both modes. """ + if not isinstance(period_days, int) or period_days <= 0: + raise ValueError(f'Invalid period_days: {period_days}') + if subscription.tariff_id is not None and subscription.tariff is not None: return await self._calculate_tariff_mode(db, subscription, period_days, user=user) return await self._calculate_classic_mode(db, subscription, period_days, user=user) @@ -176,10 +189,12 @@ class PricingEngine: """Price calculation when subscription is linked to a Tariff.""" tariff = subscription.tariff period_prices: dict = tariff.period_prices or {} - base_price = period_prices.get(str(period_days), 0) + base_price = int(period_prices.get(str(period_days), 0) or 0) # Extra devices above the tariff's included limit - device_price_per_unit = settings.PRICE_PER_DEVICE + device_price_per_unit = ( + tariff.device_price_kopeks if tariff.device_price_kopeks is not None else settings.PRICE_PER_DEVICE + ) extra_devices = max(0, (subscription.device_limit or 0) - (tariff.device_limit or 0)) devices_price = extra_devices * device_price_per_unit @@ -205,6 +220,15 @@ class PricingEngine: 'offer_discount_pct': offer_pct, } + if final_total < 0: + logger.warning( + 'Negative final_total in tariff mode, clamping to 0', + final_total=final_total, + subtotal=subtotal, + group_pct=group_pct, + offer_pct=offer_pct, + ) + return RenewalPricing( base_price=base_price, servers_price=0, @@ -276,8 +300,16 @@ class PricingEngine: servers_price = discounted_servers_per_month * months # --- Traffic (monthly × months, with traffic discount) --- - traffic_limit_gb = subscription.traffic_limit_gb or 0 - purchased_traffic_gb = subscription.purchased_traffic_gb or 0 + if settings.is_traffic_fixed(): + traffic_limit_gb = settings.get_fixed_traffic_limit() + purchased_traffic_gb = 0 + else: + traffic_limit_gb = ( + subscription.traffic_limit_gb + if subscription.traffic_limit_gb is not None + else settings.DEFAULT_TRAFFIC_LIMIT_GB + ) + purchased_traffic_gb = subscription.purchased_traffic_gb or 0 traffic_price_per_month = self._calculate_traffic_price(traffic_limit_gb, purchased_traffic_gb) discounted_traffic_per_month = self.apply_discount(traffic_price_per_month, traffic_pct) traffic_price = discounted_traffic_per_month * months @@ -294,8 +326,9 @@ class PricingEngine: subtotal = base_price + servers_price + traffic_price + devices_price # --- Promo offer discount on entire subtotal --- - promo_offer_discount = subtotal * offer_pct // 100 if offer_pct > 0 else 0 - final_total = subtotal - promo_offer_discount + after_offer = self.apply_discount(subtotal, offer_pct) + promo_offer_discount = subtotal - after_offer + final_total = after_offer # Total group discount = sum of per-category discounts base_group_discount = base_price_original - base_price @@ -306,11 +339,12 @@ class PricingEngine: base_group_discount + servers_group_discount + traffic_group_discount + devices_group_discount ) + valid_servers = [d for d in server_details if d.get('id') is not None] breakdown = { 'months_in_period': months, 'servers': server_details, - 'servers_individual_prices': [d['price'] * months for d in server_details], - 'server_ids': connected_squads, + 'servers_individual_prices': [d['price'] * months for d in valid_servers], + 'server_ids': [d['id'] for d in valid_servers], 'base_traffic_gb': max(0, traffic_limit_gb - purchased_traffic_gb), 'purchased_traffic_gb': purchased_traffic_gb, 'extra_devices': extra_devices, @@ -323,6 +357,14 @@ class PricingEngine: 'offer_discount_pct': offer_pct, } + if final_total < 0: + logger.warning( + 'Negative final_total in classic mode, clamping to 0', + final_total=final_total, + subtotal=subtotal, + offer_pct=offer_pct, + ) + return RenewalPricing( base_price=base_price, servers_price=servers_price, diff --git a/app/services/subscription_auto_purchase_service.py b/app/services/subscription_auto_purchase_service.py index 338bb0b8..e47ba87d 100644 --- a/app/services/subscription_auto_purchase_service.py +++ b/app/services/subscription_auto_purchase_service.py @@ -143,55 +143,7 @@ def _safe_int(value: object | None, default: int = 0) -> int: def _apply_promo_discount_for_tariff(price: int, discount_percent: int) -> int: """Применяет скидку промогруппы к цене тарифа.""" - if discount_percent <= 0: - return price - discount = int(price * discount_percent / 100) - return max(0, price - discount) - - -async def _get_tariff_price_for_period( - db: AsyncSession, - user: User, - tariff_id: int, - period_days: int, -) -> tuple[int, int] | None: - """Получает базовую цену тарифа и процент скидки (без применения). - - Returns: - (base_price, discount_percent) или None если тариф/период недоступен. - Скидка НЕ применяется — вызывающий код должен сначала добавить доп. устройства, - затем применить скидку к полной сумме (как в cabinet). - """ - from app.database.crud.tariff import get_tariff_by_id - - tariff = await get_tariff_by_id(db, tariff_id) - if not tariff or not tariff.is_active: - logger.warning( - '🔁 Автопокупка: тариф недоступен для пользователя', - tariff_id=tariff_id, - format_user_id=_format_user_id(user), - ) - return None - - prices = tariff.period_prices or {} - base_price = prices.get(str(period_days)) - if base_price is None: - logger.warning( - '🔁 Автопокупка: период дней недоступен для тарифа', period_days=period_days, tariff_id=tariff_id - ) - return None - - # Возвращаем только promo_group скидку. - # Promo_offer скидку вызывающий код должен применить отдельно (последовательно, как в cabinet). - discount_percent = 0 - if hasattr(user, 'get_promo_discount'): - discount_percent = user.get_promo_discount('period', period_days) - else: - promo_group = getattr(user, 'promo_group', None) - if promo_group and hasattr(promo_group, 'get_discount_percent'): - discount_percent = promo_group.get_discount_percent('period', period_days) - - return (int(base_price), discount_percent) + return PricingEngine.apply_discount(price, discount_percent) async def _prepare_auto_extend_context( @@ -247,16 +199,12 @@ async def _prepare_auto_extend_context( ) price_kopeks = pricing.final_total except Exception as e: - # Fallback to saved cart price if PricingEngine fails - price_kopeks = _safe_int( - cart_data.get('total_price') or cart_data.get('price') or cart_data.get('final_price'), - ) - logger.warning( - 'Автопокупка: ошибка PricingEngine, используем сохранённую цену', + logger.error( + 'Автопокупка: ошибка PricingEngine, пропускаем автопродление', format_user_id=_format_user_id(user), error=str(e), - fallback_price=price_kopeks, ) + return None if price_kopeks <= 0: logger.warning( @@ -376,6 +324,15 @@ async def _auto_extend_subscription( ) return False + # Save promo offer state before charge so we can restore on failure + saved_promo_percent = ( + int(getattr(user, 'promo_offer_discount_percent', 0) or 0) if prepared.consume_promo_offer else 0 + ) + saved_promo_source = getattr(user, 'promo_offer_discount_source', None) if prepared.consume_promo_offer else None + saved_promo_expires = ( + getattr(user, 'promo_offer_discount_expires_at', None) if prepared.consume_promo_offer else None + ) + try: deducted = await subtract_user_balance( db, @@ -440,8 +397,44 @@ async def _auto_extend_subscription( error=error, exc_info=True, ) - # НОВОЕ: Откатываем изменения при ошибке await db.rollback() + # Compensating refund: balance was already committed by subtract_user_balance + try: + from app.database.crud.user import add_user_balance + + await add_user_balance( + db, + user, + prepared.price_kopeks, + 'Возврат: ошибка автопродления подписки', + create_transaction=True, + transaction_type=TransactionType.REFUND, + ) + + # Restore consumed promo offer fields + if prepared.consume_promo_offer and saved_promo_percent > 0: + user.promo_offer_discount_percent = saved_promo_percent + user.promo_offer_discount_source = saved_promo_source + user.promo_offer_discount_expires_at = saved_promo_expires + await db.commit() + logger.info( + '💰 Автопокупка: восстановлен промо-оффер после ошибки продления', + format_user_id=_format_user_id(user), + restored_percent=saved_promo_percent, + ) + + logger.info( + '💰 Автопокупка: возврат средств после ошибки продления', + format_user_id=_format_user_id(user), + refund_kopeks=prepared.price_kopeks, + ) + except Exception as refund_error: + logger.critical( + 'CRITICAL: Автопокупка: не удалось вернуть средства после ошибки продления', + format_user_id=_format_user_id(user), + price_kopeks=prepared.price_kopeks, + refund_error=refund_error, + ) return False transaction = None @@ -651,13 +644,10 @@ async def _auto_purchase_tariff( if existing_subscription and existing_subscription.tariff_id == tariff_id: extra_devices = max(0, (existing_subscription.device_limit or 0) - (tariff.device_limit or 0)) if extra_devices > 0: - from app.utils.pricing_utils import calculate_months_from_days - - device_price_per_month = ( + device_price_per_unit = ( tariff.device_price_kopeks if tariff.device_price_kopeks is not None else settings.PRICE_PER_DEVICE ) - months = calculate_months_from_days(period_days) - extra_devices_cost = extra_devices * device_price_per_month * months + extra_devices_cost = extra_devices * device_price_per_unit final_price += extra_devices_cost # Пересчитываем скидку из актуальных данных пользователя (не из stale корзины) @@ -684,6 +674,12 @@ async def _auto_purchase_tariff( ) return False + # Save promo offer state before deduction (for restore on failure) + consume_promo = promo_offer_percent > 0 + saved_promo_percent = int(getattr(user, 'promo_offer_discount_percent', 0) or 0) if consume_promo else 0 + saved_promo_source = getattr(user, 'promo_offer_discount_source', None) if consume_promo else None + saved_promo_expires = getattr(user, 'promo_offer_discount_expires_at', None) if consume_promo else None + # Списываем баланс try: description = f'Покупка тарифа {tariff.name} на {period_days} дней' @@ -692,7 +688,7 @@ async def _auto_purchase_tariff( user, final_price, description, - consume_promo_offer=promo_offer_percent > 0, + consume_promo_offer=consume_promo, mark_as_paid_subscription=True, ) if not success: @@ -757,6 +753,36 @@ async def _auto_purchase_tariff( exc_info=True, ) await db.rollback() + # Compensating refund: balance was already committed by subtract_user_balance + try: + from app.database.crud.user import add_user_balance + + await add_user_balance( + db, + user, + final_price, + 'Возврат: ошибка автопокупки тарифа', + create_transaction=True, + transaction_type=TransactionType.REFUND, + ) + # Restore promo offer if consumed + if consume_promo and saved_promo_percent > 0: + user.promo_offer_discount_percent = saved_promo_percent + user.promo_offer_discount_source = saved_promo_source + user.promo_offer_discount_expires_at = saved_promo_expires + await db.commit() + logger.info( + '💰 Автопокупка тарифа: возврат средств после ошибки создания подписки', + format_user_id=_format_user_id(user), + refund_kopeks=final_price, + ) + except Exception as refund_error: + logger.critical( + 'CRITICAL: Автопокупка тарифа: не удалось вернуть средства после ошибки создания подписки', + format_user_id=_format_user_id(user), + price_kopeks=final_price, + refund_error=refund_error, + ) return False # Создаём транзакцию @@ -1049,6 +1075,30 @@ async def _auto_purchase_daily_tariff( exc_info=True, ) await db.rollback() + # Compensating refund: balance was already committed by subtract_user_balance + try: + from app.database.crud.user import add_user_balance + + await add_user_balance( + db, + user, + daily_price, + 'Возврат: ошибка автопокупки суточного тарифа', + create_transaction=True, + transaction_type=TransactionType.REFUND, + ) + logger.info( + '💰 Автопокупка суточного тарифа: возврат средств после ошибки создания подписки', + format_user_id=_format_user_id(user), + refund_kopeks=daily_price, + ) + except Exception as refund_error: + logger.critical( + 'CRITICAL: Автопокупка суточного тарифа: не удалось вернуть средства', + format_user_id=_format_user_id(user), + price_kopeks=daily_price, + refund_error=refund_error, + ) return False # Создаём транзакцию @@ -1563,6 +1613,30 @@ async def _auto_add_traffic( exc_info=True, ) await db.rollback() + # Compensating refund: balance was already committed by subtract_user_balance + try: + from app.database.crud.user import add_user_balance + + await add_user_balance( + db, + user, + price_kopeks, + 'Возврат: ошибка автопокупки трафика', + create_transaction=True, + transaction_type=TransactionType.REFUND, + ) + logger.info( + '💰 Автопокупка трафика: возврат средств после ошибки добавления трафика', + format_user_id=_format_user_id(user), + refund_kopeks=price_kopeks, + ) + except Exception as refund_error: + logger.critical( + 'CRITICAL: Автопокупка трафика: не удалось вернуть средства', + format_user_id=_format_user_id(user), + price_kopeks=price_kopeks, + refund_error=refund_error, + ) return False # Реактивируем подписку если она была DISABLED/EXPIRED (например, после LIMITED/EXPIRED в RemnaWave) @@ -1803,6 +1877,13 @@ async def try_auto_extend_expired_after_topup( consume_promo_offer = get_user_active_promo_discount_percent(user) > 0 + # Save promo offer state before deduction (for restore on failure) + saved_promo_percent = ( + int(getattr(user, 'promo_offer_discount_percent', 0) or 0) if consume_promo_offer else 0 + ) + saved_promo_source = getattr(user, 'promo_offer_discount_source', None) if consume_promo_offer else None + saved_promo_expires = getattr(user, 'promo_offer_discount_expires_at', None) if consume_promo_offer else None + # Deduct balance description = f'Автопродление истёкшей подписки на {period_days} дней' try: @@ -1855,6 +1936,36 @@ async def try_auto_extend_expired_after_topup( exc_info=True, ) await db.rollback() + # Compensating refund: balance was already committed by subtract_user_balance + try: + from app.database.crud.user import add_user_balance + + await add_user_balance( + db, + user, + renewal_cost, + 'Возврат: ошибка автопродления истёкшей подписки', + create_transaction=True, + transaction_type=TransactionType.REFUND, + ) + # Restore promo offer if consumed + if consume_promo_offer and saved_promo_percent > 0: + user.promo_offer_discount_percent = saved_promo_percent + user.promo_offer_discount_source = saved_promo_source + user.promo_offer_discount_expires_at = saved_promo_expires + await db.commit() + logger.info( + '💰 Автопродление expired: возврат средств после ошибки продления', + format_user_id=_format_user_id(user), + refund_kopeks=renewal_cost, + ) + except Exception as refund_error: + logger.critical( + 'CRITICAL: Автопродление expired: не удалось вернуть средства', + format_user_id=_format_user_id(user), + price_kopeks=renewal_cost, + refund_error=refund_error, + ) return False # Create transaction record @@ -2113,6 +2224,30 @@ async def try_resume_disabled_daily_after_topup( exc_info=True, ) await db.rollback() + # Compensating refund: balance was already committed by subtract_user_balance + try: + from app.database.crud.user import add_user_balance + + await add_user_balance( + db, + user, + daily_price, + 'Возврат: ошибка авто-возобновления суточной подписки', + create_transaction=True, + transaction_type=TransactionType.REFUND, + ) + logger.info( + '💰 Авто-возобновление daily: возврат средств после ошибки активации', + format_user_id=_format_user_id(user), + refund_kopeks=daily_price, + ) + except Exception as refund_error: + logger.critical( + 'CRITICAL: Авто-возобновление daily: не удалось вернуть средства', + format_user_id=_format_user_id(user), + price_kopeks=daily_price, + refund_error=refund_error, + ) return False logger.info( diff --git a/app/services/subscription_service.py b/app/services/subscription_service.py index 08004705..a1eef3ff 100644 --- a/app/services/subscription_service.py +++ b/app/services/subscription_service.py @@ -42,26 +42,6 @@ def _resolve_discount_percent( return 0 -def _resolve_addon_discount_percent( - user: User | None, - promo_group: PromoGroup | None, - category: str, - *, - period_days: int | None = None, -) -> int: - group = promo_group or (user.get_primary_promo_group() if user else None) - - if group is not None and not getattr(group, 'apply_discounts_to_addons', True): - return 0 - - return _resolve_discount_percent( - user, - promo_group, - category, - period_days=period_days, - ) - - def get_traffic_reset_strategy(tariff=None): """Получает стратегию сброса трафика. @@ -711,106 +691,6 @@ class SubscriptionService: logger.error('Ошибка синхронизации подписки', subscription_id=subscription.id, error=e) return False, 'unknown_error' - async def calculate_subscription_price( - self, - period_days: int, - traffic_gb: int, - server_squad_ids: list[int], - devices: int, - db: AsyncSession, - *, - user: User | None = None, - promo_group: PromoGroup | None = None, - ) -> tuple[int, list[int]]: - from app.config import PERIOD_PRICES - from app.database.crud.server_squad import get_server_squad_by_id - - if settings.MAX_DEVICES_LIMIT > 0 and devices > settings.MAX_DEVICES_LIMIT: - raise ValueError(f'Превышен максимальный лимит устройств: {settings.MAX_DEVICES_LIMIT}') - - base_price_original = PERIOD_PRICES.get(period_days, 0) - period_discount_percent = _resolve_discount_percent( - user, - promo_group, - 'period', - period_days=period_days, - ) - base_discount_total = base_price_original * period_discount_percent // 100 - base_price = base_price_original - base_discount_total - - promo_group = promo_group or (user.get_primary_promo_group() if user else None) - - traffic_price = settings.get_traffic_price(traffic_gb) - traffic_discount_percent = _resolve_discount_percent( - user, - promo_group, - 'traffic', - period_days=period_days, - ) - traffic_discount = traffic_price * traffic_discount_percent // 100 - discounted_traffic_price = traffic_price - traffic_discount - - server_prices = [] - total_servers_price = 0 - servers_discount_percent = _resolve_discount_percent( - user, - promo_group, - 'servers', - period_days=period_days, - ) - - for server_id in server_squad_ids: - server = await get_server_squad_by_id(db, server_id) - if server and server.is_available and not server.is_full: - server_price = server.price_kopeks - server_discount = server_price * servers_discount_percent // 100 - discounted_server_price = server_price - server_discount - server_prices.append(discounted_server_price) - total_servers_price += discounted_server_price - log_message = f'Сервер {server.display_name}: {server_price / 100}₽' - if server_discount > 0: - log_message += f' (скидка {servers_discount_percent}%: -{server_discount / 100}₽ → {discounted_server_price / 100}₽)' - logger.debug(log_message) - else: - server_prices.append(0) - logger.warning('Сервер ID недоступен', server_id=server_id) - - devices_price = max(0, devices - settings.DEFAULT_DEVICE_LIMIT) * settings.PRICE_PER_DEVICE - devices_discount_percent = _resolve_discount_percent( - user, - promo_group, - 'devices', - period_days=period_days, - ) - devices_discount = devices_price * devices_discount_percent // 100 - discounted_devices_price = devices_price - devices_discount - - total_price = base_price + discounted_traffic_price + total_servers_price + discounted_devices_price - - logger.debug('Расчет стоимости новой подписки:') - base_log = f' Период {period_days} дней: {base_price_original / 100}₽' - if base_discount_total > 0: - base_log += f' → {base_price / 100}₽ (скидка {period_discount_percent}%: -{base_discount_total / 100}₽)' - logger.debug(base_log) - if discounted_traffic_price > 0: - message = f' Трафик {traffic_gb} ГБ: {traffic_price / 100}₽' - if traffic_discount > 0: - message += f' (скидка {traffic_discount_percent}%: -{traffic_discount / 100}₽ → {discounted_traffic_price / 100}₽)' - logger.debug(message) - if total_servers_price > 0: - message = f' Серверы ({len(server_squad_ids)}): {total_servers_price / 100}₽' - if servers_discount_percent > 0: - message += f' (скидка {servers_discount_percent}% применяется ко всем серверам)' - logger.debug(message) - if discounted_devices_price > 0: - message = f' Устройства ({devices}): {devices_price / 100}₽' - if devices_discount > 0: - message += f' (скидка {devices_discount_percent}%: -{devices_discount / 100}₽ → {discounted_devices_price / 100}₽)' - logger.debug(message) - logger.debug('ИТОГО: ₽', total_price=total_price / 100) - - return total_price, server_prices - async def validate_and_clean_subscription(self, db: AsyncSession, subscription: Subscription, user: User) -> bool: try: needs_cleanup = False @@ -913,14 +793,6 @@ class SubscriptionService: default_prices = [0] * len(country_uuids) return sum(default_prices), default_prices - async def _get_countries_price(self, country_uuids: list[str], db: AsyncSession) -> int: - try: - total_price, _ = await self.get_countries_price_by_uuids(country_uuids, db) - return total_price - except Exception as e: - logger.error('Ошибка получения цен стран', error=e) - return len(country_uuids) * 1000 - async def calculate_subscription_price_with_months( self, period_days: int, @@ -1035,97 +907,6 @@ class SubscriptionService: return total_price, server_prices - async def calculate_addon_price_with_remaining_period( - self, - subscription: Subscription, - additional_traffic_gb: int = 0, - additional_devices: int = 0, - additional_server_ids: list[int] = None, - db: AsyncSession = None, - ) -> int: - if additional_server_ids is None: - additional_server_ids = [] - - now = datetime.now(UTC) - days_to_pay = max(1, (subscription.end_date - now).days) - period_hint_days = days_to_pay - - user = getattr(subscription, 'user', None) - promo_group = user.promo_group if user else None - - total_price = 0 - - if additional_traffic_gb > 0: - traffic_price_per_month = settings.get_traffic_price(additional_traffic_gb) - traffic_discount_percent = _resolve_addon_discount_percent( - user, - promo_group, - 'traffic', - period_days=period_hint_days, - ) - traffic_discount_per_month = traffic_price_per_month * traffic_discount_percent // 100 - discounted_traffic_per_month = traffic_price_per_month - traffic_discount_per_month - traffic_total_price = int(discounted_traffic_per_month * days_to_pay / 30) - total_price += traffic_total_price - message = ( - f'Трафик +{additional_traffic_gb}ГБ: {traffic_price_per_month / 100}₽/мес x {days_to_pay} дн.' - f' = {traffic_total_price / 100}₽' - ) - if traffic_discount_per_month > 0: - message += f' (скидка {traffic_discount_percent}%: -{int(traffic_discount_per_month * days_to_pay / 30) / 100}₽)' - logger.info(message) - - if additional_devices > 0: - devices_price_per_month = additional_devices * settings.PRICE_PER_DEVICE - devices_discount_percent = _resolve_addon_discount_percent( - user, - promo_group, - 'devices', - period_days=period_hint_days, - ) - devices_discount_per_month = devices_price_per_month * devices_discount_percent // 100 - discounted_devices_per_month = devices_price_per_month - devices_discount_per_month - devices_total_price = int(discounted_devices_per_month * days_to_pay / 30) - total_price += devices_total_price - message = ( - f'Устройства +{additional_devices}: {devices_price_per_month / 100}₽/мес x {days_to_pay} дн.' - f' = {devices_total_price / 100}₽' - ) - if devices_discount_per_month > 0: - message += f' (скидка {devices_discount_percent}%: -{int(devices_discount_per_month * days_to_pay / 30) / 100}₽)' - logger.info(message) - - if additional_server_ids and db: - for server_id in additional_server_ids: - from app.database.crud.server_squad import get_server_squad_by_id - - server = await get_server_squad_by_id(db, server_id) - if server and server.is_available: - server_price_per_month = server.price_kopeks - servers_discount_percent = _resolve_addon_discount_percent( - user, - promo_group, - 'servers', - period_days=period_hint_days, - ) - server_discount_per_month = server_price_per_month * servers_discount_percent // 100 - discounted_server_per_month = server_price_per_month - server_discount_per_month - server_total_price = int(discounted_server_per_month * days_to_pay / 30) - total_price += server_total_price - message = ( - f'Сервер {server.display_name}: {server_price_per_month / 100}₽/мес x {days_to_pay} дн.' - f' = {server_total_price / 100}₽' - ) - if server_discount_per_month > 0: - message += ( - f' (скидка {servers_discount_percent}%:' - f' -{int(server_discount_per_month * days_to_pay / 30) / 100}₽)' - ) - logger.info(message) - - logger.info('Итого доплата за дн.: ₽', days_to_pay=days_to_pay, total_price=total_price / 100) - return total_price - def _gb_to_bytes(self, gb: int | None) -> int: if not gb: # None or 0 return 0