admin felület fejlesztése garázs, előfizeztési csomagok
This commit is contained in:
Roo
201
docs/p0_save_bug_pricing_schema_audit_report_2026-06-24.md
Normal file
201
docs/p0_save_bug_pricing_schema_audit_report_2026-06-24.md
Normal file
@@ -0,0 +1,201 @@
|
||||
# P0 AUDIT REPORT: Save Bug & Pricing Schema Investigation
|
||||
|
||||
**Date:** 2026-06-24
|
||||
**Status:** Read-Only Investigation Complete
|
||||
**Files Audited:** 4 (+1 DB query)
|
||||
**Bugs Found:** 3 (1 Critical, 1 Medium, 1 Low)
|
||||
|
||||
---
|
||||
|
||||
## 1. FRONTEND PAYLOAD AUDIT
|
||||
|
||||
### File: [`frontend_admin/pages/packages/index.vue`](../frontend_admin/pages/packages/index.vue)
|
||||
|
||||
#### ✅ `is_default_fallback` and `trial_days_on_signup` ARE in payload
|
||||
|
||||
Both fields are correctly mapped in both POST (create) and PATCH (update):
|
||||
|
||||
- **POST (lines 670-687):** Sends both fields at root level.
|
||||
```javascript
|
||||
body: {
|
||||
name: form.name.trim().toLowerCase(),
|
||||
rules: rulesPayload,
|
||||
is_custom: form.is_custom,
|
||||
tier_level: form.tier_level,
|
||||
feature_capabilities: {},
|
||||
is_default_fallback: form.is_default_fallback, // ✅ Present
|
||||
trial_days_on_signup: form.trial_days_on_signup, // ✅ Present
|
||||
}
|
||||
```
|
||||
|
||||
- **PATCH (lines 697-704):** Sends both fields at root level.
|
||||
```javascript
|
||||
body: {
|
||||
name: form.name.trim().toLowerCase(),
|
||||
rules: rulesPayload,
|
||||
is_custom: form.is_custom,
|
||||
tier_level: form.tier_level,
|
||||
is_default_fallback: form.is_default_fallback, // ✅ Present
|
||||
trial_days_on_signup: form.trial_days_on_signup, // ✅ Present
|
||||
}
|
||||
```
|
||||
|
||||
#### ❌ `rulesPayload` structure (lines 646-668)
|
||||
|
||||
The `rulesPayload` is reconstructed fresh on every save with a flat structure:
|
||||
|
||||
```javascript
|
||||
const rulesPayload: any = {
|
||||
type: form.type,
|
||||
display_name: form.display_name || null,
|
||||
pricing_zones: {
|
||||
DEFAULT: { // ⚠️ ONLY DEFAULT ZONE!
|
||||
monthly_price: form.monthly_price,
|
||||
yearly_price: form.yearly_price,
|
||||
currency: form.currency.toUpperCase(),
|
||||
},
|
||||
},
|
||||
allowances: { ... },
|
||||
marketing: { ... },
|
||||
lifecycle: { is_public: true }, // ⚠️ available_until NOT included
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 2. BACKEND API AUDIT
|
||||
|
||||
### File: [`backend/app/api/v1/endpoints/admin_packages.py`](../backend/app/api/v1/endpoints/admin_packages.py)
|
||||
|
||||
#### ✅ Singleton Logic is CORRECT
|
||||
|
||||
- **`is_default_fallback` True (lines 283-294):** Correctly clears fallback on ALL OTHER tiers using `SubscriptionTier.id != tier_id`, then sets current to True.
|
||||
- **`is_default_fallback` False (lines 295-296):** Sets current tier to False. Safe.
|
||||
- **`trial_days_on_signup` > 0 (lines 299-310):** Correctly clears all other tiers using `SubscriptionTier.id != tier_id`.
|
||||
|
||||
No logical flaw found in the singleton implementation.
|
||||
|
||||
#### ✅ `exclude_unset=True` is used correctly
|
||||
|
||||
Line 248: `update_data = payload.model_dump(exclude_unset=True)` — This ensures that only the fields the frontend sends are applied, leaving untouched fields as-is.
|
||||
|
||||
#### ❌ Lifecycle preservation logic has a gap (lines 270-272)
|
||||
|
||||
```python
|
||||
if isinstance(new_rules, dict) and new_rules.get("lifecycle") is None and tier.rules.get("lifecycle"):
|
||||
new_rules["lifecycle"] = tier.rules["lifecycle"]
|
||||
```
|
||||
|
||||
This preserves the existing lifecycle **only if `lifecycle` is absent** from the incoming payload. But the frontend **always sends** `lifecycle: { is_public: true }` (without `available_until`), so the condition `new_rules.get("lifecycle") is None` is **FALSE**, and the existing `available_until` gets overwritten/lost.
|
||||
|
||||
---
|
||||
|
||||
## 3. PRICING SCHEMA AUDIT
|
||||
|
||||
### Database Schema: [`system.subscription_tiers`](../backend/app/models/core_logic.py)
|
||||
|
||||
| Column | Type | Notes |
|
||||
|--------|------|-------|
|
||||
| `id` | `INTEGER PK` | |
|
||||
| `name` | `VARCHAR` | Unique, indexed |
|
||||
| `rules` | **`JSONB`** | **All pricing stored here** |
|
||||
| `is_custom` | `BOOLEAN` | Default: false |
|
||||
| `feature_capabilities` | `JSONB` | Default: `{}` |
|
||||
| `tier_level` | `INTEGER` | Default: 0 |
|
||||
| `is_default_fallback` | `BOOLEAN` | Default: false, singleton |
|
||||
| `trial_days_on_signup` | `INTEGER` | Default: 0, singleton |
|
||||
|
||||
**There are NO dedicated pricing columns.** The price is stored entirely inside the `rules` JSONB column.
|
||||
|
||||
### Pricing JSONB Structure
|
||||
|
||||
Current schema via [`SubscriptionRulesModel`](../backend/app/schemas/subscription.py:86):
|
||||
|
||||
```json
|
||||
{
|
||||
"pricing_zones": {
|
||||
"HU": { "monthly_price": 990.0, "yearly_price": 9990.0, "currency": "HUF", "credit_price": 25000 },
|
||||
"US": { "monthly_price": 3.5, "yearly_price": 35.9, "currency": "USD", "credit_price": 25000 },
|
||||
"DEFAULT": { "monthly_price": 2.99, "yearly_price": 29.9, "currency": "EUR", "credit_price": 25000 }
|
||||
},
|
||||
"pricing": { // Legacy single-zone fallback
|
||||
"monthly_price": 2.99, "yearly_price": 29.9, "currency": "EUR", "credit_price": 25000
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Existing data confirms** (from `private_pro_v1`, id=14): The `pricing_zones` dictionary supports **multiple country keys** (HU, US, DEFAULT), each with `PricingZoneModel`:
|
||||
- `monthly_price: float`
|
||||
- `yearly_price: float`
|
||||
- `currency: str` (ISO 4217)
|
||||
- `credit_price: Optional[int]`
|
||||
|
||||
### Multi-Currency / Finance Models
|
||||
|
||||
The project has these finance models that could be linked:
|
||||
- [`OrganizationSubscription`](../backend/app/models/core_logic.py:71) — `finance.org_subscriptions` — links orgs to tiers
|
||||
- [`UserSubscription`](../backend/app/models/core_logic.py:102) — `finance.user_subscriptions` — links users to tiers
|
||||
- [`CreditTransaction`](../backend/app/models/core_logic.py:132) — `finance.credit_logs` — tracks credit usage
|
||||
- `CostCategory` in [`fleet_finance`](../backend/app/models/fleet_finance.py) — cost categories with `min_tier` access control
|
||||
|
||||
---
|
||||
|
||||
## 4. 🐛 BUG REGISTRY
|
||||
|
||||
### 🐛 BUG #1 — CRITICAL: Multi-Zone Pricing Data Loss on Edit
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| **Severity** | 🔴 **CRITICAL** |
|
||||
| **Location** | [`index.vue:609`](../frontend_admin/pages/packages/index.vue:609) (read) + [`index.vue:649-654`](../frontend_admin/pages/packages/index.vue:649-654) (write) |
|
||||
| **Root Cause** | The form is single-zone only. `openEditModal()` reads only `pricing_zones.DEFAULT` (or `rules.pricing`). `savePackage()` writes ONLY `pricing_zones: { DEFAULT: {...} }`. |
|
||||
| **Impact** | Every time an admin edits a package with multiple pricing zones (e.g., `private_pro_v1` with HU, US, DEFAULT), the HU and US zones are **silently destroyed**. This is irreversible data loss. |
|
||||
| **Extracted Code** | [`index.vue:609`](../frontend_admin/pages/packages/index.vue:609): `const pricing = rules.pricing_zones?.DEFAULT \|\| rules.pricing \|\| {}` → reads ONE zone
|
||||
[`index.vue:649-654`](../frontend_admin/pages/packages/index.vue:649-654): `pricing_zones: { DEFAULT: { monthly_price, yearly_price, currency } }` → writes ONLY DEFAULT |
|
||||
| **Fix Suggestion** | The frontend form needs multi-zone pricing support (add/remove/edit zones dynamically). Or, backend PATCH should merge `pricing_zones` instead of replacing them. |
|
||||
|
||||
### 🐛 BUG #2 — MEDIUM: Lifecycle `available_until` Overwritten on Edit
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| **Severity** | 🟡 **MEDIUM** |
|
||||
| **Location** | [`index.vue:665-668`](../frontend_admin/pages/packages/index.vue:665-668) (frontend) + [`admin_packages.py:270-272`](../backend/app/api/v1/endpoints/admin_packages.py:270-272) (backend gap) |
|
||||
| **Root Cause** | Frontend always sends `lifecycle: { is_public: true }` without `available_until`. The backend only preserves existing lifecycle if it's **wholly absent** from the payload. Since frontend always provides it (stripped), `available_until` is lost. |
|
||||
| **Impact** | Packages with time-limited availability (`available_until` set) lose that data on edit. |
|
||||
| **Extracted Code** | [`admin_packages.py:270`](../backend/app/api/v1/endpoints/admin_packages.py:270): `if isinstance(new_rules, dict) and new_rules.get("lifecycle") is None ...` — the guard fails because frontend always sends `lifecycle`. |
|
||||
| **Fix Suggestion** | Backend: merge lifecycle fields individually instead of wholesale replacement. Or frontend: read and include `available_until` from existing data. |
|
||||
|
||||
### 🐛 BUG #3 — LOW: Legacy `pricing` Field Converted to `pricing_zones.DEFAULT`
|
||||
|
||||
| Field | Value |
|
||||
|-------|-------|
|
||||
| **Severity** | 🟢 **LOW** |
|
||||
| **Location** | [`index.vue:609`](../frontend_admin/pages/packages/index.vue:609) + [`index.vue:649-654`](../frontend_admin/pages/packages/index.vue:649-654) |
|
||||
| **Root Cause** | On read, falls back to `rules.pricing` (legacy single-zone). On write, converts to `pricing_zones.DEFAULT` format. The legacy `pricing` field is not preserved. |
|
||||
| **Impact** | Existing packages using legacy `pricing` field get restructured on edit. While functionally equivalent, this changes the data shape. |
|
||||
| **Fix Suggestion** | Backend PATCH endpoint should preserve the legacy `pricing` field if present but not updated. |
|
||||
|
||||
---
|
||||
|
||||
## 5. ARCHITECT SUMMARY
|
||||
|
||||
### What works correctly:
|
||||
1. **`is_default_fallback`** singleton logic — both in `create_package` and `update_package` endpoints
|
||||
2. **`trial_days_on_signup`** singleton logic — both endpoints
|
||||
3. **Pydantic schema validation** — `SubscriptionRulesModel` properly validates the JSONB structure
|
||||
4. **Fallback tier resolution** in `subscription_service.py` — correctly queries `is_default_fallback == True` for expired subscriptions
|
||||
5. **Trial assignment** in `assign_default_trial()` — correctly queries `trial_days_on_signup > 0`
|
||||
|
||||
### What is BROKEN:
|
||||
1. **🔴 CRITICAL: Multi-zone pricing** — every edit destroys all non-DEFAULT pricing zones
|
||||
2. **🟡 MEDIUM: Lifecycle data loss** — `available_until` is silently overwritten on every edit
|
||||
3. **🟢 LOW: Legacy pricing** — `pricing` field is converted to `pricing_zones.DEFAULT` on edit
|
||||
|
||||
### Key architectural note:
|
||||
The pricing model uses **JSONB with `pricing_zones: Dict[str, PricingZoneModel]`** — this is already a flexible multi-currency design, but the **frontend form is single-zone only**, which is the root cause of Bug #1.
|
||||
|
||||
The `SubscriptionTier` model has **no dedicated pricing columns** — everything is in the `rules` JSONB blob. This is consistent with DDD principles but means any pricing display/editing must go through the `SubscriptionRulesModel` Pydantic schema.
|
||||
|
||||
---
|
||||
|
||||
*Report generated by Architect mode. No code was modified during this investigation.*
|
||||
Reference in New Issue
Block a user