From 7aace2a5517f4a717267a5b89238b13cf5063695 Mon Sep 17 00:00:00 2001
From: Pontoporeia <pontoporeia@happyngreen.fr>
Date: Tue, 31 Mar 2026 23:06:45 +0200
Subject: [PATCH] Add refactoring recommendations for
 controller/template/routing separation

---
 REFACTORING_RECOMMENDATIONS.md | 615 +++++++++++++++++++++++++++++++++
 TODO.md                        |   1 +
 2 files changed, 616 insertions(+)
 create mode 100644 REFACTORING_RECOMMENDATIONS.md

diff --git a/REFACTORING_RECOMMENDATIONS.md b/REFACTORING_RECOMMENDATIONS.md
new file mode 100644
index 0000000..9fb73b8
--- /dev/null
+++ b/REFACTORING_RECOMMENDATIONS.md
@@ -0,0 +1,615 @@
+# Posterg: Refactoring Recommendations
+
+Concrete improvements to the separation between templating, routing, and backend logic — staying in PHP, no framework required.
+
+---
+
+## 1. Extract a Micro-Router + Middleware Pipeline
+
+### Problem
+
+Every file repeats the same preamble. The 7 action handlers and 17 page controllers all independently:
+
+```php
+require_once __DIR__ . '/../../config/bootstrap.php';
+require_once __DIR__ . '/../../src/AdminAuth.php';
+AdminAuth::requireLogin();
+if (empty($_SESSION['csrf_token'])) {
+    $_SESSION['csrf_token'] = bin2hex(random_bytes(32));
+}
+require_once __DIR__ . '/../../src/Database.php';
+```
+
+This is ~6-8 identical lines per file × 24 files = ~170 lines of pure duplication. When the CSRF check pattern changes, every action handler must be updated in lockstep.
+
+### Solution
+
+Create `src/App.php` — a thin request dispatcher with middleware hooks:
+
+```php
+// src/App.php
+class App {
+    private static bool $booted = false;
+
+    /** Boot once per request: load Database, ensure CSRF token exists. */
+    public static function boot(): Database {
+        if (!self::$booted) {
+            require_once APP_ROOT . '/src/Database.php';
+            self::$booted = true;
+        }
+        if (empty($_SESSION['csrf_token'])) {
+            $_SESSION['csrf_token'] = bin2hex(random_bytes(32));
+        }
+        return Database::getInstance();
+    }
+
+    /** Gate for admin pages: auth + CSRF token. */
+    public static function adminGuard(): Database {
+        require_once APP_ROOT . '/src/AdminAuth.php';
+        AdminAuth::requireLogin();
+        return self::boot();
+    }
+
+    /** Validate CSRF on POST. Call at the top of every action handler. */
+    public static function verifyCsrf(): void {
+        if ($_SERVER['REQUEST_METHOD'] !== 'POST'
+            || !isset($_POST['csrf_token'], $_SESSION['csrf_token'])
+            || !hash_equals($_SESSION['csrf_token'], $_POST['csrf_token'])) {
+            http_response_code(403);
+            exit('CSRF token invalide.');
+        }
+    }
+
+    /** Regenerate CSRF after a successful mutation. */
+    public static function rotateCsrf(): void {
+        $_SESSION['csrf_token'] = bin2hex(random_bytes(32));
+    }
+
+    /** Flash a message into the session and redirect. */
+    public static function redirect(string $url, ?string $success = null, ?string $error = null): never {
+        if ($success) $_SESSION['success'] = $success;
+        if ($error)   $_SESSION['error']   = $error;
+        header('Location: ' . $url);
+        exit;
+    }
+}
+```
+
+Every admin page becomes:
+
+```php
+<?php
+require_once __DIR__ . '/../../config/bootstrap.php';
+$db = App::adminGuard();
+// ... page-specific logic
+```
+
+Every action handler becomes:
+
+```php
+<?php
+require_once __DIR__ . '/../../../config/bootstrap.php';
+$db = App::adminGuard();
+App::verifyCsrf();
+// ... mutation logic
+App::rotateCsrf();
+App::redirect('/admin/', success: 'Done.');
+```
+
+**Impact**: Eliminates ~170 lines of duplication. Centralises the CSRF lifecycle (generate, verify, rotate) in one place. Any change to the auth or CSRF pattern is a single-file edit.
+
+---
+
+## 2. Separate Controllers from Templates
+
+### Problem
+
+Every page file (e.g. `public/index.php`, `public/search.php`, `public/admin/edit.php`) is a single file that mixes three concerns:
+
+1. **Data fetching** (DB queries, input validation, pagination math)
+2. **View variable preparation** (`$pageTitle`, `$ogTags`, `$extraCss`, `$bodyClass`)
+3. **HTML rendering** (the entire template, inline)
+
+`system.php` is the extreme case: 400+ lines of PHP logic (systemd checks, curl pings, disk stats, log parsing, nginx syntax highlighting) followed by 200+ lines of inline `<style>`, then 200+ lines of HTML, then 30+ lines of inline `<script>`.
+
+`search.php` is another: the répertoire index view and the search results view are two entirely different pages sharing a single file because they share a URL.
+
+### Solution
+
+Introduce a `controllers/` directory. Each controller is a function that does the data work and returns an associative array. The page file becomes a thin bridge.
+
+**Directory structure:**
+
+```
+src/
+  App.php               ← new: middleware/helpers
+  controllers/
+    HomeController.php
+    SearchController.php
+    TfeController.php
+    admin/
+      ThesisListController.php
+      ThesisEditController.php
+      ThesisAddController.php
+      TagController.php
+      PageController.php
+      SystemController.php
+      AccountController.php
+  ...existing files...
+
+templates/
+  public/
+    home.php
+    search-results.php
+    search-index.php
+    tfe.php
+    apropos.php
+    licence.php
+  admin/
+    thesis-list.php
+    thesis-edit.php
+    thesis-add.php
+    tags.php
+    pages-list.php
+    pages-edit.php
+    system.php
+    account.php
+  ...existing shared templates (head.php, header.php, footer.php)...
+```
+
+**Example — `search.php` split:**
+
+```php
+// src/controllers/SearchController.php
+class SearchController {
+    public static function index(Database $db): array {
+        // Collect params, run queries, compute pagination
+        // ...all current logic from the top of search.php...
+        return [
+            'hasSearch'   => $hasSearch,
+            'results'     => $results,
+            'totalItems'  => $totalItems,
+            'totalPages'  => $totalPages,
+            'years'       => $years,
+            'orientations'=> $orientations,
+            'apPrograms'  => $apPrograms,
+            'keywords'    => $keywords,
+            'students'    => $students,
+            'authorMap'   => $authorMap,
+            'page'        => $page,
+            'error'       => $validationError,
+            // Template config
+            'pageTitle'   => 'Répertoire – Posterg',
+            'bodyClass'   => 'search-body',
+            'extraCss'    => ['/assets/css/search.css'],
+            'currentNav'  => 'repertoire',
+        ];
+    }
+}
+```
+
+```php
+// public/search.php (entire file)
+<?php
+require_once __DIR__ . '/../config/bootstrap.php';
+require_once APP_ROOT . '/src/RateLimit.php';
+
+// Rate limiting (stays here — it's a routing concern)
+$rateLimit = new RateLimit(30, 60);
+if (!$rateLimit->check()) { /* ...429 response... */ }
+$rateLimit->sendHeaders();
+
+$db = App::boot();
+require_once APP_ROOT . '/src/controllers/SearchController.php';
+$data = SearchController::index($db);
+extract($data);  // populates $hasSearch, $results, $pageTitle, etc.
+
+include APP_ROOT . '/templates/head.php';
+include APP_ROOT . '/templates/header.php';
+if ($hasSearch) {
+    include APP_ROOT . '/templates/public/search-results.php';
+} else {
+    include APP_ROOT . '/templates/public/search-index.php';
+}
+include APP_ROOT . '/templates/footer.php';
+```
+
+```
+// templates/public/search-results.php
+// Pure HTML + minimal <?= ?> for output. No DB queries. No input processing.
+```
+
+**Impact**: Templates become auditable for XSS — they only do output. Controllers are testable — they return arrays, no output buffering needed. The `extract()` bridge keeps the familiar variable-name convention without changing every template.
+
+---
+
+## 3. Introduce a `render()` Helper to Replace the 5-Line Include Chain
+
+### Problem
+
+Every page ends with the same sequence:
+
+```php
+include APP_ROOT . '/templates/head.php';
+include APP_ROOT . '/templates/header.php';
+// ... main content ...
+include APP_ROOT . '/templates/footer.php';        // or admin/footer.php
+```
+
+The admin variant also requires `$isAdmin = true; $bodyClass = 'admin-body';` to be set before the head include. Forgetting any variable or include breaks the page silently.
+
+### Solution
+
+Add a `render()` function to `App`:
+
+```php
+// In src/App.php
+public static function render(string $template, array $vars = []): void {
+    extract($vars);
+    include APP_ROOT . '/templates/head.php';
+    include APP_ROOT . '/templates/header.php';
+    include APP_ROOT . '/templates/' . $template;
+    // Choose footer based on admin flag
+    if (!empty($isAdmin)) {
+        include APP_ROOT . '/templates/admin/footer.php';
+    } else {
+        include APP_ROOT . '/templates/footer.php';
+    }
+}
+```
+
+Every page becomes:
+
+```php
+// public/licence.php
+<?php
+require_once __DIR__ . '/../config/bootstrap.php';
+$db = App::boot();
+require_once APP_ROOT . '/src/controllers/LicenceController.php';
+App::render('public/licence.php', LicenceController::index($db));
+```
+
+Every admin page becomes:
+
+```php
+// public/admin/tags.php
+<?php
+require_once __DIR__ . '/../../config/bootstrap.php';
+$db = App::adminGuard();
+require_once APP_ROOT . '/src/controllers/admin/TagController.php';
+App::render('admin/tags.php', TagController::index($db));
+```
+
+**Impact**: No more forgotten includes. The head→header→content→footer pipeline is enforced. Admin footer selection is automatic. Template variables are explicit (passed as array keys, not ambient scope).
+
+---
+
+## 4. Consolidate Action Handlers into Controller Methods
+
+### Problem
+
+The `public/admin/actions/` directory contains 7 POST-only files that each:
+
+1. Require bootstrap + auth
+2. Verify CSRF
+3. Extract + validate `$_POST` data
+4. Call `Database` methods
+5. Rotate CSRF
+6. Flash a message
+7. Redirect
+
+Steps 1-2 and 5-7 are identical in every file. The actual business logic (step 4) is usually 5-15 lines.
+
+`actions/publish.php` (95 lines) does exactly one thing: flip `is_published` on 1-N theses. The other 80 lines are auth, CSRF, validation boilerplate.
+
+### Solution
+
+Merge each action into its controller as a `handlePost()` or action-specific static method:
+
+```php
+// src/controllers/admin/ThesisListController.php
+class ThesisListController {
+    public static function index(Database $db): array {
+        // ... current admin/index.php data logic ...
+    }
+
+    public static function publish(Database $db): never {
+        App::verifyCsrf();
+        $action = $_POST['action'] ?? '';
+        $isBulk = !empty($_POST['bulk']);
+        // ... 15 lines of actual logic ...
+        App::rotateCsrf();
+        App::redirect('/admin/', success: "$count TFE(s) publié(s).");
+    }
+
+    public static function toggleMaintenance(Database $db): never {
+        App::verifyCsrf();
+        // ... 6 lines ...
+        App::rotateCsrf();
+        App::redirect('/admin/', success: 'Maintenance toggled.');
+    }
+}
+```
+
+The page file handles dispatch:
+
+```php
+// public/admin/index.php
+<?php
+require_once __DIR__ . '/../../config/bootstrap.php';
+$db = App::adminGuard();
+require_once APP_ROOT . '/src/controllers/admin/ThesisListController.php';
+
+if ($_SERVER['REQUEST_METHOD'] === 'POST') {
+    $action = $_POST['_action'] ?? '';
+    match($action) {
+        'publish'     => ThesisListController::publish($db),
+        'maintenance' => ThesisListController::toggleMaintenance($db),
+        default       => App::redirect('/admin/', error: 'Action inconnue.'),
+    };
+}
+
+App::render('admin/thesis-list.php', ThesisListController::index($db));
+```
+
+**Or** keep the existing `actions/*.php` files but reduce each to 3 lines:
+
+```php
+// public/admin/actions/publish.php (entire file)
+<?php
+require_once __DIR__ . '/../../../config/bootstrap.php';
+$db = App::adminGuard();
+App::verifyCsrf();
+require_once APP_ROOT . '/src/controllers/admin/ThesisListController.php';
+ThesisListController::publish($db);
+```
+
+**Impact**: The `actions/` directory goes from 7 files × 50-100 lines each ≈ 500 lines, to 7 files × 5 lines each ≈ 35 lines. The business logic moves into testable controller methods.
+
+---
+
+## 5. Extract Inline CSS and JS from `system.php`
+
+### Problem
+
+`system.php` contains:
+- 180 lines of `<style>` embedded in the page
+- 40 lines of `<script>` embedded in the page
+- 12 PHP helper functions defined inline (`safeExec`, `systemdStatus`, `localHttpCheck`, `humanBytes`, `statusLabel`, `statusClass`, `readLogTail`, `logLineClass`, `nginxLineClass`)
+
+This makes it the largest file in the project (500+ lines) and impossible to cache the CSS/JS independently.
+
+### Solution
+
+1. Move the `<style>` block → `public/assets/css/system.css`, reference it via `$extraCss`
+2. Move the `<script>` block → `public/assets/js/system.js`, reference it via `$extraJs`
+3. Move the 12 helper functions → `src/controllers/admin/SystemController.php`
+4. Move the data-gathering logic (checks array, PHP info, disk stats, log reading) into `SystemController::index()`
+
+The template becomes pure HTML with `<?= ?>` interpolation.
+
+**Impact**: `system.php` goes from ~500 lines to ~5 lines (boot + controller + render). The CSS/JS becomes cacheable by the browser (nginx `expires 30d` rule already exists for `.css`/`.js`). The helpers become unit-testable.
+
+---
+
+## 6. Introduce Template Partials for Repeated UI Patterns
+
+### Problem
+
+Several HTML patterns are copy-pasted across templates:
+
+**Flash messages** — Identical block in `admin/index.php`, `admin/edit.php`, `admin/tags.php`, `admin/pages.php`, `admin/account.php`:
+```php
+<?php if (isset($_SESSION['error'])): ?>
+<div class="admin-alert admin-alert--error">⚠ <?= htmlspecialchars($_SESSION['error']); unset($_SESSION['error']); ?></div>
+<?php endif; ?>
+<?php if (isset($_SESSION['success'])): ?>
+<div class="admin-alert admin-alert--success">✓ <?= htmlspecialchars($_SESSION['success']); unset($_SESSION['success']); ?></div>
+<?php endif; ?>
+```
+
+**Pagination** — Near-identical block in `index.php` and `search.php` (30 lines each, slightly different URL building).
+
+**Select dropdowns with "selected" logic** — Repeated in `admin/add.php` and `admin/edit.php` for orientation, AP, finality, license, access type.
+
+**Jury fieldset + JS** — Duplicated between `admin/add.php` and `admin/edit.php` (50+ lines of identical HTML + 20 lines of identical JS).
+
+### Solution
+
+Create small partial templates:
+
+```
+templates/
+  partials/
+    flash-messages.php     ← reads $_SESSION['error'] / $_SESSION['success']
+    pagination.php         ← receives $page, $totalPages, $baseUrl
+    admin/
+      select-field.php     ← receives $name, $label, $options, $selected
+      checkbox-list.php    ← receives $name, $label, $options, $checked
+      jury-fieldset.php    ← receives $jury (array), outputs fieldset + JS
+```
+
+**Example — `flash-messages.php`:**
+
+```php
+<?php
+// templates/partials/flash-messages.php
+$_flashError   = $_SESSION['error']   ?? $_SESSION['admin_error']   ?? $_SESSION['edit_error']   ?? null;
+$_flashSuccess = $_SESSION['success'] ?? $_SESSION['admin_success'] ?? $_SESSION['edit_success'] ?? null;
+unset($_SESSION['error'], $_SESSION['success'], $_SESSION['admin_error'],
+      $_SESSION['admin_success'], $_SESSION['edit_error'], $_SESSION['edit_success']);
+?>
+<?php if ($_flashError): ?>
+<div class="admin-alert admin-alert--error">⚠ <?= htmlspecialchars($_flashError) ?></div>
+<?php endif; ?>
+<?php if ($_flashSuccess): ?>
+<div class="admin-alert admin-alert--success">✓ <?= htmlspecialchars($_flashSuccess) ?></div>
+<?php endif; ?>
+```
+
+This also fixes a latent bug: the project uses 6 different session keys for flash messages (`error`, `success`, `admin_error`, `admin_success`, `edit_error`, `edit_success`, `form_error`). Centralising flash handling would unify these into two keys.
+
+**Impact**: Eliminates ~200 lines of duplicated HTML. The jury fieldset (duplicated between add and edit) becomes a single 50-line partial. Form field partials make admin pages shorter and more consistent.
+
+---
+
+## 7. Unify Flash Message Keys
+
+### Problem
+
+The project uses 7 different session keys for flash messages across different pages:
+
+| Key | Used by |
+|-----|---------|
+| `$_SESSION['error']` | `admin/index.php`, `visibility.php`, `account.php` |
+| `$_SESSION['success']` | `admin/index.php`, `visibility.php`, `pages.php`, `account.php` |
+| `$_SESSION['admin_error']` | `tags.php` |
+| `$_SESSION['admin_success']` | `tags.php` |
+| `$_SESSION['edit_error']` | `admin/edit.php` |
+| `$_SESSION['edit_success']` | `admin/edit.php` |
+| `$_SESSION['form_error']` | `admin/add.php` |
+| `$_SESSION['form_data']` | `admin/add.php` (re-population) |
+
+This means flash messages can silently persist across pages if a redirect sends the user somewhere that doesn't read the matching key.
+
+### Solution
+
+Standardise on two keys: `$_SESSION['_flash_error']` and `$_SESSION['_flash_success']`. Consume them in the shared `flash-messages.php` partial. Add `App::flash(string $type, string $message)` helper.
+
+---
+
+## 8. Move OG Tag Construction into Controller Logic
+
+### Problem
+
+Every public page constructs `$ogTags` inline before the template, with ~15 lines of boilerplate. `tfe.php` has 25 lines of OG logic including image resolution (banner → cover → none) and description truncation.
+
+### Solution
+
+Move OG tag logic into each controller's return array. Create a helper:
+
+```php
+// In src/App.php or a dedicated helpers file
+public static function ogTags(array $overrides = []): array {
+    return array_merge([
+        'type'        => 'website',
+        'site_name'   => 'Posterg – ERG',
+        'title'       => 'Posterg',
+        'description' => '',
+        'url'         => '',
+        'image'       => '',
+    ], $overrides);
+}
+```
+
+The TFE controller builds the OG image resolution as part of its data preparation. The template never touches it.
+
+---
+
+## Summary: Proposed File Layout
+
+```
+src/
+  App.php                          ← NEW: boot, adminGuard, verifyCsrf, render, redirect, flash
+  Database.php                     ← unchanged
+  AdminAuth.php                    ← unchanged
+  RateLimit.php                    ← unchanged
+  Parsedown.php                    ← unchanged
+  config.php                       ← unchanged
+  controllers/
+    HomeController.php             ← extracted from public/index.php
+    SearchController.php           ← extracted from public/search.php
+    TfeController.php              ← extracted from public/tfe.php
+    AproposController.php          ← extracted from public/apropos.php
+    LicenceController.php          ← extracted from public/licence.php
+    admin/
+      ThesisListController.php     ← extracted from admin/index.php + actions/publish.php + actions/maintenance.php
+      ThesisEditController.php     ← extracted from admin/edit.php + actions/edit.php
+      ThesisAddController.php      ← extracted from admin/add.php + actions/formulaire.php
+      TagController.php            ← extracted from admin/tags.php + actions/tag.php
+      PageController.php           ← extracted from admin/pages.php + pages-edit.php + actions/page.php
+      SystemController.php         ← extracted from admin/system.php (400 lines of logic)
+      AccountController.php        ← extracted from admin/account.php + actions/account.php
+      ImportController.php         ← extracted from admin/import.php
+
+templates/
+  head.php                         ← unchanged
+  header.php                       ← unchanged
+  footer.php                       ← unchanged
+  search-bar.php                   ← unchanged
+  admin/
+    footer.php                     ← unchanged
+  partials/
+    flash-messages.php             ← NEW
+    pagination.php                 ← NEW
+    admin/
+      jury-fieldset.php            ← NEW (deduplicated from add.php + edit.php)
+      select-field.php             ← NEW
+      checkbox-list.php            ← NEW
+  public/
+    home.php                       ← extracted HTML from index.php
+    search-results.php             ← extracted HTML from search.php (results view)
+    search-index.php               ← extracted HTML from search.php (répertoire view)
+    tfe.php                        ← extracted HTML from tfe.php
+    apropos.php                    ← extracted HTML from apropos.php
+    licence.php                    ← extracted HTML from licence.php
+  admin/
+    thesis-list.php                ← extracted HTML from admin/index.php
+    thesis-edit.php                ← extracted HTML from admin/edit.php
+    thesis-add.php                 ← extracted HTML from admin/add.php
+    tags.php                       ← extracted HTML from admin/tags.php
+    pages-list.php                 ← extracted HTML from admin/pages.php
+    pages-edit.php                 ← extracted HTML from admin/pages-edit.php
+    system.php                     ← extracted HTML from admin/system.php
+    account.php                    ← extracted HTML from admin/account.php
+
+public/
+  index.php                        ← 5-8 lines: boot, controller, render
+  search.php                       ← 8-10 lines: boot, rate limit, controller, render
+  tfe.php                          ← 5-8 lines
+  apropos.php                      ← 5-8 lines
+  licence.php                      ← 5-8 lines
+  media.php                        ← unchanged (already clean)
+  maintenance.php                  ← unchanged
+  live-reload.php                  ← unchanged
+  admin/
+    index.php                      ← 8-10 lines: boot, dispatch POST or render
+    edit.php                       ← 8-10 lines
+    add.php                        ← 8-10 lines
+    tags.php                       ← 6-8 lines
+    pages.php                      ← 5-8 lines
+    pages-edit.php                 ← 5-8 lines
+    system.php                     ← 5-8 lines
+    account.php                    ← 8-10 lines
+    import.php                     ← 5-8 lines
+    login.php                      ← unchanged
+    logout.php                     ← unchanged
+    actions/                       ← can be REMOVED (merged into controllers)
+      OR reduced to 3-5 line stubs
+```
+
+## Estimated Impact
+
+| Metric | Before | After |
+|--------|--------|-------|
+| Lines in `public/` page files | ~4,200 | ~200 |
+| Lines in `actions/` handlers | ~500 | 0 (merged) |
+| Duplicated boilerplate | ~300 lines | ~0 |
+| Duplicated HTML (jury, flash, pagination) | ~200 lines | ~0 |
+| `system.php` | 530 lines | ~8 lines (page) + ~180 (controller) + ~100 (CSS) + ~30 (JS) |
+| Testable controller methods | 0 | ~15 |
+| Flash session key variants | 7 | 2 |
+
+Total net reduction: ~800–1,000 lines eliminated through deduplication, with cleaner separation making the remaining code auditable and testable.
+
+## Execution Order
+
+This can be done incrementally, one page at a time, with zero disruption:
+
+1. **Create `src/App.php`** with `boot()`, `adminGuard()`, `verifyCsrf()`, `rotateCsrf()`, `redirect()`, `render()`, `flash()`
+2. **Create `templates/partials/flash-messages.php`** and adopt it in one admin page
+3. **Extract `SystemController`** — the biggest single-file win (500 → 8 lines)
+4. **Extract `SearchController`** — the most complex public page
+5. **Extract `ThesisEditController`** — merges `edit.php` + `actions/edit.php`, deduplicates jury fieldset
+6. **Do remaining controllers one by one**, smallest first
+7. **Unify flash keys** project-wide as the last step
diff --git a/TODO.md b/TODO.md
index dc0c409..51610c1 100644
--- a/TODO.md
+++ b/TODO.md
@@ -10,3 +10,4 @@
   - `common.css`: `.site-nav` → `header nav`, `.site-nav__logo` → `header nav > a`, etc.
   - `admin.css`: `.admin-nav` → `.admin-body header nav`, logout via `[data-nav-logout]` attribute
 - [x] PHP vs Flask architecture analysis (`ANALYSIS_PHP_VS_FLASK.md`)
+- [x] Refactoring recommendations for controller/template separation (`REFACTORING_RECOMMENDATIONS.md`)