houseassassin/remnawave-bedolaga-telegram-bot
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: address 6-agent review findings for PricingEngine

Browse Source 
H1: log error when tariff_id set but tariff relationship not loaded
H2: warn on CLASSIC_PERIOD_PRICES→PERIOD_PRICES fallback
M1: fix float division in miniapp tariff purchase (use PricingEngine.apply_discount)
M2: fix format_period Russian pluralization for teen-hundreds (111-119, etc.)
M3: deduplicate _resolve_discount_percent — import from pricing_utils
M4: fix N+1 queries in compute_simple_subscription_price (batch fetch)
M5: add period_days validation tests (negative, zero, float)
M6: add user=None tests for tariff and classic modes
M7: fix float division in calculate_prorated_price (use // instead of /)
L1: add context to _calculate_servers_price error log
L2: add comment clarifying ClassicBreakdown.group_discount_pct type
L3: add test for original_total property
L4: inline _apply_percentage_discount wrapper in subscription_purchase_service
L5: replace global _server_id_counter with itertools.count() in tests
This commit is contained in:
Fringg
2026-03-13 05:45:46 +03:00
parent fe4e6acb53
commit c9f2dffabf
7 changed files with 161 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/services/pricing_engine.py
+18 -4
View File
@@ -42,6 +42,7 @@ class ClassicBreakdown:
base_traffic_gb: int
purchased_traffic_gb: int
extra_devices: int
# NB: dict[str, int] per-category (period/servers/traffic/devices), unlike TariffBreakdown's single int
group_discount_pct: dict[str, int]
offer_discount_pct: int
@@ -110,8 +111,8 @@ class PricingEngine:
try:
servers = await get_server_squads_by_uuids(db, country_uuids)
except Exception as e:
logger.error('Ошибка пакетной загрузки серверов', error=str(e))
except Exception as e: # intentional broad catch: pricing must not crash on DB errors, servers_price=0 is safe (user pays less)
logger.error('Ошибка пакетной загрузки серверов', error=str(e), squad_uuids=country_uuids)
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}
@@ -196,8 +197,15 @@ class PricingEngine:
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)
if subscription.tariff_id is not None:
if subscription.tariff is None:
logger.error(
'tariff_id set but tariff relationship not loaded, falling back to classic mode',
subscription_id=getattr(subscription, 'id', None),
tariff_id=subscription.tariff_id,
)
else:
return await self._calculate_tariff_mode(db, subscription, period_days, user=user)
return await self._calculate_classic_mode(db, subscription, period_days, user=user)
# ------------------------------------------------------------------
@@ -297,6 +305,12 @@ class PricingEngine:
base_price_original = CLASSIC_PERIOD_PRICES.get(period_days)
if base_price_original is None:
base_price_original = PERIOD_PRICES.get(period_days, 0)
if base_price_original > 0:
logger.warning(
'CLASSIC_PERIOD_PRICES miss, falling back to PERIOD_PRICES — verify price is not from tariff regime',
period_days=period_days,
fallback_price_kopeks=base_price_original,
)
# --- Per-category discount percents ---
period_pct = 0
app/services/subscription_purchase_service.py
+7 -12
View File
@@ -27,6 +27,7 @@ from app.database.models import ServerSquad, Subscription, SubscriptionStatus, T
from app.localization.texts import get_texts
from app.services.subscription_service import SubscriptionService
from app.utils.pricing_utils import (
apply_percentage_discount,
calculate_months_from_days,
format_period_description,
validate_pricing_calculation,
@@ -266,15 +267,9 @@ class PurchaseBalanceError(Exception):
super().__init__(message)
def _apply_percentage_discount(amount: int, percent: int) -> tuple[int, int]:
"""Delegate to shared apply_percentage_discount (uses PricingEngine internally)."""
from app.utils.pricing_utils import apply_percentage_discount
return apply_percentage_discount(amount, percent)
def _apply_discount_to_monthly_component(amount_per_month: int, percent: int, months: int) -> dict[str, int]:
discounted_per_month, discount_per_month = _apply_percentage_discount(amount_per_month, percent)
discounted_per_month, discount_per_month = apply_percentage_discount(amount_per_month, percent)
return {
'original_per_month': amount_per_month,
'discounted_per_month': discounted_per_month,
@@ -293,7 +288,7 @@ def _apply_promo_offer_discount(user: User | None, amount: int) -> tuple[int, in
percent = _get_promo_offer_discount_percent(user)
if amount <= 0 or percent <= 0:
return amount, 0, 0
discounted, discount_value = _apply_percentage_discount(amount, percent)
discounted, discount_value = apply_percentage_discount(amount, percent)
return discounted, discount_value, percent
@@ -303,7 +298,7 @@ def _build_server_option(
texts,
) -> PurchaseServerOption:
base_per_month = int(getattr(server, 'price_kopeks', 0) or 0)
discounted_per_month, _ = _apply_percentage_discount(base_per_month, discount_percent)
discounted_per_month, _ = apply_percentage_discount(base_per_month, discount_percent)
return PurchaseServerOption(
uuid=server.squad_uuid,
name=getattr(server, 'display_name', server.squad_uuid) or server.squad_uuid,
@@ -387,7 +382,7 @@ class MiniAppSubscriptionPurchaseService:
base_price_original = PERIOD_PRICES.get(period_days, 0)
period_discount_percent = user.get_promo_discount('period', period_days)
base_price, base_discount_total = _apply_percentage_discount(base_price_original, period_discount_percent)
base_price, base_discount_total = apply_percentage_discount(base_price_original, period_discount_percent)
base_price_label = texts.format_price(base_price)
base_price_original_label = (
texts.format_price(base_price_original)
@@ -520,7 +515,7 @@ class MiniAppSubscriptionPurchaseService:
for package in packages:
value = int(package.get('gb') or 0)
price_per_month = int(package.get('price') or 0)
discounted_per_month, discount_value = _apply_percentage_discount(price_per_month, discount_percent)
discounted_per_month, discount_value = apply_percentage_discount(price_per_month, discount_percent)
label = texts.format_traffic(value or 0)
options.append(
PurchaseTrafficOption(
@@ -594,7 +589,7 @@ class MiniAppSubscriptionPurchaseService:
) -> PurchaseDevicesConfig:
discount_percent = user.get_promo_discount('devices', period_days)
unit_price = settings.PRICE_PER_DEVICE
discounted_unit_price, unit_discount_value = _apply_percentage_discount(unit_price, discount_percent)
discounted_unit_price, unit_discount_value = apply_percentage_discount(unit_price, discount_percent)
price_label = texts.format_price(discounted_unit_price)
original_label = (
texts.format_price(unit_price) if unit_discount_value and unit_price != discounted_unit_price else None
app/services/subscription_service.py
+5 -22
View File
@@ -14,6 +14,7 @@ from app.database.models import PromoGroup, Subscription, SubscriptionStatus, Us
from app.external.remnawave_api import RemnaWaveAPI, RemnaWaveAPIError, RemnaWaveUser, TrafficLimitStrategy, UserStatus
from app.utils.pricing_utils import (
calculate_months_from_days,
resolve_discount_percent,
)
from app.utils.subscription_utils import (
resolve_hwid_device_limit_for_payload,
@@ -23,24 +24,6 @@ from app.utils.subscription_utils import (
logger = structlog.get_logger(__name__)
def _resolve_discount_percent(
user: User | None,
promo_group: PromoGroup | None,
category: str,
*,
period_days: int | None = None,
) -> int:
if user is not None:
try:
return user.get_promo_discount(category, period_days)
except AttributeError:
pass
if promo_group is not None:
return promo_group.get_discount_percent(category, period_days)
return 0
def get_traffic_reset_strategy(tariff=None):
"""Получает стратегию сброса трафика.
@@ -813,7 +796,7 @@ class SubscriptionService:
months_in_period = calculate_months_from_days(period_days)
base_price_original = PERIOD_PRICES.get(period_days, 0)
period_discount_percent = _resolve_discount_percent(
period_discount_percent = resolve_discount_percent(
user,
promo_group,
'period',
@@ -825,7 +808,7 @@ class SubscriptionService:
promo_group = promo_group or (user.get_primary_promo_group() if user else None)
traffic_price_per_month = settings.get_traffic_price(traffic_gb)
traffic_discount_percent = _resolve_discount_percent(
traffic_discount_percent = resolve_discount_percent(
user,
promo_group,
'traffic',
@@ -837,7 +820,7 @@ class SubscriptionService:
server_prices = []
total_servers_price = 0
servers_discount_percent = _resolve_discount_percent(
servers_discount_percent = resolve_discount_percent(
user,
promo_group,
'servers',
@@ -865,7 +848,7 @@ class SubscriptionService:
additional_devices = max(0, devices - settings.DEFAULT_DEVICE_LIMIT)
devices_price_per_month = additional_devices * settings.PRICE_PER_DEVICE
devices_discount_percent = _resolve_discount_percent(
devices_discount_percent = resolve_discount_percent(
user,
promo_group,
'devices',
app/utils/formatting.py
+11 -9
View File
@@ -21,12 +21,14 @@ def format_price_kopeks(kopeks: int, compact: bool = False) -> str:
def format_period(days: int) -> str:
"""Форматирует период."""
if days == 1:
return '1 день'
if days < 5:
return f'{days} дня'
if days < 21 or days % 10 >= 5 or days % 10 == 0:
return f'{days} дней'
if days % 10 == 1:
return f'{days} день'
return f'{days} дня'
mod100 = days % 100
mod10 = days % 10
if 11 <= mod100 <= 19:
word = 'дней'
elif mod10 == 1:
word = 'день'
elif 2 <= mod10 <= 4:
word = 'дня'
else:
word = 'дней'
return f'{days} {word}'
app/utils/pricing_utils.py
+9 -3
View File
@@ -28,7 +28,7 @@ def calculate_prorated_price(monthly_price: int, end_date: datetime, min_charge_
days_remaining = max(1, (end_date - now).days)
days_to_charge = max(min_charge_days, days_remaining)
total_price = int(monthly_price * days_to_charge / 30)
total_price = monthly_price * days_to_charge // 30
if monthly_price > 0:
total_price = max(100, total_price) # Минимум 1 рубль
@@ -163,14 +163,20 @@ async def compute_simple_subscription_price(
elif raw_squad:
resolved_uuids.append(str(raw_squad))
from app.database.crud.server_squad import get_server_squad_by_uuid
from app.database.crud.server_squad import get_server_squads_by_uuids
server_breakdown: list[dict[str, Any]] = []
servers_price_original = 0
servers_discount_total = 0
if resolved_uuids:
servers = await get_server_squads_by_uuids(db, resolved_uuids)
server_map = {s.squad_uuid: s for s in servers}
else:
server_map = {}
for squad_uuid in resolved_uuids:
server = await get_server_squad_by_uuid(db, squad_uuid)
server = server_map.get(squad_uuid)
if not server:
logger.warning('SIMPLE_SUBSCRIPTION_PRICE_SERVER_NOT_FOUND | squad', squad_uuid=squad_uuid)
server_breakdown.append(
app/webapi/routes/miniapp.py
+3 -1
View File
@@ -6463,7 +6463,9 @@ async def purchase_tariff_endpoint(
except (TypeError, ValueError):
pass
if discount_percent > 0:
price_kopeks = int(base_price_kopeks * (100 - discount_percent) / 100)
from app.services.pricing_engine import PricingEngine
price_kopeks = PricingEngine.apply_discount(base_price_kopeks, discount_percent)
# Apply personal promo_offer discount on top of group discount
consume_promo_offer = False
tests/test_pricing_engine.py
+108 -4
View File
@@ -1,3 +1,5 @@
import itertools
import pytest
from app.services.pricing_engine import PricingEngine, RenewalPricing
@@ -75,16 +77,46 @@ class TestStackedDiscounts:
from unittest.mock import AsyncMock, MagicMock, patch
_server_id_counter = 0
class TestPeriodDaysValidation:
@pytest.mark.asyncio
async def test_negative_period_days_raises(self):
engine = PricingEngine()
db = AsyncMock()
subscription = MagicMock()
subscription.tariff_id = None
subscription.tariff = None
with pytest.raises(ValueError, match='Invalid period_days'):
await engine.calculate_renewal_price(db, subscription, -1)
@pytest.mark.asyncio
async def test_zero_period_days_raises(self):
engine = PricingEngine()
db = AsyncMock()
subscription = MagicMock()
subscription.tariff_id = None
subscription.tariff = None
with pytest.raises(ValueError, match='Invalid period_days'):
await engine.calculate_renewal_price(db, subscription, 0)
@pytest.mark.asyncio
async def test_float_period_days_raises(self):
engine = PricingEngine()
db = AsyncMock()
subscription = MagicMock()
subscription.tariff_id = None
subscription.tariff = None
with pytest.raises(ValueError, match='Invalid period_days'):
await engine.calculate_renewal_price(db, subscription, 30.0)
_server_id_seq = itertools.count(1)
def _make_server(
price_kopeks=5000, is_available=True, is_full=False, allowed_promo_groups=None, server_id=None, squad_uuid=None
):
global _server_id_counter
if server_id is None:
_server_id_counter += 1
server_id = _server_id_counter
server_id = next(_server_id_seq)
server = MagicMock()
server.id = server_id
server.squad_uuid = squad_uuid
@@ -371,6 +403,29 @@ class TestCalculateRenewalPriceTariffMode:
assert result.final_total == 10000
assert result.breakdown.get('extra_devices') == 0
@pytest.mark.asyncio
async def test_tariff_user_none(self):
"""When user=None, no discounts are applied."""
engine = PricingEngine()
db = AsyncMock()
subscription = MagicMock()
subscription.tariff_id = 1
subscription.tariff = MagicMock()
subscription.tariff.period_prices = {'30': 20000}
subscription.tariff.device_limit = 1
subscription.tariff.device_price_kopeks = None
subscription.tariff.id = 1
subscription.device_limit = 1
with (
patch('app.services.pricing_engine.get_user_active_promo_discount_percent', return_value=0),
patch('app.services.pricing_engine.settings') as ms,
):
ms.PRICE_PER_DEVICE = 5000
result = await engine.calculate_renewal_price(db, subscription, 30, user=None)
assert result.final_total == 20000
assert result.promo_group_discount == 0
assert result.promo_offer_discount == 0
class TestCalculateRenewalPriceClassicMode:
@pytest.mark.asyncio
@@ -693,6 +748,33 @@ class TestCalculateRenewalPriceClassicMode:
assert result.promo_group_discount == 3700
assert result.final_total == 9000 + 4800 + 3500 + 8000
@pytest.mark.asyncio
async def test_classic_user_none(self):
"""When user=None, no discounts are applied."""
engine = PricingEngine()
db = AsyncMock()
subscription = MagicMock()
subscription.tariff_id = None
subscription.tariff = None
subscription.connected_squads = []
subscription.traffic_limit_gb = 0
subscription.purchased_traffic_gb = 0
subscription.device_limit = 1
with (
patch('app.services.pricing_engine.get_user_active_promo_discount_percent', return_value=0),
patch('app.services.pricing_engine.settings') as ms,
patch('app.services.pricing_engine.CLASSIC_PERIOD_PRICES', {30: 15000}),
patch('app.services.pricing_engine.PERIOD_PRICES', {}),
):
ms.get_traffic_price.return_value = 0
ms.PRICE_PER_DEVICE = 0
ms.DEFAULT_DEVICE_LIMIT = 1
ms.is_traffic_fixed.return_value = False
result = await engine.calculate_renewal_price(db, subscription, 30, user=None)
assert result.final_total == 15000
assert result.promo_group_discount == 0
assert result.promo_offer_discount == 0
class TestServerPromoGroupFiltering:
@pytest.mark.asyncio
@@ -864,3 +946,25 @@ class TestOriginalPriceIdentity:
# original should equal base_original + servers_original + traffic_original + devices_original
expected_original = 10000 + 4000 + 3000 + 5000 # 22000
assert original == expected_original
@pytest.mark.asyncio
async def test_original_total_property_tariff(self):
"""original_total property returns correct value."""
engine = PricingEngine()
db = AsyncMock()
tariff = MagicMock()
tariff.id = 1
tariff.period_prices = {'30': 20000}
tariff.device_price_kopeks = None
tariff.device_limit = 1
sub = MagicMock()
sub.tariff_id = 1
sub.tariff = tariff
sub.device_limit = 1
user = MagicMock()
promo_group = MagicMock()
promo_group.get_discount_percent = MagicMock(return_value=10)
user.promo_group = promo_group
with patch('app.services.pricing_engine.get_user_active_promo_discount_percent', return_value=5):
result = await engine.calculate_renewal_price(db, sub, 30, user=user)
assert result.original_total == 20000 # undiscounted subtotal