fix: add 301 redirects from /documentation/* to /docs/*#199
fix: add 301 redirects from /documentation/* to /docs/*#199ManukMinasyan merged 3 commits intomainfrom
Conversation
- Add min:20 validation to contact form message field - Use API_DOMAIN env var for Scribe docs base URL when set
Hidden tab panels used visibility:hidden which preserves layout space, causing the tallest panel (AI Agent chat) to force container height for all tabs. Switch hidden panels to absolute positioning so only the active panel determines container height.
Old documentation URLs return 404 and are indexed in the sitemap. Redirect permanently to preserve SEO equity and fix external links.
There was a problem hiding this comment.
Pull request overview
Adds support for legacy documentation URLs by redirecting old /documentation/* routes to the current /docs/* location, addressing broken sitemap links and preserving SEO value.
Changes:
- Added a permanent redirect from
/documentation/{slug?}to/docs/{slug?}. - Adjusted homepage hero tab panel positioning/visibility behavior.
- Updated Scribe API docs base URL logic and tightened contact form validation (minimum message length).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| routes/web.php | Adds legacy /documentation/* → /docs/* redirect route. |
| resources/views/home/partials/hero.blade.php | Updates hero tab panel layout classes and show/hide behavior. |
| config/scribe.php | Changes Scribe base_url computation when app.api_domain is configured. |
| app/Http/Requests/ContactRequest.php | Adds a minimum length constraint to contact form message validation. |
| // The base URL displayed in the docs. | ||
| // If you're using `laravel` type, you can set this to a dynamic string, like '{{ config("app.tenant_url") }}' to get a dynamic base URL. | ||
| 'base_url' => config('app.url'), | ||
| 'base_url' => config('app.api_domain') ? 'https://'.config('app.api_domain') : config('app.url'), |
There was a problem hiding this comment.
This PR is described as only adding legacy /documentation/* redirects, but it also includes unrelated changes (Scribe base_url, homepage hero tab styling, and contact form validation). Either update the PR description/test plan to cover these behaviors or split them into separate PRs to keep scope focused and reviewable.
| // The base URL displayed in the docs. | ||
| // If you're using `laravel` type, you can set this to a dynamic string, like '{{ config("app.tenant_url") }}' to get a dynamic base URL. | ||
| 'base_url' => config('app.url'), | ||
| 'base_url' => config('app.api_domain') ? 'https://'.config('app.api_domain') : config('app.url'), |
There was a problem hiding this comment.
base_url hardcodes https:// and drops any scheme/port configured in app.url (eg local http://localhost:8000 or non-443 ports). This can generate incorrect URLs in the rendered API docs; consider deriving scheme + port from config('app.url') (similar to the URL::getApiUrl macro logic) when app.api_domain is set.
| Route::get('/documentation/{slug?}', fn (string $slug = '') => redirect("/docs/{$slug}", 301)) | ||
| ->where('slug', '.*'); | ||
|
|
There was a problem hiding this comment.
When slug is empty (eg requesting /documentation), this redirects to /docs/ (trailing slash) rather than /docs as described in the test plan. Consider building the target path conditionally (only add /$slug when non-empty) and normalizing leading/trailing slashes to avoid // in edge cases.
| Route::get('/documentation/{slug?}', fn (string $slug = '') => redirect("/docs/{$slug}", 301)) | |
| ->where('slug', '.*'); | |
| Route::get('/documentation/{slug?}', function (?string $slug = null) { | |
| $normalizedSlug = ltrim((string) $slug, '/'); | |
| $path = '/docs'; | |
| if ($normalizedSlug !== '') { | |
| $path .= '/' . $normalizedSlug; | |
| } | |
| return redirect($path, 301); | |
| })->where('slug', '.*'); |
| 'email' => ['required', 'email:rfc,dns', 'max:255'], | ||
| 'company' => ['nullable', 'string', 'max:255'], | ||
| 'message' => ['required', 'string', 'max:5000'], | ||
| 'message' => ['required', 'string', 'min:20', 'max:5000'], |
There was a problem hiding this comment.
This adds a new validation constraint (min:20) for contact messages, but there are no feature tests covering the /contact form submission/validation behavior. Please add tests for both the rejection case (<20 chars) and acceptance case (>=20) so this change is protected against regressions.
Summary
/documentation/*URLs return 404 (reported by OhDear sitemap check)/docs/*Test plan
/documentation→/docs(301)/documentation/api→/docs/api(301)/documentation/getting-started→/docs/getting-started(301)php artisan app:generate-sitemap