ske087
46602f1933
Remove dead code identified in docs/SANITIZATION-REVIEW.md:
- app/blueprints/content_old.py and app/blueprints/playlist.py
- app/models/group.py, app/utils/nginx_config_reader.py
- orphaned templates (content_list, edit_content, upload_content,
player_page) and the related group/Template references
Result: 6 blueprints, 82 routes, no dead modules or orphan templates.
Add files that deploy.sh and docker-entrypoint.sh already require but
which were never tracked:
- https_manager.py (referenced by deploy.sh, migrate_network.sh,
docker-entrypoint.sh)
- Caddyfile.example (seeded by deploy.sh; its absence aborts deploy)
Relocate generated Graphify artifacts from graphify-out/ to
docs/graphify-out/ (110 files, no content change) and archive the
superseded docs under docs/.
Ignore hygiene:
- ignore ad-hoc .env backups (.env.bak*) — they contain live secrets
- keep the pre-sanitization snapshots (docs/legacy code/,
docs/old_code_documentation/) on disk but out of the repo
Fix .env.example: drop a duplicated config block, genericize the
hardcoded host IP, and document HOSTNAME_INTERNAL.
207 lines
10 KiB
Markdown
207 lines
10 KiB
Markdown
# DigiServer v2 — Code Sanitization Review
|
||
|
||
**Generated:** 2026-09-10
|
||
**Snapshot:** `docs/legacy code/` (1.7 MB, 171 files, 49 Python files) — full restore point.
|
||
|
||
Analysed **42 Python files / 240 functions / 104 routes / 32 templates** under `app/` and `migrations/`.
|
||
|
||
> Review each section below and reply with the IDs you want deleted (e.g. `A1, A2, B1`).
|
||
> Nothing is deleted until you confirm. Everything is recoverable from `docs/legacy code/`.
|
||
|
||
---
|
||
|
||
## ✅ Status — Applied 2026-09-10
|
||
|
||
**Removed (A1, A2, D1, D2):**
|
||
|
||
| ID | Removed | Notes |
|
||
|---|---|---|
|
||
| A1 | `app/blueprints/content_old.py` | + 3 templates orphaned by its removal |
|
||
| A2 | `app/utils/nginx_config_reader.py` | |
|
||
| D1 | 5 group functions + their `__init__.py` exports | `get_player_status_info` kept (live) |
|
||
| D2 | `app/models/group.py`, `Content.groups`, `Content.group_count` | |
|
||
|
||
**Extra cleanup triggered by A1/D2:**
|
||
- Deleted orphaned templates: `upload_content.html` (278), `edit_content.html` (11)
|
||
- Removed the dead `groups` key from `/api/system-info` and `group_count` from `/api/content`
|
||
- Removed the `'group_id': getattr(player, 'group_id', None)` compat shim from `/api/player-status`
|
||
- Updated 8 documentation files
|
||
|
||
**Also removed (B1, B2) — applied in a second pass:**
|
||
|
||
| ID | Removed | Notes |
|
||
|---|---|---|
|
||
| B1 | `app/blueprints/playlist.py` (310 LOC) + its registration in `app.py` | Whole legacy blueprint; its only real route redirected to `content.manage_playlist_content` |
|
||
| B2 | 3 routes in `players.py`: `reorder_content`, `reorder_playlist`, `remove_from_playlist` (~103 LOC) | Two queried nonexistent columns (`Content.player_id`, `Content.position`, `Player.playlist_version`) → guaranteed 500s |
|
||
|
||
**Extra cleanup triggered by B1:**
|
||
- Deleted `content_list.html` (202 lines) — its only remaining reference was `url_for('playlist.manage_playlist')`. It was **already orphaned** (no Python file rendered it) after `content_old.py` was deleted, so it has been removed for real this time.
|
||
- Deleted `players/player_page.html` (227 lines) — it was **never rendered** by any view (the `players.player_page` route redirects to `manage_player`), so it was dead UI. Corrects the earlier "false positive" note below.
|
||
|
||
> ⚠️ **Functional note:** `players.regenerate_auth_code` (`POST /players/<id>/regenerate-auth`) is now
|
||
> referenced by **no template** — `player_page.html` was its only caller. The endpoint still works if
|
||
> invoked directly. The equivalent control lives in `manage_player.html` via the quickconnect flow.
|
||
> Restore `player_page.html` from `docs/legacy code/` if you want that button back.
|
||
|
||
**Result (A + B + D combined):**
|
||
```
|
||
42 → 38 Python modules 9,311 → 8,296 LOC
|
||
104 → 82 routes 32 → 28 templates
|
||
7 → 6 blueprints dead modules: 2 → 0, orphan templates: 0
|
||
```
|
||
|
||
**Verified:** all files compile; smoke test passes on a fresh DB (every API endpoint + key UI route returns
|
||
< 500); `db.metadata` no longer registers `group`/`group_content`; app boots against a migrated copy of the
|
||
real `dashboard.db` with all data intact; `app.blueprints` no longer contains `playlist`.
|
||
|
||
> ⚠️ **Still open:** the real `data/instance/dashboard.db` predates the `original_filename` migration.
|
||
> On startup the entrypoint now applies it automatically. To fix locally, run:
|
||
> ```
|
||
> DATABASE_URL=sqlite:///$PWD/data/instance/dashboard.db ./.venv/bin/python migrations/add_original_filename_to_content.py
|
||
> ```
|
||
|
||
---
|
||
|
||
## Section A — Dead modules (zero importers)
|
||
|
||
| ID | Target | LOC | Evidence | Risk |
|
||
|---|---|---|---|---|
|
||
| **A1** | `app/blueprints/content_old.py` | 500 | `app.py` imports `content.py`; this file's `content_bp` is **never registered**. Superseded "old" content workflow. | **Low** |
|
||
| **A2** | `app/utils/nginx_config_reader.py` | 120 | Never imported anywhere. Stack migrated nginx → Caddy, so the reader is obsolete. | **Low** |
|
||
|
||
```mermaid
|
||
graph LR
|
||
app_py["app.py<br/>register_blueprints()"] --> content["content.py<br/>content_bp ✅ ACTIVE"]
|
||
content_old["content_old.py<br/>content_bp ❌ DEAD"] -.->|never imported| x1[" "]
|
||
nginx["nginx_config_reader.py<br/>❌ DEAD"] -.->|never imported| x2[" "]
|
||
caddy["caddy_manager.py<br/>✅ ACTIVE"] --> app_py
|
||
style content_old fill:#7f1d1d,color:#fff
|
||
style nginx fill:#7f1d1d,color:#fff
|
||
```
|
||
|
||
---
|
||
|
||
## Section B — Legacy duplicate route surface — ✅ **REMOVED**
|
||
|
||
Two blueprints exposed **parallel implementations of the same operations**. Only the `content.*`
|
||
versions were wired to the UI; the legacy ones had no template references.
|
||
|
||
| ID | Target | LOC | Evidence | Risk |
|
||
|---|---|---|---|---|
|
||
| **B1** ✅ | `app/blueprints/playlist.py` (whole file + registration) | 310 | Entire file was legacy per-player playlist. Its `manage_playlist` route did nothing but **redirect** to the modern `content.manage_playlist_content`. The other 6 routes had **no template reference**. | **Low–Med** |
|
||
| **B2** ✅ | 3 routes in `players.py`: `reorder_content`, `reorder_playlist`, `remove_from_playlist` | ~103 | Superseded by `content.*`. Two queried nonexistent columns (`Content.player_id`, `.position`, `Player.playlist_version`) → guaranteed HTTP 500 if called. | **Low** |
|
||
|
||
**Duplicate operation matrix**
|
||
|
||
| Operation | Modern (LIVE) | Legacy (DEAD) |
|
||
|---|---|---|
|
||
| Add content | `content.add_content_to_playlist` | `playlist.add_to_playlist` |
|
||
| Remove content | `content.remove_content_from_playlist` | `playlist.remove_from_playlist`, `players.remove_from_playlist` ⚠️broken |
|
||
| Reorder | `content.reorder_playlist_content` | `playlist.reorder_playlist`, `players.reorder_content` ⚠️broken, `players.reorder_playlist` ⚠️broken |
|
||
| Set duration | `content.update_playlist_content_duration` | `playlist.update_duration` |
|
||
| Mute audio | `content.update_playlist_content_muted` | `playlist.update_muted` |
|
||
| Toggle edit | `content.update_playlist_content_edit_enabled` | — |
|
||
| Clear | — | `playlist.clear_playlist` |
|
||
|
||
---
|
||
|
||
## Section C — Broken code: references to columns that do not exist
|
||
|
||
Confirmed against the live DB schema. These raised `AttributeError`/`OperationalError` at runtime.
|
||
**All resolved by deleting A1 + B2.**
|
||
|
||
| ID | Location | Broken reference | Reachable? |
|
||
|---|---|---|---|
|
||
| **C1** ✅ | `players.py:768,775` (`remove_from_playlist`) | `player.playlist_version` | Via route only (no UI link) |
|
||
| **C2** ✅ | `players.py:~700` (`reorder_playlist`) | `Content.player_id`, `Content.position` | Via route only (no UI link) |
|
||
| **C3** | `content_old.py:189-190` | `player.playlist_version` | No (dead module) |
|
||
| **C4** | `content_old.py:50,57` | `content.player_id`, `player.group` | No (dead module) |
|
||
|
||
> ✅ **Resolved by deleting A1 + B2.** If you keep those files, they must be rewritten.
|
||
|
||
**Not a bug (verified by hand):** `Content._playlist_duration` /
|
||
`._playlist_position` / `._playlist_muted` in `api.py` are set **dynamically** by
|
||
`Playlist.get_content_ordered()`. These are intentional and work correctly — the
|
||
static analyzer flags them because it only sees model class attributes.
|
||
|
||
---
|
||
|
||
## Section D — Legacy groups subsystem
|
||
|
||
Groups are fully deprecated (0 rows, `/api/groups` already commented out), but the code lingers.
|
||
|
||
| ID | Target | LOC | Evidence | Risk |
|
||
|---|---|---|---|---|
|
||
| **D1** | 5 group functions in `app/utils/group_player_management.py`: `get_group_statistics`, `assign_player_to_group`, `bulk_assign_players_to_group`, `get_online_players_count`, `get_players_by_status` + their `__init__.py` exports | ~130 | **Zero callers outside the module.** All three group functions reference `player.group_id`, **which does not exist** → broken. | **Low** |
|
||
| **D2** | `app/models/group.py` (Group model + `group_content` table) | 71 | Retained only because `Content.groups` FK relationship and `Content.group_count` reference it. Requires touching the Content model. | **Medium** |
|
||
|
||
> ⚠️ **Keep:** `get_player_status_info()` (top of the same file) is **live** — used at
|
||
> `players.py:28` and `players.py:432`. Only the group functions should go.
|
||
|
||
---
|
||
|
||
## Section E — Legacy redirect stubs
|
||
|
||
Thin compatibility shims that only redirect to the modern UI. They're harmless but keep dead
|
||
URL surface alive.
|
||
|
||
| ID | Target | Evidence |
|
||
|---|---|---|
|
||
| **E1** | `players.player_page` (`/players/<id>`) | Body is a single `redirect(url_for('players.manage_player'))`. Still `url_for`-referenced by 2 templates, so keeping it is fine. |
|
||
| **E2** | `playlist.manage_playlist` (`/playlist/<id>`) | Part of B1 — already covered by deleting that file. |
|
||
|
||
---
|
||
|
||
## Section F — Orphan templates / assets
|
||
|
||
| ID | Target | LOC | Evidence |
|
||
|---|---|---|---|
|
||
| — | (none) | — | After the B1/B2 pass the codebase has **0 orphan templates**. |
|
||
|
||
---
|
||
|
||
## Recommended batches — ✅ **ALL APPLIED**
|
||
|
||
| Batch | Contents | Total removed | Status |
|
||
|---|---|---|---|
|
||
| **Batch 1 — Safe clean** | **A1, A2** | ~620 LOC | ✅ done |
|
||
| **Batch 2 — Legacy playlist** | **B1, B2** | ~413 LOC | ✅ done |
|
||
| **Batch 3 — Groups** | **D1** | ~130 LOC | ✅ done |
|
||
| **Batch 4 — Group model** | **D2** | ~71 LOC | ✅ done |
|
||
|
||
**Every batch was followed by:** compile-all + app-factory boot + route smoke test, so hidden
|
||
dependencies were caught before moving on.
|
||
|
||
---
|
||
|
||
## How I verified this (so you can trust it)
|
||
|
||
| Check | Method |
|
||
|---|---|
|
||
| Module reachability | AST import extraction + `register_blueprint` cross-reference |
|
||
| Route usage | `url_for('endpoint')` **and** literal path matching against all templates/JS |
|
||
| Broken columns | Compared every `var.attr` access against live SQLite `PRAGMA table_info` |
|
||
| Dynamic attributes | Manually inspected `get_content_ordered()` to rule out false positives |
|
||
| Duplicate bodies | `ast.dump` body hashing across all functions (result: 0 exact duplicates) |
|
||
|
||
**Reproduce anytime:**
|
||
```
|
||
./.venv/bin/python docs/tools/sanitize_report.py # dead code + broken refs
|
||
./.venv/bin/python docs/tools/sanitize_audit.py # full function inventory
|
||
./.venv/bin/python docs/tools/sanitize_templates.py # orphan templates
|
||
./.venv/bin/python docs/tools/smoke_test.py # post-change smoke test
|
||
```
|
||
|
||
---
|
||
|
||
## ⚠️ Rollback / hygiene notes
|
||
|
||
1. **`docs/legacy code/` is excluded from the Docker image** (via `legacy code/` and
|
||
`**/legacy code/` in `.dockerignore`) so it never
|
||
ships to production or bloats the build.
|
||
2. It is **not** git-ignored, so it will appear in `git status`. Decide:
|
||
- commit it as a recovery point, or
|
||
- add `legacy code/` to `.gitignore` if you'd rather rely on git history.
|
||
3. Deleting files listed here is **not** recoverable from git unless committed first — the
|
||
`docs/legacy code/` copy is your safety net.
|