docs(proposals): add backward compatibility cleanup plan
All checks were successful
All checks were successful
Audit of all 28 "backward compatibility" instances across the codebase, grouped into 7 cleanup tasks prioritized by impact. App is not live yet so all compat shims should be removed to build clean target state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
182
docs/proposals/backward-compatibility-cleanup.md
Normal file
182
docs/proposals/backward-compatibility-cleanup.md
Normal file
@@ -0,0 +1,182 @@
|
||||
# Backward Compatibility Cleanup Plan
|
||||
|
||||
**Date:** 2026-02-14
|
||||
**Status:** Proposed
|
||||
**Motivation:** The app is not live yet. All backward compatibility shims should be removed to build the clean target state.
|
||||
|
||||
---
|
||||
|
||||
## Inventory: 28 instances found, grouped into 7 cleanup tasks
|
||||
|
||||
### Task 1: Remove legacy `location` field from Inventory model (HIGH)
|
||||
|
||||
**Files:**
|
||||
- `app/modules/inventory/models/inventory.py:33` — legacy `location = Column(String)` field
|
||||
- `app/modules/inventory/services/inventory_service.py:90,158` — writes `location=location` alongside new `bin_location`
|
||||
|
||||
**What exists:** The old single `location` field (e.g., `"SA-10-02"`) is kept in parallel with the new `warehouse` + `bin_location` split. Every inventory write populates both.
|
||||
|
||||
**Cleanup:**
|
||||
1. Remove `location` column from `Inventory` model
|
||||
2. Remove `location=location` from both create paths in `inventory_service.py`
|
||||
3. Search all reads of `.location` and migrate to `.bin_location` or `.warehouse`
|
||||
4. Update `__repr__` method
|
||||
5. Create Alembic migration to drop the column (data already in `bin_location`)
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Remove dashboard backward-compat helpers + legacy stat fields (HIGH)
|
||||
|
||||
**Files:**
|
||||
- `app/modules/core/routes/api/admin_dashboard.py:73-107,146,149` — helper functions `_extract_metric_value()`, `_widget_list_item_to_dict()`, `_extract_widget_items()` that transform new aggregator data to old response format
|
||||
- `app/modules/analytics/services/stats_service.py:294-299,494-498` — duplicate legacy fields (`total_stores`, `active_stores`, `total_imports`, `completed_imports`, etc.) alongside new schema-compatible fields
|
||||
|
||||
**What exists:** The dashboard endpoint transforms the new provider-based aggregator output into an old flat response. Stats service returns both old and new field names.
|
||||
|
||||
**Cleanup:**
|
||||
1. Remove the three `_extract_*` helper functions from `admin_dashboard.py`
|
||||
2. Return aggregator data directly from endpoints
|
||||
3. Remove legacy field names from `get_store_statistics()` and `get_import_statistics()` in `stats_service.py`
|
||||
4. Keep only the schema-compatible field names (`total`, `verified`, `pending`, etc.)
|
||||
5. Update frontend to consume the new field names
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Remove re-export shims and aliases (MEDIUM)
|
||||
|
||||
**Files:**
|
||||
- `app/core/frontend_detector.py:200-207` — `get_frontend_type()` wrapper around `FrontendDetector.detect()`
|
||||
- `app/modules/task_base.py:169-170` — `DatabaseTask = ModuleTask` alias
|
||||
- `app/modules/cms/routes/api/admin.py:26-27` — `router = admin_router` alias
|
||||
- `app/modules/cms/routes/store.py` — entire file re-exports `store_router` from `routes/api/store.py`
|
||||
- `app/modules/payments/routes/__init__.py` — re-exports `admin_router`, `store_router` from `routes/api/`
|
||||
- `app/modules/analytics/schemas/stats.py:18-34,154-169` — re-exports core dashboard schemas
|
||||
- `app/modules/billing/services/subscription_service.py:33` — re-exports `SubscriptionNotFoundException`
|
||||
- `app/modules/tenancy/models/admin.py:60-61` — comment about `AdminNotification` re-export via `models/database/__init__.py`
|
||||
- `app/modules/dev_tools/models/test_run.py:7` — re-export comment
|
||||
- `app/modules/dev_tools/models/architecture_scan.py:7` — re-export comment
|
||||
- `app/modules/dev_tools/tasks/test_runner.py:7` — re-export comment
|
||||
- `app/modules/dev_tools/tasks/code_quality.py:7` — re-export comment
|
||||
- `app/modules/dev_tools/schemas/__init__.py:6` — re-export comment
|
||||
|
||||
**What exists:** A collection of aliases, wrapper functions, and re-export modules that allow old import paths to keep working after code was moved or renamed.
|
||||
|
||||
**Cleanup (per item):**
|
||||
|
||||
| Item | Action |
|
||||
|------|--------|
|
||||
| `get_frontend_type()` | Delete function. Update callers to `FrontendDetector.detect()` |
|
||||
| `DatabaseTask = ModuleTask` | Delete alias. Rename all `DatabaseTask` imports to `ModuleTask` |
|
||||
| `router = admin_router` (CMS) | Delete alias. Update imports to use `admin_router` |
|
||||
| `cms/routes/store.py` | Delete file. Update `cms/definition.py` to import from `routes/api/store` |
|
||||
| `payments/routes/__init__.py` | Clear re-exports. Update `payments/definition.py` to import from `routes/api/` |
|
||||
| `analytics/schemas/stats.py` re-exports | Remove re-export block. Update all imports to `core.schemas.dashboard` |
|
||||
| `SubscriptionNotFoundException` re-export | Remove import. Update callers to import from `billing.exceptions` |
|
||||
| `AdminNotification` re-export | Find and remove from `models/database/__init__.py`. Update imports to `messaging.models.admin_notification` |
|
||||
| dev_tools comments | Remove backward compat comments. Verify no legacy import paths exist |
|
||||
|
||||
---
|
||||
|
||||
### Task 4: Remove module-enabling JSON fallback (MEDIUM)
|
||||
|
||||
**Files:**
|
||||
- `app/modules/service.py:140-174` — falls back from `PlatformModule` junction table to `Platform.settings["enabled_modules"]` JSON, then to "enable all modules"
|
||||
|
||||
**What exists:** Three-tier fallback for determining which modules are enabled:
|
||||
1. `PlatformModule` junction table (new, auditable)
|
||||
2. `Platform.settings["enabled_modules"]` JSON field (old)
|
||||
3. Enable all modules if nothing configured (oldest)
|
||||
|
||||
**Cleanup:**
|
||||
1. Remove the JSON fallback path (lines ~166-174)
|
||||
2. Remove the "enable all" default
|
||||
3. Require explicit `PlatformModule` records
|
||||
4. Seed scripts should create `PlatformModule` entries for all platforms
|
||||
5. Remove `_migrate_json_to_junction_table()` helper if no longer needed
|
||||
6. Update `app/modules/service.py` docstring
|
||||
|
||||
---
|
||||
|
||||
### Task 5: Remove marketplace title/description compat + menu legacy format (MEDIUM)
|
||||
|
||||
**Files:**
|
||||
- `app/modules/marketplace/services/marketplace_product_service.py:116-118` — strips `title`/`description` from product data dict before DB insert (old fields moved to translations table)
|
||||
- `app/modules/core/services/menu_service.py:263` — converts new menu structure to legacy format via `menu_to_legacy_format()`
|
||||
|
||||
**Cleanup:**
|
||||
1. **Marketplace:** Remove `product_dict.pop("title", None)` and `product_dict.pop("description", None)`. Remove these fields from `MarketplaceProductCreate`/`MarketplaceProductUpdate` schemas if present.
|
||||
2. **Menu:** Remove `menu_to_legacy_format()` call. Return new menu structure directly. Update templates to consume the new format.
|
||||
|
||||
---
|
||||
|
||||
### Task 6: Clean up billing convenience methods (LOW)
|
||||
|
||||
**Files:**
|
||||
- `app/modules/billing/services/subscription_service.py:164` — `get_subscription_for_store()` resolves store → merchant → platform → subscription
|
||||
- `app/modules/billing/services/feature_service.py:223` — `has_feature_for_store()` resolves store → merchant/platform → feature check
|
||||
|
||||
**What exists:** Convenience methods that hide the merchant/platform hierarchy from store-level callers.
|
||||
|
||||
**Cleanup:**
|
||||
1. Find all callers of `get_subscription_for_store()` and `has_feature_for_store()`
|
||||
2. Refactor callers to resolve the merchant/platform explicitly
|
||||
3. Remove the convenience methods
|
||||
4. Or: rename them to `resolve_subscription_for_store()` and drop the "backward compat" comment — these may be genuinely useful abstractions even in the target state
|
||||
|
||||
**Note:** These might actually be worth keeping as they provide a clean API. The decision is whether the store→merchant→platform resolution is truly "backward compat" or just a useful service abstraction. If kept, just remove the "backward compatibility" comments.
|
||||
|
||||
---
|
||||
|
||||
### Task 7: Clean up test fixtures and test comments (LOW)
|
||||
|
||||
**Files:**
|
||||
- `tests/fixtures/auth_fixtures.py:52,133` — `is_super_admin=True` on test_admin fixtures "for backward compatibility"
|
||||
- `tests/integration/middleware/test_merchant_domain_flow.py:10,98` — test verifying subdomain routing "backward compatibility"
|
||||
- `app/modules/tenancy/schemas/store.py:209` — language field defaults "for backward compatibility"
|
||||
- `app/modules/tenancy/routes/api/admin_stores.py:213` — deprecated store-level ownership transfer endpoint
|
||||
- `app/modules/registry.py:67-79` — `__getattr__` lazy module variables
|
||||
- `alembic/versions_backup/` — migration references (leave as-is, it's history)
|
||||
|
||||
**Cleanup:**
|
||||
|
||||
| Item | Action |
|
||||
|------|--------|
|
||||
| Auth fixtures | Remove "backward compat" comment. Decide if `test_admin` should be super_admin or not based on what tests actually need |
|
||||
| Merchant domain test | Remove "backward compat" from docstring. This is just testing subdomain routing — a current feature, not a compat shim |
|
||||
| Store schema language defaults | Remove "backward compat" comment. These are just sensible defaults |
|
||||
| Admin stores endpoint | Remove deprecated ownership transfer endpoint entirely if merchant-level endpoint exists |
|
||||
| Registry `__getattr__` | Either migrate all callers to function calls, or keep the pattern and remove the "backward compat" comment (lazy loading is a valid pattern) |
|
||||
| Alembic backup | Leave as-is — migration history should not be modified |
|
||||
|
||||
---
|
||||
|
||||
## Execution Order
|
||||
|
||||
```
|
||||
Task 1 (inventory location) ████████████ HIGH — DB column removal, needs migration
|
||||
Task 2 (dashboard/stats) ████████████ HIGH — API response shape change
|
||||
Task 3 (re-export shims) ████████ MEDIUM — mechanical find-and-replace
|
||||
Task 4 (module-enabling JSON) ████████ MEDIUM — seed script update needed
|
||||
Task 5 (marketplace/menu) ████████ MEDIUM — schema + template changes
|
||||
Task 6 (billing convenience) ████ LOW — may decide to keep
|
||||
Task 7 (tests/comments) ████ LOW — mostly comment removal
|
||||
```
|
||||
|
||||
Tasks 1-2 are the most impactful (actual dead code/fields). Tasks 3-5 are mechanical cleanup. Tasks 6-7 are mostly comment/naming hygiene.
|
||||
|
||||
---
|
||||
|
||||
## Verification
|
||||
|
||||
After each task:
|
||||
```bash
|
||||
python -c "from main import app; print('OK')"
|
||||
python -m pytest tests/ app/modules/*/tests/ -v --timeout=60
|
||||
python scripts/validate/validate_architecture.py -v
|
||||
```
|
||||
|
||||
After all tasks:
|
||||
```bash
|
||||
grep -rn "backward.compat\|backwards.compat" --include="*.py" | grep -v alembic
|
||||
# Should return 0 results (alembic history excluded)
|
||||
```
|
||||
@@ -234,6 +234,7 @@ nav:
|
||||
- Loyalty Program Analysis: proposals/loyalty-program-analysis.md
|
||||
- Permissions Plan: proposals/plan-perms.md
|
||||
- Validator Noqa & Remaining Findings: proposals/validator-noqa-suppressions-and-remaining-findings.md
|
||||
- Backward Compatibility Cleanup: proposals/backward-compatibility-cleanup.md
|
||||
|
||||
# --- Archive ---
|
||||
- Archive:
|
||||
|
||||
Reference in New Issue
Block a user