feat: add merchant user edit page with editable profile fields

- Add /admin/merchant-users/{id}/edit page route and template - Replace toggle-status button with edit button on merchant-users list - Editable fields: username, email, first name, last name - Quick actions: toggle status, delete (with double confirm) - Move RBAC two-phase plan to docs/proposals/ Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-19 23:30:50 +01:00
parent 51e512ec08
commit b9ac252a9f
6 changed files with 892 additions and 47 deletions
--- a/docs/proposals/rbac-cleanup-two-phase-plan.md
+++ b/docs/proposals/rbac-cleanup-two-phase-plan.md
@@ -0,0 +1,356 @@
+# RBAC Cleanup: Two-Phase Plan
+
+> **Phase 1 Status: COMPLETED** (2026-02-19) — All 13 steps implemented, migration `tenancy_003` applied, 1081 tests passing.
+>
+> **Phase 2 Status: PLANNED** — Pending future sprint.
+
+## Context
+
+The current role/permission system is fragmented across 4 mechanisms and 5 tables, creating confusion and bugs:
+
+| Mechanism | Where | What it determines |
+|-----------|-------|-------------------|
+| `User.role` | `users.role` (String: "admin"/"store") | Admin vs Store user |
+| `User.is_super_admin` | `users.is_super_admin` (Boolean) | Super admin privileges |
+| `StoreUser.user_type` | `store_users.user_type` ("owner"/"member") | **STALE** — ownership moved to `Merchant.owner_user_id` |
+| `StoreUser.role_id` -> `Role.permissions` | `store_users` + `roles` | Per-store granular permissions |
+
+**Key issues:**
+1. **Stale "store owner" concept** — ownership lives at `Merchant.owner_user_id`, but `StoreUser(user_type="owner")` entries are still created as redundant mirrors (~20 files)
+2. **Bug:** `merchant_service.py:72` sets `role="user"` on new merchant owners — not a valid `UserRole` value, breaks store portal login
+3. **Naming chaos** — UI shows "Type" for admins / "Role" for merchants, but DB field names are the opposite
+4. **`is_super_admin` boolean** bolted onto a 2-value enum instead of being a proper role level
+5. **147 references** to `is_super_admin` across 29 files; 30 auth dependency functions in `deps.py`
+
+**This plan covers two phases — neither to be executed before the client demo:**
+- **Phase 1** (post-demo): Pragmatic cleanup, ~50 files, fixes real bugs
+- **Phase 2** (later sprint): Full unified role assignment system, ~80-100 files
+
+---
+
+## Phase 1 — Pragmatic Cleanup (~50 files) [COMPLETED]
+
+**Goal:** Consolidate `User.role` + `User.is_super_admin` into a single 4-value enum. Remove stale `StoreUser.user_type`. Fix the `role="user"` bug. Keep `StoreUser.role_id` -> `Role.permissions` unchanged (it works fine for per-store granular perms).
+
+### Step 1.1 — Expand `UserRole` enum
+
+**File:** `app/modules/tenancy/models/user.py`
+
+```python
+# BEFORE
+class UserRole(str, Enum):
+    ADMIN = "admin"
+    STORE = "store"
+
+# AFTER
+class UserRole(str, Enum):
+    SUPER_ADMIN = "super_admin"        # Platform super administrator
+    PLATFORM_ADMIN = "platform_admin"  # Platform admin (scoped to assigned platforms)
+    MERCHANT_OWNER = "merchant_owner"  # Owns merchant(s) and all their stores
+    STORE_MEMBER = "store_member"      # Team member on specific store(s)
+```
+
+**Mapping from old to new:**
+| Old state | New `UserRole` |
+|-----------|---------------|
+| `role="admin"` + `is_super_admin=True` | `SUPER_ADMIN` |
+| `role="admin"` + `is_super_admin=False` | `PLATFORM_ADMIN` |
+| `role="store"` + owns merchant(s) | `MERCHANT_OWNER` |
+| `role="store"` + no merchant ownership | `STORE_MEMBER` |
+| `role="user"` (bug) | `MERCHANT_OWNER` (fix) |
+
+### Step 1.2 — Migration: data migration + drop `is_super_admin` + drop `user_type`
+
+**File:** `app/modules/tenancy/migrations/versions/tenancy_00X_consolidate_user_role.py` (NEW)
+
+1. Widen `users.role` column to accept new values
+2. Data migration:
+   ```sql
+   UPDATE users SET role = 'super_admin' WHERE role = 'admin' AND is_super_admin = true;
+   UPDATE users SET role = 'platform_admin' WHERE role = 'admin' AND is_super_admin = false;
+   UPDATE users SET role = 'merchant_owner' WHERE role = 'store' AND id IN (SELECT owner_user_id FROM merchants);
+   UPDATE users SET role = 'merchant_owner' WHERE role = 'user';  -- fix bug
+   UPDATE users SET role = 'store_member' WHERE role = 'store' AND id NOT IN (SELECT owner_user_id FROM merchants);
+   ```
+3. Drop `users.is_super_admin` column
+4. Drop `store_users.user_type` column
+
+### Step 1.3 — Update User model
+
+**File:** `app/modules/tenancy/models/user.py`
+
+- Remove `is_super_admin` column definition
+- Add backward-compat properties:
+  ```python
+  @property
+  def is_super_admin(self):
+      return self.role == UserRole.SUPER_ADMIN.value
+
+  @property
+  def is_admin(self):
+      return self.role in (UserRole.SUPER_ADMIN.value, UserRole.PLATFORM_ADMIN.value)
+
+  @property
+  def is_merchant_owner(self):
+      return self.role == UserRole.MERCHANT_OWNER.value
+  ```
+
+**File:** `app/modules/tenancy/models/store.py`
+
+- Remove `StoreUserType` enum
+- Remove `StoreUser.user_type` column
+- Update `StoreUser.is_owner` property to check via `User.is_owner_of(store_id)` instead of `user_type`
+
+### Step 1.4 — Update auth dependencies
+
+**File:** `app/api/deps.py` (~30 functions)
+
+| Old check | New check |
+|-----------|-----------|
+| `role == "admin"` | `role in ("super_admin", "platform_admin")` |
+| `role == "admin" and is_super_admin` | `role == "super_admin"` |
+| `role == "store"` | `role in ("merchant_owner", "store_member")` |
+
+Key functions:
+- `get_current_admin_api()` -> check `is_admin` property
+- `get_current_super_admin_api()` -> check `role == "super_admin"`
+- `get_current_store_api()` -> check role in `(merchant_owner, store_member)`
+- `get_current_merchant_api()` -> check `role == "merchant_owner"`
+- `require_store_owner()` -> already uses `User.is_owner_of()` (no change needed)
+- `require_store_permission()` -> already uses `User.has_store_permission()` (no change needed)
+
+### Step 1.5 — Update JWT token creation + backward-compat shim
+
+**File:** `app/middleware/auth.py`
+
+- `create_access_token()`: Remove `is_super_admin` from payload — `role` field now carries all info
+- `verify_token()`: Add backward-compat shim — if token has old `role="admin"` + `is_super_admin=True`, map to `"super_admin"`. Remove shim after 1 release cycle.
+
+### Step 1.6 — Update auth service
+
+**File:** `app/modules/core/services/auth_service.py`
+
+- `login_user()` / `login_merchant()` — remove `is_super_admin` from token data
+- `get_user_store_role()` — derive from `User.role` instead of checking `StoreUser.user_type`
+
+### Step 1.7 — Fix merchant creation bug
+
+**File:** `app/modules/tenancy/services/merchant_service.py`
+- Line 72: Change `role="user"` -> `role=UserRole.MERCHANT_OWNER.value`
+
+**File:** `app/modules/marketplace/services/platform_signup_service.py`
+- Remove `StoreUser(user_type=StoreUserType.OWNER.value)` creation — ownership is via `Merchant.owner_user_id`
+- Set `role=UserRole.MERCHANT_OWNER.value` on user creation
+
+### Step 1.8 — Update store team service
+
+**File:** `app/modules/tenancy/services/store_team_service.py`
+
+- `remove_team_member()`: Replace `store_user.is_owner` check -> `user.is_owner_of(store_id)`
+- `update_member_role()`: Same replacement
+- `get_team_members()`: Derive `is_owner` from `User.is_owner_of()` instead of `StoreUser.user_type`
+- `add_team_member()`: Remove `user_type` assignment, ensure new members get `role=STORE_MEMBER`
+
+### Step 1.9 — Update seed data
+
+**File:** `scripts/seed/seed_demo.py`
+- Remove `StoreUser(user_type="owner")` creation for demo merchants
+- Use new `UserRole` values for all seeded users
+
+**File:** `scripts/seed/init_production.py`
+- Update super admin creation: `role="super_admin"` instead of `role="admin", is_super_admin=True`
+
+### Step 1.10 — Update admin API routes + responses
+
+**Files:**
+- `app/modules/tenancy/routes/api/admin_users.py` — `AdminUserResponse`: remove `is_super_admin`, the `role` field is sufficient
+- `app/modules/tenancy/routes/api/admin_platform_users.py` — merchant user listing: derive owner status from `User.role == "merchant_owner"` not `owned_merchants_count`
+
+### Step 1.11 — Update admin UI templates + JS
+
+**Files:**
+- `app/modules/tenancy/templates/tenancy/admin/admin-users.html` — Change "Type" header -> "Role", display role value (super_admin / platform_admin badge)
+- `app/modules/tenancy/templates/tenancy/admin/merchant-users.html` — Display role value (merchant_owner / store_member badge) instead of deriving from `owned_merchants_count`
+- `app/modules/tenancy/static/admin/js/admin-users.js` — Update role badge rendering
+- `app/modules/tenancy/static/admin/js/merchant-users.js` — Update role badge rendering
+
+### Step 1.12 — Bulk update remaining `is_super_admin` references (~29 files, 147 occurrences)
+
+Search-and-replace across:
+- All route files checking `is_super_admin` -> use `user.is_super_admin` property (backed by enum check, so these work as-is via the compat property)
+- All template files displaying `is_super_admin` -> display `role` value
+- All JS files referencing `is_super_admin` -> use `role` field
+- All test files -> update assertions
+
+**Key files by reference count:**
+- `app/api/deps.py` — 15+ references
+- `app/middleware/auth.py` — 8+ references
+- `app/modules/tenancy/routes/api/admin_users.py` — 5+ references
+- Various admin JS files — 3-5 references each
+
+### Step 1.13 — Update RBAC documentation
+
+**Files:**
+- `docs/api/rbac.md` — Update role hierarchy, remove is_super_admin references
+- `docs/api/rbac-visual-guide.md` — Update diagrams
+- `docs/backend/rbac-quick-reference.md` — Update quick reference tables
+- `docs/backend/store-rbac.md` — Update store-level section
+- `docs/architecture/auth-rbac.md` — Update architecture overview
+
+### Phase 1 File Summary
+
+| Category | Files | Change |
+|----------|-------|--------|
+| Models | 2 | Modify User, StoreUser |
+| Migration | 1 | New data migration |
+| Auth (deps + middleware + service) | 3 | Modify |
+| Services | 3 | Modify (merchant, signup, team) |
+| API routes | 4-6 | Modify response schemas + checks |
+| Admin templates (HTML) | 2-4 | Modify badges/labels |
+| Admin JS | 4-6 | Modify role rendering |
+| Seed scripts | 2-3 | Modify |
+| Tests | 10-15 | Update assertions |
+| Documentation | 5-6 | Update |
+| **Total** | **~50** | |
+
+### Phase 1 Verification
+
+1. `alembic upgrade head` — migration applies, data migrated correctly
+2. `SELECT role, count(*) FROM users GROUP BY role` — only 4 new values
+3. `SELECT * FROM users WHERE role IN ('admin', 'store', 'user')` — empty (all migrated)
+4. `is_super_admin` column gone from users table
+5. `user_type` column gone from store_users table
+6. Admin login still works (JWT backward compat shim)
+7. Store login still works
+8. Merchant creation sets `role="merchant_owner"`
+9. Super admin can access all admin routes
+10. Platform admin scoped correctly
+11. Store team member permissions unchanged
+12. `pytest` — all tests pass
+
+---
+
+## Phase 2 — Target State (Later Sprint, ~80-100 files)
+
+**Goal:** Replace all current role mechanisms with a unified, context-aware role assignment system. One `roles` table, one `permissions` table, one `user_role_assignments` junction table that handles all contexts (platform, merchant, store).
+
+### Target Schema
+
+```
+users                    user_role_assignments         roles
+--------------+         +-------------------------+   +--------------+
+| id           |----<    | user_id (FK)            |>--| id           |
+| email        |         | role_id (FK)            |   | name         |
+| password_hash|         | context_type (enum)     |   | context_type |
+| first_name   |         |   "platform"            |   | is_system    |
+| last_name    |         |   "merchant"            |   | created_at   |
+| ...          |         |   "store"               |   +------+-------+
+| (NO role col)|         | context_id (nullable)   |          |
+| (NO is_super)|         |   NULL = global         |   role_permissions
+              +         |   platform.id           |   +--------------+
+                         |   merchant.id           |   | role_id (FK) |
+                         |   store.id              |   | permission   |
+                         | granted_by (FK->users)  |   |   (string)   |
+                         | created_at              |   +--------------+
+                         +-------------------------+
+```
+
+### Target Role Definitions
+
+**System roles (seeded, immutable):**
+
+| Role | context_type | Description |
+|------|-------------|-------------|
+| `super_admin` | platform | Full platform access, all permissions |
+| `platform_admin` | platform | Scoped to assigned platform(s) |
+| `merchant_owner` | merchant | Owns merchant, manages all stores |
+| `store_manager` | store | Full store management |
+| `store_staff` | store | Day-to-day operations |
+| `store_support` | store | Customer support only |
+| `store_viewer` | store | Read-only access |
+| `store_marketing` | store | Marketing and content |
+
+**Custom roles:** Merchants can create custom roles at store context level with specific permission sets.
+
+### How Assignments Work
+
+```
+# Super admin — global platform access
+user_role_assignments(user_id=1, role=super_admin, context_type="platform", context_id=NULL)
+
+# Platform admin — scoped to platform 3
+user_role_assignments(user_id=2, role=platform_admin, context_type="platform", context_id=3)
+
+# Merchant owner — owns merchant 5
+user_role_assignments(user_id=3, role=merchant_owner, context_type="merchant", context_id=5)
+
+# Store member — staff role on store 12
+user_role_assignments(user_id=4, role=store_staff, context_type="store", context_id=12)
+
+# Same user, different stores, different roles
+user_role_assignments(user_id=4, role=store_manager, context_type="store", context_id=15)
+```
+
+### Phase 2 Steps (High-Level)
+
+**2.1 — New models**
+- `Role` model (replaces current per-store `roles` table) — now context-aware
+- `RolePermission` model — maps role -> permission strings
+- `UserRoleAssignment` model — the unified junction table
+
+**2.2 — Migration**
+- Create new tables
+- Migrate data from:
+  - `users.role` -> `user_role_assignments` (platform/merchant context)
+  - `store_users` + `store_users.role_id` -> `user_role_assignments` (store context)
+  - `admin_platforms` -> `user_role_assignments` (platform context)
+  - Current `roles` + `role.permissions` -> new `roles` + `role_permissions`
+- Drop old tables: `store_users`, `admin_platforms`, old `roles`
+- Drop `users.role` column
+
+**2.3 — Auth service rewrite**
+- `get_user_roles(user_id, context_type?, context_id?)` — query assignments
+- `has_permission(user_id, permission, context_type, context_id)` — check via role -> permissions
+- Permission inheritance: super_admin > platform_admin > merchant_owner > store roles
+
+**2.4 — Auth dependencies rewrite**
+- Replace all 30 dependency functions with a permission-based system:
+  ```python
+  # Instead of: Depends(get_current_admin_api)
+  # Use: Depends(require_permission("stores.manage"))
+  ```
+- Role hierarchy handles escalation automatically
+
+**2.5 — JWT simplification**
+- Token payload: `{user_id, active_context: {type, id}, permissions: [...]}`
+- No more role strings in token — permissions resolved at login
+
+**2.6 — Admin UI**
+- Role management page (CRUD roles + permissions)
+- User role assignment UI (assign roles with context)
+- Remove separate admin-users / merchant-users pages -> unified users page with role filter
+
+**2.7 — Drop legacy**
+- Remove `store_users` table
+- Remove `admin_platforms` table
+- Remove old `roles` table
+- Remove `users.role` column
+- Remove all backward-compat properties
+
+### Phase 2 Benefits
+
+1. **Single source of truth** — all role info in `user_role_assignments`
+2. **Context-aware** — same user can have different roles in different stores/merchants
+3. **Extensible** — add new contexts (e.g., "warehouse") without schema changes
+4. **Auditable** — `granted_by` tracks who assigned roles
+5. **Custom roles** — merchants define their own roles with specific permissions
+6. **No derived logic** — no checking `owned_merchants_count` or `StoreUser.user_type`
+
+---
+
+## Execution Order
+
+1. **Phase 1 first** (post-demo) — safe, incremental, fixes real bugs
+2. **Phase 2 later** — larger refactor, needs dedicated sprint, ideally with test coverage from Phase 1
+
+Phase 1 is designed so Phase 2 builds cleanly on top of it. The 4-value enum from Phase 1 maps 1:1 to the system roles in Phase 2.