From b0db8133a0c202400c9f2c8ebc95221d411f1c5a Mon Sep 17 00:00:00 2001 From: Samir Boulahtit Date: Sun, 15 Feb 2026 11:49:27 +0100 Subject: [PATCH] docs(proposals): add backward compatibility cleanup plan 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 --- .../backward-compatibility-cleanup.md | 182 ++++++++++++++++++ mkdocs.yml | 1 + 2 files changed, 183 insertions(+) create mode 100644 docs/proposals/backward-compatibility-cleanup.md diff --git a/docs/proposals/backward-compatibility-cleanup.md b/docs/proposals/backward-compatibility-cleanup.md new file mode 100644 index 00000000..c97323d1 --- /dev/null +++ b/docs/proposals/backward-compatibility-cleanup.md @@ -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) +``` diff --git a/mkdocs.yml b/mkdocs.yml index 0831b215..989a4ab5 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -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: