From 56844904c99e53de60cf90de64f99025ee4d6be9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristian=20St=C3=B6ckel?= Date: Sun, 10 Mar 2024 21:41:49 +0100 Subject: [PATCH] :recycle: migrate home station endpoint to use TRWL-ID see #2411 --- API_CHANGELOG.md | 9 ++- .../API/v1/TransportController.php | 64 +++++++++++----- .../Backend/Transport/HomeController.php | 3 +- routes/api.php | 9 ++- storage/api-docs/api-docs.json | 11 ++- tests/Feature/APIv1/TransportTest.php | 73 ++++++++++--------- 6 files changed, 105 insertions(+), 64 deletions(-) diff --git a/API_CHANGELOG.md b/API_CHANGELOG.md index 08a72b9d2..d8634d5d1 100644 --- a/API_CHANGELOG.md +++ b/API_CHANGELOG.md @@ -4,8 +4,15 @@ In this we try to keep track of changes to the API. Primarily this should document changes that are not backwards compatible or belongs to already documented endpoints. This is to help you keep track of the changes and to help you update your code accordingly. +## 2024-03-10 + +Replaced `PUT /trains/station/{name}/home` with `PUT /station/{id}/home`. +The old endpoint is marked as deprecated and will be removed after 2024-06. + +Please note, that the ID is the Träwelling internal ID and not the IBNR! + ## 2024-03-01 -> **warning** +> [!WARNING] > Possibly breaking change: The implementation of next/prev links on user/{username}/statuses endpoint has been changed to adhere to the documentation. ## 2024-01-21 diff --git a/app/Http/Controllers/API/v1/TransportController.php b/app/Http/Controllers/API/v1/TransportController.php index 26ee371c6..0b68b6adf 100644 --- a/app/Http/Controllers/API/v1/TransportController.php +++ b/app/Http/Controllers/API/v1/TransportController.php @@ -21,6 +21,7 @@ use App\Models\Event; use App\Models\Station; use Carbon\Carbon; +use Exception; use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Http\JsonResponse; use Illuminate\Http\Request; @@ -420,18 +421,45 @@ public function create(Request $request): JsonResponse { } } + /** + * @param string $stationName + * + * @return JsonResponse + * @see All slashes (as well as encoded to %2F) in $name need to be replaced, preferrably by a space (%20) + * @deprecated Replaced by setHome (with "ID" instead of StationName and without "trains" in the path) + */ + public function setHomeLegacy(string $stationName): JsonResponse { //ToDo: Remove this endpoint after 2024-06 (replaced by id) + try { + $station = HafasController::getStations(query: $stationName, results: 1)->first(); + if ($station === null) { + return $this->sendError("Your query matches no station"); + } + + $station = HomeController::setHome(user: auth()->user(), station: $station); + + return $this->sendResponse( + data: new StationResource($station), + ); + } catch (HafasException) { + return $this->sendError("There has been an error with our data provider", 502); + } catch (ModelNotFoundException) { + return $this->sendError("Your query matches no station"); + } + } + + /** * @OA\Put( - * path="/trains/station/{name}/home", + * path="/station/{id}/home", * operationId="setHomeStation", * tags={"Checkin"}, * summary="Set a station as home station", * @OA\Parameter( - * name="name", + * name="id", * in="path", - * description="Name of the station", + * description="Träwelling-ID of the station", * required=true, - * example="Karlsruhe Hbf", + * example=1234, * ), * @OA\Response( * response=200, @@ -444,33 +472,29 @@ public function create(Request $request): JsonResponse { * @OA\Response(response=400, description="Bad request"), * @OA\Response(response=401, description="Unauthorized"), * @OA\Response(response=404, description="Station not found"), - * @OA\Response(response=502, description="Error with our data provider"), - * security={ - * {"passport": {"create-statuses"}}, {"token": {}} - * - * } + * @OA\Response(response=500, description="Unknown error"), + * security={{"passport": {"create-statuses"}}, {"token": {}}} * ) - * @param string $stationName + * @param int $stationId * * @return JsonResponse - * @see All slashes (as well as encoded to %2F) in $name need to be replaced, preferrably by a space (%20) */ - public function setHome(string $stationName): JsonResponse { + public function setHome(int $stationId): JsonResponse { try { - $station = HafasController::getStations(query: $stationName, results: 1)->first(); - if ($station === null) { - return $this->sendError("Your query matches no station"); - } + $station = Station::findOrFail($stationId); - $station = HomeController::setHome(user: auth()->user(), station: $station); + auth()->user()?->update([ + 'home_id' => $station->id + ]); return $this->sendResponse( data: new StationResource($station), ); - } catch (HafasException) { - return $this->sendError("There has been an error with our data provider", 502); } catch (ModelNotFoundException) { - return $this->sendError("Your query matches no station"); + return $this->sendError('The station could not be found'); + } catch (Exception $exception) { + report($exception); + return $this->sendError('Unknown error', 500); } } diff --git a/app/Http/Controllers/Backend/Transport/HomeController.php b/app/Http/Controllers/Backend/Transport/HomeController.php index a8ee56fcf..5b0b51a4a 100644 --- a/app/Http/Controllers/Backend/Transport/HomeController.php +++ b/app/Http/Controllers/Backend/Transport/HomeController.php @@ -13,7 +13,8 @@ abstract class HomeController extends Controller * @param Station $station * * @return Station - * @api v1 + * @api v1 + * @deprecated just use $user->update(...) directly...? */ public static function setHome(User $user, Station $station): Station { $user->update([ diff --git a/routes/api.php b/routes/api.php index 7cfcee586..b5f379336 100644 --- a/routes/api.php +++ b/routes/api.php @@ -87,12 +87,17 @@ Route::post('checkin', [TransportController::class, 'create']); Route::group(['prefix' => 'station'], static function() { Route::get('{name}/departures', [TransportController::class, 'departures']); - Route::put('{name}/home', [TransportController::class, 'setHome']); + Route::put('{name}/home', [TransportController::class, 'setHomeLegacy']); //ToDo: Remove this endpoint after 2024-06 (replaced by id) Route::get('nearby', [TransportController::class, 'getNextStationByCoordinates']); Route::get('autocomplete/{query}', [TransportController::class, 'getTrainStationAutocomplete']); Route::get('history', [TransportController::class, 'getTrainStationHistory']); }); }); + + Route::prefix('station')->middleware(['scope:write-statuses'])->group(static function() { + Route::put('{id}/home', [TransportController::class, 'setHome'])->whereNumber('id'); + }); + Route::group(['prefix' => 'statistics', 'middleware' => 'scope:read-statistics'], static function() { Route::get('/', [StatisticsController::class, 'getPersonalStatistics']); Route::get('/global', [StatisticsController::class, 'getGlobalStatistics']); @@ -165,7 +170,7 @@ Route::delete('/{webhookId}', [WebhookController::class, 'deleteWebhook']); }); - Route::apiResource('station', StationController::class); // currently admin/backend only + Route::apiResource('station', StationController::class); // currently admin/backend only Route::put('station/{oldStationId}/merge/{newStationId}', [StationController::class, 'merge']); // currently admin/backend only }); diff --git a/storage/api-docs/api-docs.json b/storage/api-docs/api-docs.json index 4a9a7763b..45266e2a6 100644 --- a/storage/api-docs/api-docs.json +++ b/storage/api-docs/api-docs.json @@ -3388,7 +3388,7 @@ ] } }, - "/trains/station/{name}/home": { + "/station/{id}/home": { "put": { "tags": [ "Checkin" @@ -3397,11 +3397,11 @@ "operationId": "setHomeStation", "parameters": [ { - "name": "name", + "name": "id", "in": "path", - "description": "Name of the station", + "description": "Träwelling-ID of the station", "required": true, - "example": "Karlsruhe Hbf" + "example": 1234 } ], "responses": { @@ -3429,6 +3429,9 @@ "404": { "description": "Station not found" }, + "500": { + "description": "Unknown error" + }, "502": { "description": "Error with our data provider" } diff --git a/tests/Feature/APIv1/TransportTest.php b/tests/Feature/APIv1/TransportTest.php index 6c3af2ceb..fc7f40d31 100644 --- a/tests/Feature/APIv1/TransportTest.php +++ b/tests/Feature/APIv1/TransportTest.php @@ -2,6 +2,7 @@ namespace Feature\APIv1; +use App\Models\Station; use App\Models\User; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Facades\Date; @@ -25,8 +26,8 @@ public function testGetDeparturesFetchTripAndCheckin(): void { $station = self::FRANKFURT_HBF['name']; $timestamp = Date::parse('next monday 8 am'); $this->actAsApiUserWithAllScopes(); - $response = $this->get( - uri: '/api/v1/trains/station/' . $station . '/departures?when=' . urlencode($timestamp->toIso8601String()), + $response = $this->get( + uri: '/api/v1/trains/station/' . $station . '/departures?when=' . urlencode($timestamp->toIso8601String()), ); $response->assertOk(); $response->assertJsonStructure([ @@ -63,18 +64,18 @@ public function testGetDeparturesFetchTripAndCheckin(): void { //Fetch trip with wrong origin / stopover $response = $this->get( - uri: '/api/v1/trains/trip' - . '?hafasTripId=' . $departure['tripId'] - . '&lineName=' . $departure['line']['name'] - . '&start=' . ($departure['stop']['id'] + 99999), + uri: '/api/v1/trains/trip' + . '?hafasTripId=' . $departure['tripId'] + . '&lineName=' . $departure['line']['name'] + . '&start=' . ($departure['stop']['id'] + 99999), ); $response->assertStatus(400); // Fetch correct trip $response = $this->get( - uri: '/api/v1/trains/trip' - . '?hafasTripId=' . $departure['tripId'] - . '&lineName=' . $departure['line']['name'] - . '&start=' . $departure['stop']['id'], + uri: '/api/v1/trains/trip' + . '?hafasTripId=' . $departure['tripId'] + . '&lineName=' . $departure['line']['name'] + . '&start=' . $departure['stop']['id'], ); $response->assertOk(); $response->assertJsonStructure([ @@ -105,16 +106,16 @@ public function testGetDeparturesFetchTripAndCheckin(): void { //Now checkin... $response = $this->postJson( - uri: '/api/v1/trains/checkin', - data: [ - 'tripId' => $departure['tripId'], - 'lineName' => $departure['line']['name'], - 'start' => $trip['stopovers'][0]['evaIdentifier'], - 'departure' => $trip['stopovers'][0]['departurePlanned'], - 'destination' => $trip['stopovers'][1]['evaIdentifier'], - 'arrival' => $trip['stopovers'][1]['arrivalPlanned'], - 'ibnr' => true, - ], + uri: '/api/v1/trains/checkin', + data: [ + 'tripId' => $departure['tripId'], + 'lineName' => $departure['line']['name'], + 'start' => $trip['stopovers'][0]['evaIdentifier'], + 'departure' => $trip['stopovers'][0]['departurePlanned'], + 'destination' => $trip['stopovers'][1]['evaIdentifier'], + 'arrival' => $trip['stopovers'][1]['arrivalPlanned'], + 'ibnr' => true, + ], ); $response->assertCreated(); $response->assertJsonStructure([ @@ -135,16 +136,16 @@ public function testGetDeparturesFetchTripAndCheckin(): void { //Do the same thing again! Should be a CheckInCollision $response = $this->postJson( - uri: '/api/v1/trains/checkin', - data: [ - 'tripId' => $departure['tripId'], - 'lineName' => $departure['line']['name'], - 'start' => $trip['stopovers'][0]['evaIdentifier'], - 'departure' => $trip['stopovers'][0]['departurePlanned'], - 'destination' => $trip['stopovers'][1]['evaIdentifier'], - 'arrival' => $trip['stopovers'][1]['arrivalPlanned'], - 'ibnr' => true, - ], + uri: '/api/v1/trains/checkin', + data: [ + 'tripId' => $departure['tripId'], + 'lineName' => $departure['line']['name'], + 'start' => $trip['stopovers'][0]['evaIdentifier'], + 'departure' => $trip['stopovers'][0]['departurePlanned'], + 'destination' => $trip['stopovers'][1]['evaIdentifier'], + 'arrival' => $trip['stopovers'][1]['arrivalPlanned'], + 'ibnr' => true, + ], ); $response->assertStatus(409); } @@ -180,21 +181,21 @@ public function testGetStationByCoordinatesIfNoStationIsNearby(): void { } public function testSetHome(): void { - $user = User::factory()->create(); + $user = User::factory()->create(); Passport::actingAs($user, ['*']); - $this->assertNull($user->home); + $station = Station::factory()->create(); - Http::fake(["*" => Http::response([self::HANNOVER_HBF])]); + $this->assertNull($user->home); - $response = $this->put('/api/v1/trains/station/Hannover Hbf/home'); + $response = $this->put('/api/v1/station/' . $station->id . '/home'); $response->assertOk(); $user->refresh(); - $this->assertEquals('Hannover Hbf', $user->home?->name); + $this->assertEquals($station->name, $user->home?->name); } public function testAutocompleteWithDs100(): void { - $user = User::factory()->create(); + $user = User::factory()->create(); Passport::actingAs($user, ['*']); Http::fake(["*/stations/" . self::HANNOVER_HBF['ril100'] => Http::response(self::HANNOVER_HBF)]);