admin_users_personels
This commit is contained in:
80
docs/admin_users_filter_bug_analysis.md
Normal file
80
docs/admin_users_filter_bug_analysis.md
Normal file
@@ -0,0 +1,80 @@
|
||||
# Admin Users List Filter Bug Analysis
|
||||
|
||||
**Gitea Issue:** #323
|
||||
**Scope:** Backend & Frontend (Admin UI)
|
||||
**Type:** Bug
|
||||
|
||||
## Root Cause: Frontend-Backend Parameter Name Mismatch
|
||||
|
||||
The admin users list page at [`frontend_admin/pages/users/index.vue`](frontend_admin/pages/users/index.vue:539) sends filter parameters to the backend endpoint [`GET /api/v1/admin/users`](backend/app/api/v1/endpoints/admin.py:491), but the parameter names don't match what the backend expects.
|
||||
|
||||
### Frontend sends (frontend_admin/pages/users/index.vue, lines 543-556):
|
||||
|
||||
```typescript
|
||||
// What frontend sends:
|
||||
params.status_filter = statusFilter.value // "active", "inactive", "deleted", "banned"
|
||||
params.role_filter = roleFilter.value // "user", "admin", "staff", "superadmin"
|
||||
params.plan_filter = planFilter.value // "FREE", "PREMIUM", "ENTERPRISE"
|
||||
```
|
||||
|
||||
### Backend expects (backend/app/api/v1/endpoints/admin.py, lines 491-656):
|
||||
|
||||
```python
|
||||
# What backend expects:
|
||||
role: Optional[str] = Query(None) # NOT role_filter
|
||||
is_active: Optional[bool] = Query(None) # NOT status_filter
|
||||
is_deleted: Optional[bool] = Query(None) # NOT status_filter
|
||||
# plan_filter: NOT SUPPORTED AT ALL
|
||||
```
|
||||
|
||||
## Detailed Issues
|
||||
|
||||
### Issue 1: Status Filter (`status_filter` vs `is_active`/`is_deleted`)
|
||||
|
||||
| Frontend sends | Should map to backend |
|
||||
|---------------|----------------------|
|
||||
| `status_filter=active` | `is_active=true, is_deleted=false` |
|
||||
| `status_filter=inactive` | `is_active=false, is_deleted=false` |
|
||||
| `status_filter=deleted` | `is_deleted=true` |
|
||||
| `status_filter=banned` | `is_active=false` (no `is_banned` field exists) |
|
||||
|
||||
### Issue 2: Role Filter (`role_filter` vs `role`)
|
||||
|
||||
Simple rename: `role_filter` → `role`
|
||||
|
||||
### Issue 3: Plan Filter (`plan_filter` not supported)
|
||||
|
||||
The backend has **no** `plan_filter` or `subscription_plan` query parameter at all. The `User` model does have a `subscription_plan` column (backend/app/models/identity/identity.py:152), so this needs to be added to the backend.
|
||||
|
||||
### Issue 4: `person_name` missing from response
|
||||
|
||||
The frontend `UserListItem` interface expects `person_name` (line 417), but the backend returns `first_name` and `last_name` separately (lines 640-641). The frontend displays `user.person_name || '—'` (line 246), which means it will always show `—` because `person_name` is never populated.
|
||||
|
||||
## Fix Plan
|
||||
|
||||
### Backend Changes (backend/app/api/v1/endpoints/admin.py)
|
||||
|
||||
1. **Add `plan_filter` query parameter** to the `list_users` endpoint:
|
||||
- Accept `plan_filter: Optional[str]`
|
||||
- Filter by `User.subscription_plan == plan_filter`
|
||||
- Valid values: "FREE", "PREMIUM", "ENTERPRISE" (plus "all")
|
||||
|
||||
2. **Add `person_name` to the response** by concatenating `first_name` and `last_name`.
|
||||
|
||||
### Frontend Changes (frontend_admin/pages/users/index.vue)
|
||||
|
||||
1. **Rename parameter**: Change `params.role_filter` → `params.role`
|
||||
2. **Status filter logic**: Replace `params.status_filter` with proper mapping:
|
||||
- `active` → `params.is_active = true`
|
||||
- `inactive` → `params.is_active = false`
|
||||
- `deleted` → `params.is_deleted = true`
|
||||
- `banned` → `params.is_active = false` (since banned users are deactivated)
|
||||
3. **Keep `plan_filter`** as-is (backend will support it after fix)
|
||||
4. **Fix `person_name`** display: Compute it from `first_name` + `last_name` if available
|
||||
|
||||
## Files to Modify
|
||||
|
||||
| File | Changes |
|
||||
|------|---------|
|
||||
| `backend/app/api/v1/endpoints/admin.py` | Add `plan_filter` param, add `person_name` to response |
|
||||
| `frontend_admin/pages/users/index.vue` | Fix parameter names and status mapping in `fetchUsers()` |
|
||||
189
docs/p0_subscription_jsonb_structural_audit_report.md
Normal file
189
docs/p0_subscription_jsonb_structural_audit_report.md
Normal file
@@ -0,0 +1,189 @@
|
||||
# P0 DATA STRUCTURAL AUDIT REPORT: `_extract_limit()` & JSONB Tier Structure
|
||||
|
||||
**Date:** 2026-06-28
|
||||
**Author:** System Architect
|
||||
**Scope:** [`system.subscription_tiers`](backend/app/models/core_logic.py:68), [`finance.org_subscriptions`](backend/app/models/core_logic.py:143), [`_extract_limit()`](backend/app/api/v1/endpoints/admin_organizations.py:397)
|
||||
**Status:** ✅ Structural audit complete — `_extract_limit` logic is **correct**, DB JSONB is **valid**
|
||||
|
||||
---
|
||||
|
||||
## 1. AUDIT: `system.subscription_tiers` (Base Tier Rules)
|
||||
|
||||
### Raw JSON Structure
|
||||
|
||||
All three corporate tiers have their limits stored at **`rules["allowances"]`** (nested object, 2nd level):
|
||||
|
||||
| Tier | ID | `jsonb_typeof(rules)` | `allowances.max_vehicles` | `allowances.max_garages` |
|
||||
|------|----|----------------------|--------------------------|--------------------------|
|
||||
| `private_pro_v1` | 14 | `object` ✅ | 3 | 1 |
|
||||
| `corp_premium_v1` | 16 | `object` ✅ | 20 | 3 |
|
||||
| `corp_premium_plus_v1` | 17 | `object` ✅ | 50 | 10 |
|
||||
|
||||
### Full JSON for `corp_premium_plus_v1` (id=17)
|
||||
|
||||
```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": "Ceiges Preimium 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 }, "DEFAULT": { "currency": "EUR", "credit_price": 6000, "yearly_price": 599.99, "monthly_price": 59.99 } }
|
||||
}
|
||||
```
|
||||
|
||||
### Confirmed: `rules` is a proper JSONB **object**, NOT a string
|
||||
|
||||
```sql
|
||||
SELECT jsonb_typeof(rules) FROM system.subscription_tiers WHERE id = 17;
|
||||
-- Result: "object" ✅ (not "string")
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 2. AUDIT: `finance.org_subscriptions` (Active Subs & Extra Allowances)
|
||||
|
||||
| org_id | Sub ID | Tier | Tier ID | `extra_allowances` |
|
||||
|--------|--------|------|---------|-------------------|
|
||||
| 15 | 2 | `corp_premium_v1` | 16 | (not queried) |
|
||||
| 43 | 30 | `private_pro_v1` | 14 | `{}` |
|
||||
| 44 | 22 | `corp_premium_v1` | 16 | (not queried) |
|
||||
| 45 | 21 | `corp_premium_plus_v1` | 17 | (not queried) |
|
||||
| 49 | 20 | `corp_premium_v1` | 16 | (not queried) |
|
||||
|
||||
For org_id=43: `extra_allowances` = `{}` (empty JSONB object) — no extra allowances purchased.
|
||||
|
||||
---
|
||||
|
||||
## 3. TEST SCRIPT: `_extract_limit()` Behavior
|
||||
|
||||
### Implementation (exact copy from [`admin_organizations.py:397`](backend/app/api/v1/endpoints/admin_organizations.py:397))
|
||||
|
||||
The function searches at **3 levels**:
|
||||
1. **Level 1**: Direct keys at rules root (`rules["max_vehicles"]`)
|
||||
2. **Level 2**: Nested under `rules["allowances"]` (`rules["allowances"]["max_vehicles"]`)
|
||||
3. **Level 3**: Nested under `rules["limits"]` (`rules["limits"]["max_vehicles"]`)
|
||||
|
||||
### Test Results — ALL PASS ✅
|
||||
|
||||
| Tier | Vehicles | Branches | Users | Status |
|
||||
|------|----------|----------|-------|--------|
|
||||
| `corp_premium_plus_v1` | **50** ✅ | **10** ✅ | 0 | **PASS** |
|
||||
| `corp_premium_v1` | **20** ✅ | **3** ✅ | 0 | **PASS** |
|
||||
| `private_pro_v1` | **3** ✅ | **1** ✅ | 0 | **PASS** |
|
||||
|
||||
### Edge Cases
|
||||
|
||||
| Input | Result | Expected | Status |
|
||||
|-------|--------|----------|--------|
|
||||
| Empty dict `{}` | 5 (default) | 5 | ✅ |
|
||||
| `None` | 5 (default) | 5 | ✅ |
|
||||
| String (not dict) | 5 (default) | 5 | ✅ |
|
||||
| Direct key match | 100 | 100 | ✅ |
|
||||
| `allowances.max_vehicles` | 50 | 50 | ✅ |
|
||||
| `limits.max_vehicles` | 25 | 25 | ✅ |
|
||||
|
||||
### Double-Encoding Simulation
|
||||
|
||||
When the JSON is **double-encoded** (string stored inside JSONB instead of object):
|
||||
|
||||
```
|
||||
Input: '"{\\"allowances\\": {\\"max_vehicles\\": 50}}"'
|
||||
→ json.loads() returns str, not dict
|
||||
→ _extract_limit(str, ...) returns default=0 ❌
|
||||
```
|
||||
|
||||
✅ **Confirmed: Current DB data is NOT double-encoded.** (`jsonb_typeof = "object"`)
|
||||
|
||||
---
|
||||
|
||||
## 4. ROOT CAUSE ANALYSIS
|
||||
|
||||
### The `_extract_limit()` function is **NOT the problem**
|
||||
|
||||
All tests confirm the function correctly extracts limits from the current JSONB structure. It fully supports the `allowances` nested format used by all tiers.
|
||||
|
||||
### The REAL problem is likely in the **code flow** before `_extract_limit` is called
|
||||
|
||||
The endpoint [`get_organization_details()`](backend/app/api/v1/endpoints/admin_organizations.py:462) has **two code paths**:
|
||||
|
||||
#### Path A (line 579-664) — Uses `finance.org_subscriptions` (NEW system)
|
||||
|
||||
```python
|
||||
active_subs = sub_result.scalars().all() # Query finance.org_subscriptions
|
||||
if active_subs:
|
||||
base_subs = [s for s in active_subs if s.tier and s.tier.type in (None, '', 'base')]
|
||||
primary_sub = base_subs_sorted[0] if base_subs_sorted else ...
|
||||
if primary_sub and primary_sub.tier:
|
||||
primary_rules = primary_sub.tier.rules or {} # ← THIS must work
|
||||
```
|
||||
|
||||
#### Path B (line 665-691) — Uses `fleet.organizations.subscription_tier_id` (LEGACY)
|
||||
|
||||
```python
|
||||
elif org.subscription_tier:
|
||||
rules = org.subscription_tier.rules or {} # ← LEGACY path
|
||||
```
|
||||
|
||||
### Potential failure scenarios (outside `_extract_limit`):
|
||||
|
||||
| Scenario | Symptom | Likelihood |
|
||||
|----------|---------|------------|
|
||||
| **Path A → `active_subs` is empty** (no `finance.org_subscriptions` row) | Falls to Path B | Medium |
|
||||
| **Path A → `primary_sub.tier` is `None`** (FK broken, tier missing) | Skips limit calc, `subscription_summary` stays `None` | Low |
|
||||
| **Path B → `org.subscription_tier` points to wrong tier** (old FK mismatch) | Wrong limits returned | Medium |
|
||||
| **`org.subscription_tier.rules` is `None`** (tier exists but rules column is NULL) | `{} or None` → `{}` → default | Low |
|
||||
|
||||
---
|
||||
|
||||
## 5. RECOMMENDATIONS
|
||||
|
||||
### ✅ Short-term: Verify runtime dataflow
|
||||
|
||||
1. **Test with exact org_id** that shows the 0 limit:
|
||||
```sql
|
||||
SELECT os.id, os.org_id, os.tier_id, st.name, st.rules IS NOT NULL as has_rules
|
||||
FROM finance.org_subscriptions os
|
||||
JOIN system.subscription_tiers st ON os.tier_id = st.id
|
||||
WHERE os.org_id = <problematic_org_id> AND os.is_active = true;
|
||||
```
|
||||
|
||||
2. **Check if the endpoint takes Path A or Path B** by inspecting logs/Debug mode.
|
||||
|
||||
### ✅ Long-term: Code improvements (separate ticket)
|
||||
|
||||
1. **Add logging** at key decision points in [`get_organization_details()`](backend/app/api/v1/endpoints/admin_organizations.py:579-614):
|
||||
- Log `active_subs` count
|
||||
- Log which path is taken (A vs B)
|
||||
- Log `primary_rules` keys and extracted limits
|
||||
- Log if `primary_sub.tier` is None
|
||||
|
||||
2. **Add defensive fallback** — if `_extract_limit` returns 0 but `rules` has `allowances`, log a warning:
|
||||
```python
|
||||
base_vehicles = _extract_limit(primary_rules, ["max_vehicles", "max_assets"], fc_fallback)
|
||||
if base_vehicles == 0 and primary_rules.get("allowances", {}).get("max_vehicles"):
|
||||
logger.warning(f"_extract_limit returned 0 despite allowances.max_vehicles={primary_rules['allowances']['max_vehicles']}")
|
||||
```
|
||||
|
||||
### 🔍 Most Probable Root Cause
|
||||
|
||||
The issue is likely that **the org(s) reporting 0 limits do NOT have an active row in `finance.org_subscriptions`** (Path A), and the **legacy Path B is failing** because `org.subscription_tier.rules` is `None` or the old FK points to a tier that doesn't have `allowances` populated.
|
||||
|
||||
**Next step**: Identify the exact org_id(s) that show 0 and check their `finance.org_subscriptions` presence and `fleet.organizations.subscription_tier_id` value.
|
||||
|
||||
---
|
||||
|
||||
## Appendix: Test Script Location
|
||||
|
||||
The standalone test script is at:
|
||||
[`tests/active/test_extract_limit_standalone.py`](tests/active/test_extract_limit_standalone.py)
|
||||
|
||||
Run with:
|
||||
```bash
|
||||
docker compose exec sf_api python3 /app/tests/test_extract_limit_standalone.py
|
||||
```
|
||||
259
docs/p0_subscription_math_trace_report.md
Normal file
259
docs/p0_subscription_math_trace_report.md
Normal file
@@ -0,0 +1,259 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user