From 18d8f9796d331073d92daab01b44dc1faa7df7ec Mon Sep 17 00:00:00 2001 From: Thomas David Baker Date: Sun, 17 Nov 2024 11:07:35 -0800 Subject: [PATCH 1/2] Return "mana symbols" on deck view to working order --- gatherling/Views/Components/SymbolTable.php | 4 ++-- gatherling/templates/partials/symbolTable.mustache | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gatherling/Views/Components/SymbolTable.php b/gatherling/Views/Components/SymbolTable.php index 4a41e0cd4..1ab87bf8c 100644 --- a/gatherling/Views/Components/SymbolTable.php +++ b/gatherling/Views/Components/SymbolTable.php @@ -10,7 +10,7 @@ class SymbolTable extends Component { public int $sum; /** @var array */ - public array $symbols; + public array $colors = []; public function __construct(Deck $deck) { @@ -22,7 +22,7 @@ public function __construct(Deck $deck) if ($num == 0) { continue; } - $this->symbols[] = [ + $this->colors[] = [ 'color' => $color, 'num' => $num, ]; diff --git a/gatherling/templates/partials/symbolTable.mustache b/gatherling/templates/partials/symbolTable.mustache index b1bf3ccd8..f74ab48fb 100644 --- a/gatherling/templates/partials/symbolTable.mustache +++ b/gatherling/templates/partials/symbolTable.mustache @@ -5,7 +5,7 @@ {{#colors}} - {{color}} + {{color}}   {{num}} From 1a45ff568565564ffd9a4168c65ee5f9911085c3 Mon Sep 17 00:00:00 2001 From: Thomas David Baker Date: Tue, 19 Nov 2024 11:46:45 -0800 Subject: [PATCH 2/2] Improve display and handling of "not found" errors Both at the top level and at the database level. It's super awkward that NotFoundInDatabaseException is basically a copy of NotFoundException in a different part of the exception hierarchy but I don't really want the db layer to know about http response codes. --- gatherling/Data/Db.php | 8 ++++++- gatherling/Exceptions/NotFoundException.php | 20 +++++++++++++++--- .../NotFoundInDatabaseException.php | 21 +++++++++++++++++++ gatherling/Helpers/ErrorHandler.php | 7 +++++++ gatherling/Models/Deck.php | 2 +- gatherling/Models/Entry.php | 2 +- gatherling/Models/Player.php | 8 ++----- gatherling/Views/Components/MatchTable.php | 6 +++--- .../Views/Components/Preregistration.php | 21 +++++++++++-------- .../Views/Components/RecentDecksTable.php | 4 ---- gatherling/deck.php | 2 +- gatherling/eventreport.php | 2 +- psalm-baseline.xml | 14 ------------- 13 files changed, 73 insertions(+), 44 deletions(-) create mode 100644 gatherling/Exceptions/NotFoundInDatabaseException.php diff --git a/gatherling/Data/Db.php b/gatherling/Data/Db.php index 905e4db03..00e16b6ad 100644 --- a/gatherling/Data/Db.php +++ b/gatherling/Data/Db.php @@ -7,6 +7,7 @@ use Gatherling\Exceptions\ConfigurationException; use Gatherling\Exceptions\DatabaseException; use Gatherling\Exceptions\MarshalException; +use Gatherling\Exceptions\NotFoundInDatabaseException; use PDOException; use PDOStatement; use PDO; @@ -180,7 +181,12 @@ public function select(string $sql, string $class, array $params = []): array public function selectOnly(string $sql, string $class, array $params = []): Dto { $result = $this->select($sql, $class, $params); - if (count($result) !== 1) { + if (count($result) === 0) { + $type = str_replace('Dto', '', basename(str_replace('\\', '/', $class))); + $ids = array_values(array_filter($params, fn($v) => is_int($v) || is_string($v))); + throw new NotFoundInDatabaseException("Expected 1 row, got 0 for query: $sql with params " . json_encode($params), 0, null, $type, $ids); + } + if (count($result) > 1) { throw new DatabaseException('Expected 1 row, got ' . count($result) . " for query: $sql with params " . json_encode($params)); } return $result[0]; diff --git a/gatherling/Exceptions/NotFoundException.php b/gatherling/Exceptions/NotFoundException.php index 14f786256..4f90e490b 100644 --- a/gatherling/Exceptions/NotFoundException.php +++ b/gatherling/Exceptions/NotFoundException.php @@ -8,13 +8,27 @@ class NotFoundException extends BadRequestException { - public function __construct(string $message = "", int $code = 0, ?Throwable $previous = null) - { + /** @var list */ + public array $ids; + + /** @param list $ids */ + public function __construct( + string $message, + int $code, + ?Throwable $previous, + public string $type, + array $ids + ) { + $this->ids = array_values(array_filter($ids, fn($id) => $id !== null)); parent::__construct($message, $code, $previous, 404); } public function getUserMessage(): string { - return 'The requested resource was not found'; + $msg = 'The requested ' . strtolower($this->type); + if ($this->ids !== []) { + $msg .= " (" . implode(', ', $this->ids) . ")"; + } + return $msg . " was not found"; } } diff --git a/gatherling/Exceptions/NotFoundInDatabaseException.php b/gatherling/Exceptions/NotFoundInDatabaseException.php new file mode 100644 index 000000000..6d874c9dc --- /dev/null +++ b/gatherling/Exceptions/NotFoundInDatabaseException.php @@ -0,0 +1,21 @@ + $ids */ + public function __construct( + string $message, + int $code, + ?Throwable $previous, + public string $type, + public array $ids + ) { + parent::__construct($message, $code, $previous); + } +} diff --git a/gatherling/Helpers/ErrorHandler.php b/gatherling/Helpers/ErrorHandler.php index 37c2af136..f7afe659d 100644 --- a/gatherling/Helpers/ErrorHandler.php +++ b/gatherling/Helpers/ErrorHandler.php @@ -6,6 +6,8 @@ use Gatherling\Exceptions\BadRequestException; use Gatherling\Exceptions\GatherlingException; +use Gatherling\Exceptions\NotFoundException; +use Gatherling\Exceptions\NotFoundInDatabaseException; use Gatherling\Views\Pages\Error; use Throwable; @@ -15,6 +17,11 @@ public function handle(Throwable $e): never { $requestId = Request::getRequestId(); + if ($e instanceof NotFoundInDatabaseException) { + // BAKERT this is weak but it's a start + $e = new NotFoundException($e->getMessage(), $e->getCode(), $e, $e->type, $e->ids); + } + $message = (string)$e; if ($e instanceof BadRequestException) { diff --git a/gatherling/Models/Deck.php b/gatherling/Models/Deck.php index ae0adcc70..efd3daf6e 100644 --- a/gatherling/Models/Deck.php +++ b/gatherling/Models/Deck.php @@ -424,7 +424,7 @@ public function save(): void $affectedRows = db()->modify($sql, $params); if ($affectedRows != 1) { db()->rollback($transactionName); - throw new NotFoundException('Entry for ' . $this->playername . ' in ' . $this->eventname . ' not found'); + throw new NotFoundException('Entry for ' . $this->playername . ' in ' . $this->eventname . ' not found', 0, null, 'Entry', [$this->playername, $this->eventname]); } } else { $sql = 'UPDATE decks SET archetype = :archetype, name = :name, format = :format, tribe = :tribe, deck_colors = :deck_colors, notes = :notes WHERE id = :id'; diff --git a/gatherling/Models/Entry.php b/gatherling/Models/Entry.php index 80e0f833f..b5e761f46 100644 --- a/gatherling/Models/Entry.php +++ b/gatherling/Models/Entry.php @@ -29,7 +29,7 @@ public function __construct(int $event_id, string $playername) $params = ['event_id' => $event_id, 'player' => $playername]; $entry = db()->selectOnlyOrNull($sql, EntryDto::class, $params); if ($entry == null) { - throw new NotFoundException('Entry for ' . $playername . ' in ' . $event_id . ' not found'); + throw new NotFoundException('Entry for ' . $playername . ' in ' . $event_id . ' not found', 0, null, 'Entry', [$playername, $event_id]); } $this->medal = $entry->medal; $this->drop_round = $entry->drop_round; diff --git a/gatherling/Models/Player.php b/gatherling/Models/Player.php index 3634b7d33..3b3c9f674 100644 --- a/gatherling/Models/Player.php +++ b/gatherling/Models/Player.php @@ -53,11 +53,7 @@ public function __construct(string $name) FROM players WHERE name = :name'; $params = ['name' => $name]; - try { - $result = db()->selectOnly($sql, PlayerDto::class, $params); - } catch (DatabaseException $e) { - throw new NotFoundException("Player $name is not found.", 0, $e); - } + $result = db()->selectOnly($sql, PlayerDto::class, $params); $this->name = $result->name; $this->password = $result->password; $this->host = $result->host; @@ -240,7 +236,7 @@ public static function createByName(string $playername): self $newPlayer = self::findByName($playername); if (!$newPlayer) { - throw new NotFoundException("Failed to retrieve player we just created: {$playername}"); + throw new NotFoundException("Failed to retrieve player we just created: {$playername}", 0, null, 'Player', [$playername]); } return $newPlayer; } diff --git a/gatherling/Views/Components/MatchTable.php b/gatherling/Views/Components/MatchTable.php index f26f85e17..1ab60d6a0 100644 --- a/gatherling/Views/Components/MatchTable.php +++ b/gatherling/Views/Components/MatchTable.php @@ -29,8 +29,8 @@ public function __construct(Player $player, string $selectedFormat, string $sele } $opp = $match->otherPlayer($player->name); - if (!$opp) { - throw new NotFoundException("Opponent not found for match {$match->id}"); + if ($opp === null) { + throw new NotFoundException("Opponent not found for match {$match->id}", 0, null, 'Opponent', [$player->name]); } $res = 'D'; if ($match->playerWon($player->name)) { @@ -43,7 +43,7 @@ public function __construct(Player $player, string $selectedFormat, string $sele $event = $match->getEvent(); if (!$event->id) { - throw new NotFoundException("Event not found for match {$match->id}"); + throw new NotFoundException("Event not found for match {$match->id}", 0, null, 'Event', [$match->id]); } $oppRating = $opponent->getRating('Composite', $event->start); $oppDeck = $opponent->getDeckEvent($event->id); diff --git a/gatherling/Views/Components/Preregistration.php b/gatherling/Views/Components/Preregistration.php index d1656ed98..fae7e877e 100644 --- a/gatherling/Views/Components/Preregistration.php +++ b/gatherling/Views/Components/Preregistration.php @@ -8,6 +8,7 @@ use Gatherling\Models\Entry; use Gatherling\Models\Event; use Gatherling\Models\Player; +use InvalidArgumentException; use function Safe\strtotime; @@ -24,7 +25,7 @@ class Preregistration extends Component public function __construct(Player $player) { if (!$player->name) { - throw new NotFoundException("Tried to display preregistration for a player with no name"); + throw new NotFoundException("Tried to display preregistration for a player with no name", 0, null, 'Player', []); } $upcomingEvents = Event::getUpcomingEvents($player->name); @@ -61,13 +62,14 @@ public function __construct(Player $player) } $eventLink = $targetUrl . '.php?event=' . rawurlencode($event->name); $eventName = $event->name; - if (!$event->start || !strtotime($event->start)) { - throw new NotFoundException("Event start time not found for event {$event->name}"); + $eventStart = strtotime($event->start); + if (!$eventStart) { + throw new InvalidArgumentException("Event start time not found for event {$event->name}"); } - $startingSoon = time() >= strtotime($event->start); - $startTime = new Time(strtotime($event->start), $now); + $startingSoon = time() >= $eventStart; + $startTime = new Time($eventStart, $now); if (!$event->id) { - throw new NotFoundException("Event ID not found for event {$event->name}"); + throw new InvalidArgumentException("Event ID not found for event {$event->name}"); } $entry = new Entry($event->id, $player->name); @@ -99,10 +101,11 @@ public function __construct(Player $player) foreach ($availableEvents as $event) { $eventReportLink = 'eventreport.php?event=' . rawurlencode($event->name); $eventName = $event->name; - if (!$event->start || !strtotime($event->start)) { - throw new NotFoundException("Event start time not found for event {$event->name}"); + $eventStart = strtotime($event->start); + if (!$eventStart) { + throw new InvalidArgumentException("Event start time not found for event {$event->name}"); } - $startTime = new Time(strtotime($event->start), time()); + $startTime = new Time($eventStart, time()); $isFull = $event->isFull(); $requiresMtgo = $event->client == 1 && empty($player->mtgo_username); $requiresMtga = $event->client == 2 && empty($player->mtga_username); diff --git a/gatherling/Views/Components/RecentDecksTable.php b/gatherling/Views/Components/RecentDecksTable.php index 7cebd229c..5c9330134 100644 --- a/gatherling/Views/Components/RecentDecksTable.php +++ b/gatherling/Views/Components/RecentDecksTable.php @@ -7,7 +7,6 @@ use Gatherling\Models\Entry; use Gatherling\Models\Event; use Gatherling\Models\Player; -use Gatherling\Exceptions\NotFoundException; use function Gatherling\Helpers\logger; @@ -22,9 +21,6 @@ public function __construct(Player $player) if (is_null($event)) { return; } - if (!$event->id || !$player->name || !$event->name) { - throw new NotFoundException("Seeming invalid event data for ({$event->id}|{$event->name}|{$player->name}"); - } $entry = new Entry($event->id, $player->name); if ($entry->deck) { $decks = $player->getRecentDecks(6); diff --git a/gatherling/deck.php b/gatherling/deck.php index 7767d90fe..f47b1e1db 100644 --- a/gatherling/deck.php +++ b/gatherling/deck.php @@ -96,7 +96,7 @@ function main(): never } elseif ($postMode === 'Update Deck') { $deck = updateDeck($deck, post()->string('archetype'), post()->string('name'), post()->string('notes'), post()->string('contents', ''), post()->string('sideboard', '')); if ($deck->id === null) { - throw new NotFoundException('Trying to update a deck with null id, which is not possible'); + throw new InvalidArgumentException('Trying to update a deck with null id, which is not possible'); } $deck = new Deck($deck->id); // had to do this to get the constructor to run, otherwise errors weren't loading if ($deck->isValid()) { diff --git a/gatherling/eventreport.php b/gatherling/eventreport.php index f61784bdc..d6c723d87 100644 --- a/gatherling/eventreport.php +++ b/gatherling/eventreport.php @@ -16,7 +16,7 @@ function main(): never { $eventName = get()->optionalString('event') ?? get()->optionalString('name'); - if ($eventName && Event::exists($eventName)) { + if ($eventName !== null && Event::exists($eventName)) { $event = new Event($eventName); $notYetStarted = db()->bool('SELECT `start` > NOW() AS okay FROM events WHERE `name` = :name', ['name' => $event->name]); $canPrereg = $event->prereg_allowed && $notYetStarted; diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 48f3662d1..0dc55c635 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1767,7 +1767,6 @@ - @@ -1961,14 +1960,6 @@ - - - - - - symbols]]> - - @@ -2399,11 +2390,6 @@ - - - - - name]]>