260 lines
9.4 KiB
Markdown
260 lines
9.4 KiB
Markdown
# P0 DEEP DIVE DIAGNOSTIC — Subscription Math Trace Report
|
||
|
||
**Date:** 2026-06-27
|
||
**Author:** Fast Coder (Core Developer)
|
||
**Scope:** Root cause analysis of `asset_limit` returning 0 for `corp_premium_plus_v1` tier
|
||
|
||
---
|
||
|
||
## 1. DATABASE DUMP — Raw JSONB for `corp_premium_plus_v1`
|
||
|
||
### Query
|
||
```sql
|
||
SELECT id, name, rules, feature_capabilities
|
||
FROM system.subscription_tiers
|
||
WHERE name = 'corp_premium_plus_v1';
|
||
```
|
||
|
||
### Result
|
||
|
||
| Column | Value |
|
||
|--------|-------|
|
||
| `id` | `17` |
|
||
| `name` | `corp_premium_plus_v1` |
|
||
| `rules` (JSONB) | See below |
|
||
| `feature_capabilities` | `{}` (empty object) |
|
||
|
||
### Raw `rules` JSONB (pretty-printed)
|
||
|
||
```json
|
||
{
|
||
"type": "corporate",
|
||
"pricing": {
|
||
"currency": "EUR",
|
||
"credit_price": 6000,
|
||
"yearly_price": 599.99,
|
||
"monthly_price": 59.99
|
||
},
|
||
"duration": {
|
||
"days": 30,
|
||
"allow_stacking": true
|
||
},
|
||
"ad_policy": {
|
||
"show_ads": false,
|
||
"ad_free_grace_days": 0,
|
||
"max_daily_impressions": null
|
||
},
|
||
"affiliate": {
|
||
"referral_bonus_credits": 250,
|
||
"commission_rate_percent": 20
|
||
},
|
||
"lifecycle": {
|
||
"is_public": true
|
||
},
|
||
"marketing": {
|
||
"badge": "Plus",
|
||
"subtitle": "Nagy flották prémium menedzsmentje.",
|
||
"highlight_color": "#8B5CF6"
|
||
},
|
||
"allowances": {
|
||
"max_garages": 10,
|
||
"max_vehicles": 50,
|
||
"monthly_free_credits": 800
|
||
},
|
||
"display_name": "Céges Prémium Plus",
|
||
"entitlements": [
|
||
"SRV_DATA_EXPORT",
|
||
"SRV_AI_UPLOAD",
|
||
"SRV_ACCOUNTING_SYNC"
|
||
],
|
||
"pricing_zones": {
|
||
"HU": { "currency": "HUF", "credit_price": 6000, "yearly_price": 239990, "monthly_price": 23990 },
|
||
"US": { "currency": "USD", "credit_price": 6000, "yearly_price": 699.99, "monthly_price": 69.99 },
|
||
"DEFAULT": { "currency": "EUR", "credit_price": 6000, "yearly_price": 599.99, "monthly_price": 59.99 }
|
||
}
|
||
}
|
||
```
|
||
|
||
### Key Observation
|
||
|
||
The vehicle limit **`max_vehicles: 50`** is stored **inside a nested `allowances` object** within the `rules` JSONB column:
|
||
|
||
```
|
||
rules → allowances → max_vehicles: 50
|
||
```
|
||
|
||
The `feature_capabilities` column is **empty** (`{}`).
|
||
|
||
---
|
||
|
||
## 2. MATH TRACE — The Aggregation Logic
|
||
|
||
### File: [`backend/app/api/v1/endpoints/admin_organizations.py`](backend/app/api/v1/endpoints/admin_organizations.py:600)
|
||
|
||
### The Formula (line 680)
|
||
|
||
```python
|
||
asset_limit = base_vehicles + extra_vehicles + addon_vehicles
|
||
```
|
||
|
||
Where:
|
||
- `base_vehicles` ← extracted from the **base tier's** `rules` + `feature_capabilities` + `extra_allowances`
|
||
- `extra_vehicles` ← from `base_extra.get("extra_vehicles", 0)` (admin overrides in `OrganizationSubscription.extra_allowances`)
|
||
- `addon_vehicles` ← summed across all active add-on subscriptions
|
||
|
||
### The `_extract_limit` Function (lines 601–630)
|
||
|
||
```python
|
||
def _extract_limit(
|
||
rules: dict,
|
||
fc: dict,
|
||
extra: dict,
|
||
vehicle_keys: tuple = ("max_vehicles", "vehicle_limit", "max_assets", "assets_limit"),
|
||
branch_keys: tuple = ("max_branches", "branch_limit", "max_garages", "garages_limit"),
|
||
user_keys: tuple = ("max_users", "user_limit", "max_members", "members_limit"),
|
||
default: int = 0,
|
||
):
|
||
def _pick(keys: tuple) -> int:
|
||
# 1. Check root level of rules, fc, extra
|
||
for d in (rules, fc, extra):
|
||
for k in keys:
|
||
val = d.get(k)
|
||
if val is not None and isinstance(val, (int, float)):
|
||
return int(val)
|
||
# 2. Check nested 'allowances' object inside rules
|
||
allowances = rules.get("allowances", {})
|
||
if isinstance(allowances, dict):
|
||
for k in keys:
|
||
val = allowances.get(k)
|
||
if val is not None and isinstance(val, (int, float)):
|
||
return int(val)
|
||
return default
|
||
return _pick(vehicle_keys), _pick(branch_keys), _pick(user_keys)
|
||
```
|
||
|
||
### How It's Called (line 648)
|
||
|
||
```python
|
||
base_vehicles, base_branches, base_users = _extract_limit(rules, fc, base_extra)
|
||
```
|
||
|
||
Where:
|
||
- `rules` = `active_base_sub.tier.rules` (the JSONB shown above)
|
||
- `fc` = `active_base_sub.tier.feature_capabilities` → `{}` (empty)
|
||
- `base_extra` = `active_base_sub.extra_allowances` → `{}` (empty, unless admin set overrides)
|
||
|
||
---
|
||
|
||
## 3. ROOT CAUSE ANALYSIS
|
||
|
||
### Step-by-step trace of `_pick(vehicle_keys)`:
|
||
|
||
**Phase 1 — Root level scan of `rules`, `fc`, `extra`:**
|
||
|
||
The function iterates over `(rules, fc, extra)` and checks each key in `vehicle_keys = ("max_vehicles", "vehicle_limit", "max_assets", "assets_limit")`.
|
||
|
||
| Dictionary | `max_vehicles` | `vehicle_limit` | `max_assets` | `assets_limit` |
|
||
|------------|---------------|-----------------|--------------|----------------|
|
||
| `rules` (root level) | ❌ Not found | ❌ Not found | ❌ Not found | ❌ Not found |
|
||
| `fc` = `{}` | ❌ Not found | ❌ Not found | ❌ Not found | ❌ Not found |
|
||
| `extra` = `{}` | ❌ Not found | ❌ Not found | ❌ Not found | ❌ Not found |
|
||
|
||
**Result: `None` from all three dictionaries.**
|
||
|
||
**Phase 2 — Nested `allowances` scan:**
|
||
|
||
```python
|
||
allowances = rules.get("allowances", {}) # → {"max_garages": 10, "max_vehicles": 50, "monthly_free_credits": 800}
|
||
```
|
||
|
||
Now it checks `allowances.get("max_vehicles")` → **`50`** ✓
|
||
|
||
**This SHOULD work!** The nested `allowances` fallback was added as a "P0 JSONB NESTING HOTFIX" (see comment on line 612).
|
||
|
||
### The Real Problem: TWO CODE PATHS
|
||
|
||
The `get_organization_details` function has **two separate code paths** for computing the subscription summary:
|
||
|
||
#### Path A: `active_sub` exists (line 519–550) — **OLD CODE**
|
||
```python
|
||
if active_sub and active_sub.tier:
|
||
rules = active_sub.tier.rules or {}
|
||
fc = active_sub.tier.feature_capabilities or {}
|
||
extra = active_sub.extra_allowances or {}
|
||
|
||
# Base limits from tier rules (with explicit .get() — no `or` bug)
|
||
base_vehicles = rules.get("max_vehicles", fc.get("max_vehicles", 1))
|
||
base_branches = rules.get("max_branches", fc.get("max_branches", 0))
|
||
base_users = rules.get("max_users", fc.get("max_users", 0))
|
||
```
|
||
|
||
**BUG:** This path uses `rules.get("max_vehicles", ...)` directly — it does **NOT** check the nested `allowances` object! Since `max_vehicles` is at `rules → allowances → max_vehicles`, not at `rules → max_vehicles`, this returns the fallback value of `1`.
|
||
|
||
#### Path B: Multi-subscription aggregation (line 600–699) — **NEW CODE with `_extract_limit`**
|
||
```python
|
||
if active_base_sub and active_base_sub.tier:
|
||
rules = active_base_sub.tier.rules or {}
|
||
fc = active_base_sub.tier.feature_capabilities or {}
|
||
base_extra = active_base_sub.extra_allowances or {}
|
||
|
||
base_vehicles, base_branches, base_users = _extract_limit(rules, fc, base_extra)
|
||
```
|
||
|
||
**This path DOES work correctly** because `_extract_limit` checks the nested `allowances` object.
|
||
|
||
### Which path is being hit?
|
||
|
||
Looking at the subscription query (lines 443–455):
|
||
|
||
```python
|
||
sub_stmt = (
|
||
select(OrganizationSubscription)
|
||
.options(selectinload(OrganizationSubscription.tier))
|
||
.where(
|
||
OrganizationSubscription.org_id == org_id,
|
||
OrganizationSubscription.is_active == True,
|
||
OrganizationSubscription.valid_until >= now,
|
||
)
|
||
.order_by(OrganizationSubscription.id.desc())
|
||
.limit(1)
|
||
)
|
||
sub_result = await db.execute(sub_stmt)
|
||
active_sub = sub_result.scalar_one_or_none()
|
||
```
|
||
|
||
This query returns a **single** subscription row (`.limit(1)`). If the org has a `finance.org_subscriptions` row with `is_active=True` and `valid_until >= now`, then `active_sub` is set and **Path A** is executed — which has the bug.
|
||
|
||
**Path B** (the multi-subscription aggregation with `_extract_limit`) is only reached if the subscription query returns NO rows (`active_sub` is None), which would be the case if the org has no subscription record.
|
||
|
||
### Summary of the Bug
|
||
|
||
| Aspect | Detail |
|
||
|--------|--------|
|
||
| **Bug Location** | [`admin_organizations.py:525`](backend/app/api/v1/endpoints/admin_organizations.py:525) — Path A |
|
||
| **Root Cause** | `rules.get("max_vehicles", ...)` only checks the **root level** of the `rules` JSONB. The actual value is nested inside `rules → allowances → max_vehicles`. |
|
||
| **Affected Data** | All tiers that store limits inside a nested `allowances` object (all tiers in the DB use this structure) |
|
||
| **Why it returns 0** | It doesn't return 0 — it returns `1` (the fallback from `fc.get("max_vehicles", 1)`). But the **Total Limit** equals the **Extra Limit** because the base calculation returns only the fallback `1` instead of the real `50`. |
|
||
| **Fix** | Path A needs to also check the nested `allowances` object, just like `_extract_limit` does in Path B. |
|
||
|
||
---
|
||
|
||
## 4. COMPARISON: `evidence.py` Works Correctly
|
||
|
||
Interestingly, the [`evidence.py`](backend/app/api/v1/endpoints/evidence.py:36) endpoint already handles this correctly:
|
||
|
||
```python
|
||
if tier and tier.rules:
|
||
max_vehicles = tier.rules.get("allowances", {}).get("max_vehicles", 1)
|
||
max_allowed = max(int(max_vehicles), 1)
|
||
```
|
||
|
||
It directly accesses `rules → allowances → max_vehicles` with the nested `.get("allowances", {}).get("max_vehicles", 1)` pattern.
|
||
|
||
---
|
||
|
||
## 5. CONCLUSION
|
||
|
||
The `_extract_limit` function (Path B) is **correct** — it properly handles the nested `allowances` structure. However, the **old Path A** (lines 519–550) is still active and is the one being hit when the org has a valid `finance.org_subscriptions` row. Path A uses `rules.get("max_vehicles", ...)` which only checks the root level of the JSONB, missing the nested `allowances` object.
|
||
|
||
**The fix is to update Path A** (lines 525–527) to also search inside the nested `allowances` object, or to refactor Path A to use the same `_extract_limit` helper that Path B uses.
|