Fix #1307: reset config.theme to default + document composer install on v2.0 upgrade

README.md

@@ -47,6 +47,12 @@ The [quickstart](https://sbpp.github.io/docs/quickstart/) guide gives you a deta
 The master branch doesn't include the required dependencies or compiled plugins you need to run SourceBans++.
 Here is a quick summary of getting the master branch code up and running.
 
+> **Upgrading from 1.x?** v2.0.0 introduces new PHP dependencies and
+> resets `config.theme` to `default`. Read [`UPGRADING.md`](UPGRADING.md)
+> **before** you `git pull` or unzip a release tarball over an existing
+> install — the `composer install` step is required for git-based
+> upgrades and is not optional.
+
 ### Installing webpanel dependencies
 - Follow the [quickstart](https://sbpp.github.io/docs/quickstart/) guide and upload the webpanel files to your web server
 - Install [composer](https://getcomposer.org/) - [Installation Guide](https://getcomposer.org/doc/00-intro.md#installation-linux-unix-macos)

UPGRADING.md

@@ -8,6 +8,83 @@ land on the [`CHANGELOG.md`](CHANGELOG.md) instead.
 Future entries will land here as the project ships major upgrades; this
 section currently covers the v2.0.0 upgrade path.
 
+## PHP dependencies (#1307, v2.0.0)
+
+**v2.0.0 introduces new PHP dependencies and bumps the major versions
+of others.** A v1.x install with a `vendor/` directory left over from a
+prior `composer install` will pass `web/init.php`'s autoload check but
+fatal at runtime the first time v2.0 code references a class the old
+`vendor/` doesn't ship — usually mid-way through a page render, with
+the actual `Class "…" not found` line buried in your PHP error log.
+
+Run `composer install` from the panel root **before** you visit
+`web/updater/index.php`:
+
+```sh
+cd /path/to/sourcebans/web
+composer install --no-dev --optimize-autoloader
+```
+
+The new / bumped dependencies in v2.0.0:
+
+- `symfony/mailer ^7.2` — replaced the v1.x `PHPMailer` dependency.
+- `league/commonmark ^2.5` — added at #1113 to safely render
+  admin-authored Markdown for the dashboard intro text.
+- `lcobucci/jwt ^5.0` — major-bumped from v1.x's `^4`.
+- `smarty/smarty ^v5.4` — major-bumped from v1.x's `^v3`.
+- `php >= 8.5` — raised at #1289 from v1.x's `>=7.4`.
+
+### Why the panel doesn't catch this for you
+
+`web/init.php` only checks that `vendor/autoload.php` *exists*; it
+can't tell whether the autoloader's contents match what v2.0 expects
+without paying an eager class-resolve cost on every request. Release
+**tarballs** ship a fresh `web/includes/vendor/` so tarball upgrades
+work out of the box. **Git-based upgrades** (`git pull`, `git
+checkout`, drop a checkout over an existing tree) inherit the old
+`vendor/` and need the explicit `composer install` above.
+
+A panel that fatals after you ran the updater isn't permanently
+broken — `composer install` from the panel root will get you back to
+a working state. But the panel renders 500s in the meantime, so it's
+worth running the install step **first**.
+
+## Theme compatibility (#1307, v2.0.0)
+
+**v2.0.0 ships a complete chrome rewrite (#1123 / #1207 / #1259 /
+#1275)** — new typed View DTOs (`Sbpp\View\*`), new admin sidebar
+partial (`core/admin_sidebar.tpl`), every template signature changed,
+MooTools / `xajax` / `sb-callback.php` removed, the `openTab()` JS
+helper deleted. A fork theme inherited from a v1.x install does not
+contain the templates v2.0 expects to render — best case the operator
+gets `Smarty: Unable to load template …` fatals, worst case Smarty
+falls through to a half-rendered page where every template variable
+is undefined.
+
+To keep the panel actually loadable after the upgrade, the v2.0
+updater (migration `808.php`) **resets `config.theme` to `default`**.
+This happens automatically when you run `web/updater/index.php`. No
+operator action is required.
+
+If you maintain a fork theme:
+
+1. The panel will be on `default` immediately after the upgrade.
+2. Port your fork against the v2.0 default theme as your reference
+   (diff `web/themes/default/` between the v1.x and v2.0 trees to see
+   the surface area). The new conventions live in
+   [`AGENTS.md`](AGENTS.md) — most relevant: typed View DTOs, the
+   `?section=…` admin sub-route pattern, the `core/admin_sidebar.tpl`
+   partial, and the empty-state shapes in `web/themes/default/css/theme.css`.
+3. Once your fork ships v2.0-shaped templates, re-select it from
+   **Admin → Settings → Themes**. The DB-side reset is a one-shot at
+   upgrade time; nothing in the panel will switch you back to
+   `default` again.
+
+The pre-existing fork theme directory under `web/themes/<fork>/` is
+left in place — only the `:prefix_settings.config.theme` row is
+rewritten. So your work isn't lost; the panel just stops trying to
+render against templates that don't exist.
+
 ## Telemetry (#1126, v2.0.0)
 
 **SourceBans++ 2.0.0 ships with default-on anonymous telemetry.**

web/tests/integration/UpgradeThemeResetTest.php

@@ -0,0 +1,136 @@
+<?php
+
+namespace Sbpp\Tests\Integration;
+
+use Sbpp\Tests\ApiTestCase;
+use Sbpp\Tests\Fixture;
+
+/**
+ * Issue #1307: v2.0.0 ships a complete chrome rewrite (#1123 / #1207
+ * / #1259 / #1275). Fork themes inherited from v1.x do not contain
+ * the v2.0 templates and fatal on the first post-upgrade page load.
+ *
+ * `web/updater/data/808.php` resets `:prefix_settings.config.theme`
+ * to `'default'` so the panel actually loads after the upgrade.
+ *
+ * Four properties exercised here:
+ *   1. A fork theme value is rewritten to 'default' on first run.
+ *   2. An install already on 'default' is left untouched (the WHERE
+ *      clause matches no rows).
+ *   3. Re-running the migration immediately after the first pass is
+ *      a no-op (the WHERE clause excludes the now-default row).
+ *   4. The default value the migration writes matches what
+ *      `data.sql` seeds for fresh installs (so fresh and upgraded
+ *      installs converge).
+ */
+final class UpgradeThemeResetTest extends ApiTestCase
+{
+    private function setSetting(string $key, string $value): void
+    {
+        $pdo = Fixture::rawPdo();
+        $stmt = $pdo->prepare(sprintf(
+            'REPLACE INTO `%s_settings` (`setting`, `value`) VALUES (?, ?)',
+            DB_PREFIX
+        ));
+        $stmt->execute([$key, $value]);
+        \Config::init($GLOBALS['PDO']);
+    }
+
+    private function readSetting(string $key): ?string
+    {
+        $pdo = Fixture::rawPdo();
+        $stmt = $pdo->prepare(sprintf(
+            'SELECT value FROM `%s_settings` WHERE `setting` = ?',
+            DB_PREFIX
+        ));
+        $stmt->execute([$key]);
+        $row = $stmt->fetch(\PDO::FETCH_ASSOC);
+        return $row === false ? null : (string) $row['value'];
+    }
+
+    private function runMigration(): bool
+    {
+        // The migration is `require_once`'d inside the Updater instance
+        // scope so `$this->dbs` is in scope. Reproduce the same shape with
+        // an anonymous class. `require` (not require_once) so this test
+        // can run after the production updater path has already loaded
+        // the file.
+        $ctx = new class($GLOBALS['PDO']) {
+            public function __construct(public \Database $dbs) {}
+            public function run(string $path): mixed { return require $path; }
+        };
+        return (bool) $ctx->run(ROOT . 'updater/data/808.php');
+    }
+
+    public function testForkThemeIsResetToDefault(): void
+    {
+        // Mirror a v1.x install that's been pointing at a forked theme
+        // directory (e.g. `tf2c`, `darkred`) — the panel will fatal on
+        // the first post-upgrade render against missing v2.0 templates
+        // unless the migration converges this back to 'default'.
+        $this->setSetting('config.theme', 'tf2c');
+        $this->assertSame('tf2c', $this->readSetting('config.theme'),
+            'Pre-condition: the fork theme should be set before the migration runs.');
+
+        $this->assertTrue($this->runMigration(), 'Migration should report success.');
+
+        $this->assertSame('default', $this->readSetting('config.theme'),
+            'Migration must rewrite a fork theme value back to default so v2.0 chrome renders.');
+    }
+
+    public function testInstallAlreadyOnDefaultIsUntouched(): void
+    {
+        // data.sql seeds 'default'; an install that was already on the
+        // shipped theme should be a no-op for the migration. The WHERE
+        // clause excludes the row, so the underlying UPDATE matches 0
+        // rows on this path.
+        $this->setSetting('config.theme', 'default');
+        $this->assertSame('default', $this->readSetting('config.theme'));
+
+        $this->assertTrue($this->runMigration());
+
+        $this->assertSame('default', $this->readSetting('config.theme'),
+            'Migration must not touch an install already on default.');
+    }
+
+    public function testRerunImmediatelyAfterFirstPassIsNoOp(): void
+    {
+        // After the first run leaves the row at 'default', the WHERE
+        // clause excludes that row and the second run matches zero rows.
+        // This is the idempotency guarantee the migration relies on per
+        // AGENTS.md "Updater migrations" — `Updater` has no rollback,
+        // partial state must be safe to re-run, and the runner itself
+        // skips anything <= config.version on a healthy upgrade.
+        $this->setSetting('config.theme', 'tf2c');
+
+        $this->assertTrue($this->runMigration(), 'First run should succeed.');
+        $this->assertSame('default', $this->readSetting('config.theme'));
+
+        $this->assertTrue($this->runMigration(), 'Re-run should still report success.');
+        $this->assertSame('default', $this->readSetting('config.theme'),
+            'Re-running must be a no-op (the WHERE clause excludes the row once it holds default).');
+    }
+
+    public function testMigrationDefaultMatchesDataSqlSeed(): void
+    {
+        // `data.sql` line 48 seeds `('config.theme', 'default')`. The
+        // migration writes the same string. Lock that parity in: a
+        // future rename of the shipped theme directory would otherwise
+        // diverge fresh installs (data.sql) from upgraded installs
+        // (this migration).
+        $dataSql = (string) file_get_contents(ROOT . 'install/includes/sql/data.sql');
+        $this->assertNotSame('', $dataSql, 'Could not read install/includes/sql/data.sql');
+
+        $this->assertMatchesRegularExpression(
+            "/'config\\.theme',\\s*'default'/",
+            $dataSql,
+            'data.sql seed for config.theme drifted away from "default" — update the migration too.'
+        );
+
+        $this->setSetting('config.theme', 'tf2c');
+        $this->runMigration();
+
+        $this->assertSame('default', $this->readSetting('config.theme'),
+            'Migration default value must match the data.sql seed for fresh installs.');
+    }
+}

web/updater/data/808.php

@@ -0,0 +1,42 @@
+<?php
+
+// Issue #1307: v2.0.0 ships a complete chrome rewrite (#1123 / #1207 /
+// #1259 / #1275) — new typed View DTOs (`Sbpp\View\*`), new admin
+// sidebar partial (`core/admin_sidebar.tpl`), `core/admin_tabs.tpl`
+// reduced to the back-link partial, every template signature changed,
+// MooTools / `xajax` / `sb-callback.php` removed, `openTab()` JS
+// deleted. A fork theme inherited from v1.x literally does not contain
+// the templates v2.0 expects to render — best case the operator gets
+// `Smarty: Unable to load template …` fatals, worst case Smarty falls
+// through to a half-rendered page where every variable is undefined.
+//
+// The updater wizard itself runs against `default` (the `IS_UPDATE`
+// override in `web/init.php` lines 217-219), but that scoping ends
+// when the operator clicks "Return to panel" and the next request
+// reads `:prefix_settings.config.theme` from disk again.
+//
+// Force `config.theme` back to the in-tree shipped theme so the panel
+// actually loads after the upgrade. Operators who maintain a fork can
+// re-select it from Admin → Settings → Themes once they've ported it
+// to the v2.0 templating contract (per #1115's "Theme authors"
+// guidance).
+//
+// Idempotent: the WHERE clause matches no rows on a re-run, and
+// matches no rows on installs that were already on `default` to begin
+// with. The default value here matches the seed in
+// `web/install/includes/sql/data.sql` so fresh installs and upgraded
+// installs converge.
+//
+// `$this` is supplied by Updater::update() which loads this file
+// inside the Updater instance scope; PHPStan can't see that, so the
+// next two calls are suppressed in the same way every sibling
+// migration would be.
+// @phpstan-ignore variable.undefined
+$this->dbs->query(
+    "UPDATE `:prefix_settings` SET `value` = 'default' "
+    . "WHERE `setting` = 'config.theme' AND `value` <> 'default'"
+);
+// @phpstan-ignore variable.undefined
+$this->dbs->execute();
+
+return true;

web/updater/store.json

@@ -45,5 +45,6 @@
     "804": "804.php",
     "805": "805.php",
     "806": "806.php",
-    "807": "807.php"
+    "807": "807.php",
+    "808": "808.php"
 }