Fix appload() returning a coerced HTTP 200 instead of a 404

ItemController::appload() was declared ': ?string', so its two error
branches that 'return response()->json([...], 404)' had the JsonResponse
coerced through Response::__toString() into a raw HTTP message served as
an HTTP 200 body. Widen the return type to
'\Illuminate\Http\JsonResponse|string|null' so those branches emit
real 404 JSON responses. The method body is unchanged, so the happy path
still returns the same JSON string and the frontend contract is preserved.

Flip the endpoint characterization test to assert the corrected 404.
This commit is contained in:
KodeStar
2026-07-08 20:33:26 +01:00
parent 46e09d172a
commit f1eec81591
2 changed files with 66 additions and 1 deletions
+1 -1
View File
@@ -430,7 +430,7 @@ class ItemController extends Controller
*
* @throws GuzzleException
*/
public function appload(Request $request): ?string
public function appload(Request $request): \Illuminate\Http\JsonResponse|string|null
{
$output = [];
$appid = $request->input('app');
+65
View File
@@ -0,0 +1,65 @@
<?php
namespace Tests\Feature;
use App\Item;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Tests\TestCase;
/**
* End-to-end coverage for the AJAX POST endpoints that the dashboard relies
* on (the routes excluded from CSRF verification: order / appload).
*
* CSRF itself is disabled while running unit tests, so these tests deliberately
* do NOT assert "an un-tokened POST succeeds" (that would be meaningless).
* Instead they exercise the controller + routing end-to-end with realistic
* input and assert the real, observable behaviour, which is what would break
* if the controller or router regressed on a framework upgrade.
*/
class AjaxPostEndpointsTest extends TestCase
{
use RefreshDatabase;
public function test_order_endpoint_persists_the_new_item_order(): void
{
$this->seed();
$first = Item::factory()->create(['order' => 5]);
$second = Item::factory()->create(['order' => 9]);
// POST the ids in reverse: index 0 => $second, index 1 => $first.
$response = $this->post('/order', [
'order' => [$second->id, $first->id],
]);
$response->assertStatus(200);
$this->assertSame(0, (int) $second->fresh()->order);
$this->assertSame(1, (int) $first->fresh()->order);
}
public function test_appload_returns_null_for_the_none_selection(): void
{
$this->seed();
$response = $this->post('/appload', ['app' => 'null']);
$response->assertStatus(200);
$this->assertSame('', $response->getContent());
}
public function test_appload_surfaces_a_not_found_error_for_an_unknown_app(): void
{
$this->seed();
$response = $this->post('/appload', ['app' => 'this-app-does-not-exist']);
// For an unknown app the controller returns a genuine 404 JSON
// response. appload() is declared to return
// JsonResponse|string|null, so the JsonResponse is served as-is
// (correct status + JSON body) rather than being coerced through
// Response::__toString() into a raw HTTP message served as a 200.
$response->assertStatus(404);
$response->assertExactJson(['error' => 'Application not found.']);
}
}