Skip to content

chore(php): backed enums for log/ban/web-permission columns (#1290) - #1297

Merged
rumblefrog merged 2 commits into
mainfrom
chore/1290-enums
May 8, 2026
Merged

rumblefrog merged 2 commits into
mainfrom
chore/1290-enums

Conversation

@rumblefrog

Copy link
Copy Markdown
Member

Summary

Closes part of #1290. Phase D from the umbrella — backed PHP-side enums wrapping the on-disk column types. Schema is unchanged.

  • D.1: LogType (string-backed) wraps :prefix_log.type enum('m','w','e'). Log::add(string $type) signature now takes LogType. Log.php's switch-based search WHERE builder also refactored to a LogSearchType enum + whereClause() method.
  • D.2: BanType (int-backed) wraps :prefix_bans.type tinyint.
  • D.3: BanRemoval (string-backed; departure from the issue body — the on-disk column is varchar(3) NULL not int) wraps :prefix_bans.RemoveType and :prefix_comms.RemoveType. Cases D / U / E.
  • D.4: WebPermission (int-backed) wraps the ADMIN_* integer-bitmask flags from web/configs/permissions/web.json (mirrors init.php's defined constants — both shapes coexist for backward compatibility). 31 cases. Static helpers WebPermission::mask(...$flags) and WebPermission::fromMask(int).

HasAccess() signature now WebPermission|int|string $flags — the legacy HasAccess(ADMIN_OWNER | ADMIN_X) shape and the modern HasAccess(WebPermission::mask(...)) shape both work.

Wire-format unchanged

composer api-contract clean — no Perms.ADMIN_* shape change; the JS contract still emits the same constants (the enum is purely a PHP-side wrapper).

Notable decisions (from the review)

  1. BanRemoval is string-backed, not int — the on-disk column is varchar(3) NULL. Reading from disk: null / '' means the ban is still active; D/U/E are terminal states. Helper docblocks call this out so future readers know the contract.
  2. HasAccess() is NOT variadic — the docblock + AGENTS.md anti-pattern row use the correct WebPermission::mask(...) shape (NOT HasAccess(WebPermission::Owner, WebPermission::AddBan), which would bind the second flag as $aid and crash).
  3. Regression test pinning WebPermission cases to web/configs/permissions/web.json (WebPermissionTest::testWebPermissionEnumMatchesWebJson). Drift now fails the gate.
  4. Unit tests for WebPermission::fromMask covering known-bit round-trip, zero, unknown-bit silent drop, and 2^31 high-bit on 64-bit PHP.
  5. Deliberately legacy call shapes preserved (with rationale documented in the AGENTS.md anti-patterns):
    • HasAccess(ALL_WEB, $aid) — ALL_WEB is a derived rolled-up bitmask, not a flag.
    • HasAccess(SM_RCON . SM_ROOT) — SourceMod char-flag string, not an int bitmask.
    • Runtime-dynamic HasAccess($flag) in AdminTabs, SmartyCustomFunctions, View/Perms, etc. — value is computed at runtime from JSON config.
    • RemoveType = 'E' literals in PruneBans / PruneComms static SQL strings (inline predicates, not bound values).

sbError() now maps to LogType cases

$entry = match ($errno) {
    E_USER_ERROR   => [LogType::Error,   'PHP Error',   'Fatal Error'],
    E_USER_WARNING => [LogType::Warning, 'PHP Warning', 'Error'],
    E_USER_NOTICE  => [LogType::Message, 'PHP Notice',  'Notice'],
    default        => null,
};

Closing invariants

$ rg "Log::add\('[a-z]'" web/        # empty
$ rg "HasAccess\(\s*ADMIN_" web/     # only the docblock reference
$ rg "varchar\(1\)" web/includes AGENTS.md ARCHITECTURE.md  # empty

Test plan

  • ./sbpp.sh phpstan clean (225 files)
  • ./sbpp.sh test 393 tests pass (5 new WebPermissionTest cases)
  • ./sbpp.sh ts-check clean
  • ./sbpp.sh composer api-contract byte-identical
  • PHPStan baseline net -12 lines vs main (2 ignore blocks dropped after PR3's int-cast on mktime hour/minute args)

rumblefrog added 2 commits May 8, 2026 03:55
…ms (#1290)

Phase D from #1290 — backed PHP-side enums wrapping the on-disk column
types. Schema is unchanged.

- D.1: LogType enum (string-backed, letter codes match
       :prefix_log.type varchar(1)). Log::add(string $type) signature
       now takes LogType. Call sites converted across pages and
       handlers. Log.php's switch-based search WHERE builder
       refactored to a LogSearchType enum + whereClause() method.
- D.2: BanType enum (int-backed, wraps :prefix_bans.type tinyint).
       Steam=0, Ip=1. Writers/readers in bans.php / page.banlist.php /
       page.home.php / admin.edit.ban.php migrated.
- D.3: BanRemoval enum (string-backed, wraps the RemoveType varchar(3)
       column on :prefix_bans / :prefix_comms — Deleted='D', Unbanned='U',
       Expired='E'). String-backed because the column is varchar(3) on
       disk; the issue's umbrella prompt described it as int but the
       schema is varchar — the enum job is to mirror the on-disk type,
       so string is correct.
- D.4: WebPermission enum (int-backed, wraps the ADMIN_* bitmask
       constants from init.php). HasAccess() now accepts
       WebPermission|int|string. WebPermission::mask(...) assembles
       multi-flag bitmasks; WebPermission::fromMask(int) decodes one.
       The ADMIN_* defines stay for procedural back-compat. Call sites
       migrated across 36 files (admin.edit.adminperms.php carries the
       largest single-file blast radius at 34 sites).

The on-disk schema is untouched — every enum's value mirrors the
existing column type. PHPStan-dba sees \$enum->value at every bind
site, so column types still match the live MariaDB schema. Wire format
unchanged (api-contract clean).

Plus AGENTS.md (Conventions: new "Backed enums for column-typed
fields" subsection; Anti-patterns: rows for Log::add letter codes,
HasAccess(ADMIN_*|...) bitmasks, RemoveType='U'|'D'|'E' SQL literals,
\$row['type']==0|1 ban-type branches; Where to find what: row for
the new enum files) and ARCHITECTURE.md (Database schema → new
"Column-typed PHP enums" subsection cataloguing all five enums and
the wrap-around-existing-columns contract).
- M1: WebPermission class docblock + AGENTS.md anti-pattern row
      updated to use the correct WebPermission::mask(...) shape
      (HasAccess is not variadic — second arg is $aid).
- M2: regression test pinning WebPermission enum cases to
      web/configs/permissions/web.json. Drift now fails the gate.
- M3: unit tests for WebPermission::fromMask covering known-bit
      round-trips, zero, unknown-bit silent drop, and 2^31 high-bit
      on 64-bit PHP.
- m1: docblock truth — cases are ordered numerically, not
      web.json-grouped.
- m2: replace "varchar(1)" with "enum('m','w','e')" in LogType,
      Log, AGENTS.md, ARCHITECTURE.md.
- m3: resolveDateRange docblock now documents the time-component
      fallback.
- m4: Log::getCount drops the unused $search parameter.

PHPStan + PHPUnit + api-contract still clean.
@rumblefrog
rumblefrog added this pull request to the merge queue May 8, 2026
Merged via the queue into main with commit c3beff2 May 8, 2026
5 checks passed
@rumblefrog
rumblefrog deleted the chore/1290-enums branch May 8, 2026 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant