Merge pull request #833 from PennyDreadfulMTG/trophies

Better handling of season on eventreport.php
PennyDreadfulMTG · Nov 10, 2024 · 675091b · 675091b
2 parents 9125579 + 8188cbd
commit 675091b
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 22 deletions.
diff --git a/gatherling/Helpers/Request.php b/gatherling/Helpers/Request.php
@@ -18,7 +18,7 @@ public function int(string $key, int|false $default = false): int
 
     public function optionalInt(string $key): ?int
     {
-        return marshal($this->vars[$key] ?? null)->optionalInt();
+        return marshal($this->coalesceNumeric($key))->optionalInt();
     }
 
     public function string(string $key, string|false $default = false): string
@@ -39,7 +39,7 @@ public function float(string $key, float|false $default = false): float
     /** @psalm-suppress PossiblyUnusedMethod */
     public function optionalFloat(string $key): ?float
     {
-        return marshal($this->vars[$key] ?? null)->optionalFloat();
+        return marshal($this->coalesceNumeric($key))->optionalFloat();
     }
 
     /** @return list<int> */
@@ -71,4 +71,17 @@ public function dictString(string $key): array
     {
         return marshal($this->vars[$key] ?? null)->dictString();
     }
+
+    // Coalesce like ?? does, but additionally if the value is an empty string, return null.
+    // This is how we want to treat something like 'season=' in a querystring.
+    private function coalesceNumeric(string $key): mixed
+    {
+        if (!isset($this->vars[$key])) {
+            return null;
+        }
+        if ($this->vars[$key] === '') {
+            return null;
+        }
+        return $this->vars[$key];
+    }
 }
diff --git a/gatherling/Views/Components/NumDropMenu.php b/gatherling/Views/Components/NumDropMenu.php
@@ -8,12 +8,8 @@
 
 class NumDropMenu extends DropMenu
 {
-    public function __construct(string $field, ?string $title, int $max, int|string|null $def, int $min = 0, ?string $special = null)
+    public function __construct(string $field, ?string $title, int $max, ?int $def, int $min = 0, ?string $special = null)
     {
-        if ($def === '') {
-            $def = -1;
-        }
-
         $options = [];
         if ($special) {
             $options[] = [
@@ -26,7 +22,7 @@ public function __construct(string $field, ?string $title, int $max, int|string|
             $options[] = [
                 'text'       => (string) $n,
                 'value'      => (string) $n,
-                'isSelected' => $n == $def,
+                'isSelected' => $n === $def,
             ];
         }
 

diff --git a/gatherling/Views/Pages/EventForm.php b/gatherling/Views/Pages/EventForm.php
@@ -70,17 +70,17 @@ public function __construct(Event $event, bool $edit)
         if ($event->start != null) {
             $date = $event->start;
             preg_match('/([0-9]+)-([0-9]+)-([0-9]+) ([0-9]+):([0-9]+):.*/', $date, $datearr);
-            $year = $datearr[1];
-            $month = $datearr[2];
-            $day = $datearr[3];
-            $hour = $datearr[4];
-            $minutes = $datearr[5];
+            $year = (int) $datearr[1];
+            $month = (int) $datearr[2];
+            $day = (int) $datearr[3];
+            $hour = (int) $datearr[4];
+            $minutes = (int) $datearr[5];
         } else {
-            $year = date('Y', time());
-            $month = date('n', time());
-            $day = date('j', time());
-            $hour = date('H', time());
-            $minutes = date('i', time());
+            $year = (int) date('Y', time());
+            $month = (int) date('n', time());
+            $day = (int) date('j', time());
+            $hour = (int) date('H', time());
+            $minutes = (int) date('i', time());
         }
 
         $navLinks = [];

diff --git a/gatherling/eventreport.php b/gatherling/eventreport.php
@@ -60,7 +60,7 @@ function eventList(string $format, string $series, ?int $season): array
         $params['series'] = $series;
     }
 
-    if (!empty($season)) {
+    if ($season !== null) {
         $sql .= ' AND e.season = :season';
         $params['season'] = $season;
     }

diff --git a/tests/Helpers/MarshallerTest.php b/tests/Helpers/MarshallerTest.php
@@ -36,6 +36,12 @@ public function testOptionalInt(): void
         marshal(123.45)->optionalInt();
     }
 
+    public function testOptionalIntThrowsOnEmptyString(): void
+    {
+        $this->expectException(MarshalException::class);
+        marshal('')->optionalInt();
+    }
+
     public function testString(): void
     {
         $this->assertEquals('hello', marshal('hello')->string('key'));

diff --git a/tests/Helpers/RequestTest.php b/tests/Helpers/RequestTest.php
@@ -25,6 +25,13 @@ public function testIntMissing(): void
         $request->int('baz');
     }
 
+    public function testIntThrowsOnEmptyString(): void
+    {
+        $request = new Request(['foo' => '']);
+        $this->expectException(MarshalException::class);
+        $request->int('foo');
+    }
+
     public function testOptionalInt(): void
     {
         $request = new Request(['foo' => '123', 'bar' => '123.4']);
@@ -34,6 +41,12 @@ public function testOptionalInt(): void
         $request->optionalInt('bar');
     }
 
+    public function testOptionalIntReturnsNullOnEmptyString(): void
+    {
+        $request = new Request(['foo' => '']);
+        $this->assertNull($request->optionalInt('foo'));
+    }
+
     public function testFloat(): void
     {
         $request = new Request(['foo' => '123.45']);

diff --git a/tests/Views/Components/NumDropMenuTest.php b/tests/Views/Components/NumDropMenuTest.php
@@ -18,13 +18,14 @@ public function testNumDropMenu(): void
         $this->assertCount(12, $html->filter('select')->filter('option'));
         $this->assertEquals('5', $html->filter('select')->filter('option[selected]')->attr('value'));
 
-        $numDropMenu = new NumDropMenu('test', 'Test', 10, '5');
+        $numDropMenu = new NumDropMenu('test', 'Test', 10, null);
         $html = new Crawler($numDropMenu->render());
         $this->assertEquals('test', $html->filter('select')->attr('name'));
         $this->assertCount(12, $html->filter('select')->filter('option'));
-        $this->assertEquals('5', $html->filter('select')->filter('option[selected]')->attr('value'));
+        // Nothing is explicitly marked as selected, not even the default option
+        $this->assertEquals(0, $html->filter('select')->filter('option[selected]')->count());
 
-        $numDropMenu = new NumDropMenu('test', 'Test', 10, null);
+        $numDropMenu = new NumDropMenu('test', 'Test', 10, 0);
         $html = new Crawler($numDropMenu->render());
         $this->assertEquals('test', $html->filter('select')->attr('name'));
         $this->assertCount(12, $html->filter('select')->filter('option'));