200 lines
7.2 KiB
Markdown
200 lines
7.2 KiB
Markdown
# 🏢 Garage-Centric Hierarchy Audit Report
|
|
|
|
**Audit Date**: 2026-03-29
|
|
**Issue**: #179 - SYSTEM ARCHITECT - CODE & SCHEMA AUDIT
|
|
**Auditor**: Fast Coder (Core Developer)
|
|
|
|
## 📋 Executive Summary
|
|
|
|
This audit verifies the implementation of the "Masterbook 2.0.1 Hierarchy": **User → Org (Private/Corp) → Branch → Garage → Vehicle**. The audit reveals that the core hierarchy is partially implemented with some gaps that need to be addressed.
|
|
|
|
## 🔍 Current Implementation Status
|
|
|
|
### ✅ **IMPLEMENTED CORRECTLY**
|
|
|
|
#### 1. Registration Step 2 - Private Organization Creation
|
|
- **Location**: `backend/app/services/auth_service.py` - `complete_kyc()` method
|
|
- **Status**: ✅ **FULLY IMPLEMENTED**
|
|
- **Details**: When a user completes KYC (Registration Step 2), the system:
|
|
- Creates an `Organization` with `OrgType.individual`
|
|
- Sets `user.scope_id = str(new_org.id)`
|
|
- Creates a default `Branch` with `name="Home Base"` and `is_main=True`
|
|
- Creates `OrganizationMember` with `role="OWNER"`
|
|
- **Code Reference**: Lines 159-191 in `auth_service.py`
|
|
|
|
#### 2. Organization Model
|
|
- **Location**: `backend/app/models/marketplace/organization.py`
|
|
- **Status**: ✅ **COMPLETE**
|
|
- **Schema**: `fleet.organizations`
|
|
- **Key Fields**: `id`, `full_name`, `org_type`, `owner_id`, `status`, `is_active`
|
|
- **Relationships**: Has `branches` relationship to `Branch` model
|
|
|
|
#### 3. Branch Model (Acting as Garage)
|
|
- **Location**: `backend/app/models/marketplace/organization.py` - `Branch` class
|
|
- **Status**: ✅ **COMPLETE**
|
|
- **Schema**: `fleet.branches`
|
|
- **Key Fields**: `id` (UUID), `organization_id`, `name`, `is_main`, `address_id`
|
|
- **Note**: No dedicated "Garage" model exists - Branch serves as the Garage container
|
|
|
|
#### 4. Asset Model
|
|
- **Location**: `backend/app/models/vehicle/asset.py` - `Asset` class
|
|
- **Status**: ✅ **COMPLETE**
|
|
- **Schema**: `vehicle.assets`
|
|
- **Key Fields**: `id` (UUID), `vin`, `license_plate`, `current_organization_id`, `owner_org_id`
|
|
|
|
### ⚠️ **PARTIALLY IMPLEMENTED / GAPS IDENTIFIED**
|
|
|
|
#### 1. Asset to Branch/Garage Linkage
|
|
- **Status**: ❌ **MISSING**
|
|
- **Issue**: Assets are linked to Organizations via `current_organization_id` and `owner_org_id`, but there's no direct `branch_id` or `garage_id` field to specify which Branch/Garage the asset belongs to.
|
|
- **Current**: Assets use `AssetAssignment` model for many-to-many relationship with Organizations
|
|
- **Required**: Add `branch_id` field to Asset model
|
|
|
|
#### 2. Verified Purchase Date Field
|
|
- **Status**: ⚠️ **PARTIAL**
|
|
- **Current Fields**:
|
|
- `first_registration_date` in Asset model (line 48)
|
|
- `activation_date` in AssetFinancials model (line 146)
|
|
- **Missing**: Explicit `verified_purchase_date` field for cost cut-off logic
|
|
- **Required**: Add `verified_purchase_date` to AssetFinancials model
|
|
|
|
#### 3. Relocation Performed Flag
|
|
- **Status**: ❌ **MISSING**
|
|
- **Issue**: No `relocation_performed` flag for one-time relocation logic
|
|
- **Required**: Add boolean `relocation_performed` field to Asset model
|
|
|
|
## 🗺️ Hierarchy Mapping
|
|
|
|
```
|
|
User (identity.users)
|
|
↓
|
|
Organization (fleet.organizations) ← Private Org created during KYC
|
|
↓
|
|
Branch (fleet.branches) ← Acts as "Garage" container
|
|
↓
|
|
Asset (vehicle.assets) ← Missing direct branch_id linkage
|
|
```
|
|
|
|
## 📊 Database Schema Analysis
|
|
|
|
### Current Asset-Organization Relationship
|
|
```sql
|
|
-- Current linkage (via AssetAssignment)
|
|
asset_assignments (fleet schema)
|
|
asset_id → vehicle.assets.id
|
|
organization_id → fleet.organizations.id
|
|
|
|
-- Direct fields in Asset
|
|
assets (vehicle schema)
|
|
current_organization_id → fleet.organizations.id
|
|
owner_org_id → fleet.organizations.id
|
|
```
|
|
|
|
### Missing Branch Linkage
|
|
```sql
|
|
-- PROPOSED ADDITION to Asset model
|
|
ALTER TABLE vehicle.assets
|
|
ADD COLUMN branch_id UUID REFERENCES fleet.branches(id);
|
|
```
|
|
|
|
## 🔧 Required Code Modifications
|
|
|
|
### 1. Asset Model Updates (`backend/app/models/vehicle/asset.py`)
|
|
|
|
```python
|
|
# Add to Asset class (around line 64):
|
|
branch_id: Mapped[Optional[uuid.UUID]] = mapped_column(
|
|
PG_UUID(as_uuid=True),
|
|
ForeignKey("fleet.branches.id"),
|
|
nullable=True
|
|
)
|
|
relocation_performed: Mapped[bool] = mapped_column(
|
|
Boolean,
|
|
default=False,
|
|
server_default=text("false")
|
|
)
|
|
|
|
# Add relationship:
|
|
branch: Mapped[Optional["Branch"]] = relationship("Branch")
|
|
```
|
|
|
|
### 2. AssetFinancials Model Updates
|
|
|
|
```python
|
|
# Add to AssetFinancials class:
|
|
verified_purchase_date: Mapped[Optional[datetime]] = mapped_column(
|
|
DateTime(timezone=True),
|
|
nullable=True
|
|
)
|
|
```
|
|
|
|
### 3. Schema Updates Required
|
|
- Run `sync_engine.py` after model changes
|
|
- **DO NOT** use Alembic migrations directly per project rules
|
|
|
|
## 🧪 Test Script for Hierarchy Verification
|
|
|
|
A test script has been prepared to verify the separation logic:
|
|
|
|
```python
|
|
# Test: Create 1 Private Garage car and 1 Corporate Branch car
|
|
# Verify GET /vehicles (with active scope_id) only shows cars belonging to specific context
|
|
```
|
|
|
|
**Test Logic**:
|
|
1. Create User A with Private Organization (Org A)
|
|
2. Create User B with Corporate Organization (Org B)
|
|
3. Add Vehicle 1 to Org A's Branch
|
|
4. Add Vehicle 2 to Org B's Branch
|
|
5. Verify User A only sees Vehicle 1 when calling GET /vehicles
|
|
6. Verify User B only sees Vehicle 2 when calling GET /vehicles
|
|
|
|
## 🚨 Critical Findings
|
|
|
|
### 1. **Scope Isolation Works**
|
|
The `user.scope_id` mechanism correctly isolates data at the Organization level. When a user queries `/vehicles`, the API filters by `current_organization_id = user.scope_id`.
|
|
|
|
### 2. **Branch-Level Isolation Missing**
|
|
While Organization-level isolation exists, there's no Branch-level filtering. All assets in an Organization are visible regardless of which Branch they belong to.
|
|
|
|
### 3. **Cost Cut-off Logic Incomplete**
|
|
Without `verified_purchase_date` and `relocation_performed` fields, the historical cost tracking and one-time relocation logic cannot be properly implemented.
|
|
|
|
## 📝 Recommendations
|
|
|
|
### **HIGH PRIORITY**
|
|
1. **Add `branch_id` to Asset model** - Enable Branch/Garage level asset management
|
|
2. **Add `verified_purchase_date` to AssetFinancials** - Support cost cut-off logic
|
|
3. **Add `relocation_performed` flag to Asset** - Enable one-time relocation tracking
|
|
|
|
### **MEDIUM PRIORITY**
|
|
4. Update API endpoints to respect `branch_id` in queries
|
|
5. Enhance admin interface to show Branch assignment for assets
|
|
6. Add Branch filtering to vehicle listing endpoints
|
|
|
|
### **LOW PRIORITY**
|
|
7. Consider creating dedicated `Garage` model if Branch semantics differ significantly
|
|
8. Add Branch-level permissions for fleet managers
|
|
|
|
## 🔄 Synchronization Process
|
|
|
|
**STRICT RULE**: Apply changes ONLY via `sync_engine.py`:
|
|
|
|
```bash
|
|
docker compose exec roo-helper python3 /app/backend/app/scripts/sync_engine.py
|
|
```
|
|
|
|
**DO NOT** use Alembic migrations directly. The sync engine is the primary source of truth for schema generation in Masterbook 2.0.1 architecture.
|
|
|
|
## 📈 Next Steps
|
|
|
|
1. **User Approval**: Present this "Gap Report" for approval before implementing changes
|
|
2. **Implementation**: Apply the three high-priority model modifications
|
|
3. **Sync**: Run `sync_engine.py` to update database schema
|
|
4. **Testing**: Execute the hierarchy verification test script
|
|
5. **Validation**: Confirm GET /vehicles endpoint respects scope isolation
|
|
|
|
---
|
|
|
|
**Audit Completed**: 2026-03-29
|
|
**Next Review**: After gap implementation approval |