From 1da9c75955dd890d4faba6cabf3254702b1f9a44 Mon Sep 17 00:00:00 2001 From: William Allen <16820599+williamjallen@users.noreply.github.com> Date: Tue, 2 Jan 2024 10:24:16 -0500 Subject: [PATCH] Pluralize index routes (#1900) It is traditional for "index" routes which display a list of subitems which can be navigated to via unique IDs to be pluralized. For example, a page showing the questions available should be under the route `/questions`, while a specific question should be available at `/questions/`. Examples of major websites which follow this convention: - https://stackoverflow.com/questions - https://github.com/orgs/Kitware/repositories - https://gitlab.kitware.com/cmake/cmake/-/merge_requests CDash currently has an inconsistent mix of route styles. As we work to modernize the codebase, we should also modernize our routing structure while maintaining backwards compatibility wherever possible. This PR standardizes the plurality of our routes which do not end with `.php`. Updating every link across the site is a major undertaking, and I plan to gradually update links as I refactor various pages as part of CDash 3.4. --- routes/web.php | 45 +++++++++++++++++++++++-------------- tests/Feature/CDashTest.php | 4 ++-- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/routes/web.php b/routes/web.php index acfb2335ac..206e61dfb1 100755 --- a/routes/web.php +++ b/routes/web.php @@ -66,60 +66,71 @@ return redirect("/image/{$imgid}", 301); }); -Route::get('/build/{id}', 'BuildController@summary'); +Route::get('/builds/{id}', 'BuildController@summary'); +Route::permanentRedirect('/build/{id}', '/builds/{id}'); Route::get('/buildSummary.php', function (Request $request) { $buildid = $request->query('buildid'); - return redirect("/build/{$buildid}", 301); + return redirect("/builds/{$buildid}", 301); }); -Route::get('/build/{id}/configure', 'BuildController@configure'); +Route::get('/builds/{id}/configure', 'BuildController@configure'); +Route::permanentRedirect('/builds/{id}/configure', '/builds/{id}/configure'); Route::get('/viewConfigure.php', function (Request $request) { $buildid = $request->query('buildid'); - return redirect("/build/{$buildid}/configure", 301); + return redirect("/builds/{$buildid}/configure", 301); }); -Route::get('/build/{id}/update', 'BuildController@update'); +Route::get('/builds/{id}/update', 'BuildController@update'); +Route::permanentRedirect('/build/{id}/update', '/builds/{id}/update'); Route::get('/viewUpdate.php', function (Request $request) { $buildid = $request->query('buildid'); - return redirect("/build/{$buildid}/update", 301); + return redirect("/builds/{$buildid}/update", 301); }); -Route::get('/build/{id}/notes', 'BuildController@notes'); +Route::get('/builds/{id}/notes', 'BuildController@notes'); +Route::permanentRedirect('/build/{id}/notes', '/builds/{id}/notes'); Route::get('/viewNotes.php', function (Request $request) { $buildid = $request->query('buildid'); - return redirect("/build/{$buildid}/notes", 301); + return redirect("/builds/{$buildid}/notes", 301); }); -Route::get('/build/{id}/dynamic_analysis', 'DynamicAnalysisController@viewDynamicAnalysis') +Route::get('/builds/{id}/dynamic_analysis', 'DynamicAnalysisController@viewDynamicAnalysis') ->whereNumber('id'); +Route::permanentRedirect('/build/{id}/dynamic_analysis', '/builds/{id}/dynamic_analysis'); Route::get('/viewDynamicAnalysis.php', function (Request $request) { $buildid = $request->query('buildid'); - return redirect("/build/{$buildid}/dynamic_analysis", 301); + return redirect("/builds/{$buildid}/dynamic_analysis", 301); }); -Route::get('/project/{id}/edit', 'EditProjectController@edit'); -Route::get('/project/new', 'EditProjectController@create'); +Route::get('/projects/{id}/edit', 'EditProjectController@edit'); +Route::permanentRedirect('/project/{id}/edit', '/projects/{id}/edit'); -Route::get('/project/{id}/testmeasurements', 'ManageMeasurementsController@show'); +Route::get('/projects/new', 'EditProjectController@create'); +Route::permanentRedirect('/project/new', '/projects/new'); -Route::get('/project/{id}/ctest_configuration', 'CTestConfigurationController@get') +Route::get('/projects/{id}/testmeasurements', 'ManageMeasurementsController@show'); +Route::permanentRedirect('/project/{id}/testmeasurements', '/projects/{id}/testmeasurements'); + +Route::get('/projects/{id}/ctest_configuration', 'CTestConfigurationController@get') ->whereNumber('id'); +Route::permanentRedirect('/project/{id}/ctest_configuration', '/projects/{id}/ctest_configuration'); Route::get('/generateCTestConfig.php', function (Request $request) { $projectid = $request->query('projectid'); if (!is_numeric($projectid)) { abort(400, 'Not a valid projectid!'); } - return redirect("/project/{$projectid}/ctest_configuration", 301); + return redirect("/projects/{$projectid}/ctest_configuration", 301); }); -Route::get('/test/{id}', 'TestController@details'); +Route::get('/tests/{id}', 'TestController@details'); +Route::permanentRedirect('/test/{id}', '/tests/{id}'); Route::get('/testDetails.php', function (Request $request) { $buildid = $request->query('build'); $testid = $request->query('test'); $buildtest = \App\Models\BuildTest::where('buildid', $buildid)->where('testid', $testid)->first(); if ($buildtest !== null) { - return redirect("/test/{$buildtest->id}", 301); + return redirect("/tests/{$buildtest->id}", 301); } abort(404); }); diff --git a/tests/Feature/CDashTest.php b/tests/Feature/CDashTest.php index 094d76d829..3338ebe61f 100755 --- a/tests/Feature/CDashTest.php +++ b/tests/Feature/CDashTest.php @@ -77,10 +77,10 @@ public function testRedirects() URL::forceRootUrl('http://localhost'); $response = $this->call('GET', '/buildSummary.php', ['buildid' => '2']); - $response->assertRedirect('/build/2'); + $response->assertRedirect('/builds/2'); $response = $this->call('GET', '/viewConfigure.php', ['buildid' => '5']); - $response->assertRedirect('/build/5/configure'); + $response->assertRedirect('/builds/5/configure'); } public function testOverrideLoginField()