Merge pull request #521 from lanedirt/520-bug-when-abandoning-only-re…

…maining-planet-an-error-is-returned-but-moon-is-deleted Fix planet abandon sanity check order and error reporting
lanedirt · Jan 7, 2025 · 639b6ec · 639b6ec
2 parents 5912715 + e0f9fdd
commit 639b6ec
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 7 deletions.
diff --git a/app/Http/Controllers/PlanetAbandonController.php b/app/Http/Controllers/PlanetAbandonController.php
@@ -143,9 +143,22 @@ public function abandon(PlayerService $player): JsonResponse
             ]);
         }
 
-        // Abandon the planet.
-        $planetToDelete->abandonPlanet();
+        try {
+            // Abandon the planet.
+            $planetToDelete->abandonPlanet();
+        } catch (Exception $e) {
+            // Exception occured, return error.
+            return response()->json([
+                'status' => 'error',
+                'errorbox' => [
+                    'type' => 'fadeBox',
+                    'text' => $e->getMessage(),
+                    'failed' => true,
+                ],
+            ]);
+        }
 
+        // Return success message.
         return response()->json([
             'status' => 'error',
             'errorbox' => [

diff --git a/app/Services/PlanetService.php b/app/Services/PlanetService.php
@@ -172,6 +172,11 @@ public function isValidPlanetName(string $name): bool
      */
     public function abandonPlanet(): void
     {
+        // Sanity check: disallow abandoning the last remaining planet of user.
+        if ($this->isPlanet() && $this->player->planets->planetCount() < 2) {
+            throw new RuntimeException('Cannot abandon only remaining planet.');
+        }
+
         // If this is a planet and has a moon, delete the moon first
         if ($this->isPlanet() && $this->hasMoon()) {
             $this->moon()->abandonPlanet();
@@ -187,10 +192,6 @@ public function abandonPlanet(): void
         // Building queues
         BuildingQueue::where('planet_id', $this->planet->id)->delete();
 
-        if ($this->isPlanet() && $this->player->planets->planetCount() < 2) {
-            throw new RuntimeException('Cannot abandon only remaining planet.');
-        }
-
         // Update the player's current planet if it is the planet being abandoned.
         if ($this->getPlayer()->getCurrentPlanetId() === $this->planet->id) {
             $this->getPlayer()->setCurrentPlanetId(0);

diff --git a/tests/Feature/PlanetAbandonTest.php b/tests/Feature/PlanetAbandonTest.php
@@ -60,6 +60,9 @@ public function testFirstPlanetAbandonFail(): void
             '_token' => csrf_token(),
             'password' => 'password',
         ]);
-        $response->assertStatus(500);
+
+        // Assert that response is HTTP 200 but contains error message.
+        $response->assertStatus(200);
+        $this->assertStringContainsString('Cannot abandon only remaining planet', (string)$response->getContent());
     }
 }