Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(web): adopt browser routing for cleaner URIs #911

Merged
merged 9 commits into from
Oct 28, 2024
Merged
6 changes: 3 additions & 3 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# building frontend
FROM node:22-slim as frontend
FROM node:22-slim AS frontend
WORKDIR /app/frontend

COPY web/package.json web/yarn.lock ./
Expand Down Expand Up @@ -40,8 +40,8 @@ FROM python:3.12-slim
ENV MAX_UPLOAD_SIZE=100M

# set up the system
RUN apt update && \
apt install --yes nginx dumb-init libmagic1 gettext && \
RUN apt-get update && \
apt-get install --yes nginx dumb-init libmagic1 gettext && \
rm -rf /var/lib/apt/lists/*

RUN mkdir -p /var/docat/doc
Expand Down
3 changes: 2 additions & 1 deletion docat/docat/nginx/default
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ server {

location /doc {
root /var/docat;
absolute_redirect off;
}

location /api {
Expand All @@ -23,6 +24,6 @@ server {
}

location / {
try_files $uri $uri/ =404;
try_files $uri $uri/ /index.html =404;
}
}
2 changes: 1 addition & 1 deletion docat/docat/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def is_forbidden_project_name(name: str) -> bool:
a page on the docat website.
"""
name = name.lower().strip()
return name in ["upload", "claim", "delete", "help"]
return name in ["upload", "claim", "delete", "help", "doc", "api"]


UNITS_MAPPING = [
Expand Down
2 changes: 1 addition & 1 deletion docat/tests/test_rename.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def test_rename_rejects_forbidden_project_name(client_with_claimed_project):
assert create_response.status_code == 201

with patch("os.rename") as rename_mock:
for project_name in ["upload", "claim", "Delete ", "help"]:
for project_name in ["upload", "claim", "Delete ", "help", "Doc", "API"]:
rename_response = client_with_claimed_project.put(f"/api/some-project/rename/{project_name}", headers={"Docat-Api-Key": "1234"})
assert rename_response.status_code == 400
assert rename_response.json() == {
Expand Down
2 changes: 1 addition & 1 deletion docat/tests/test_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def test_upload_rejects_forbidden_project_name(client_with_claimed_project):
"""

with patch("docat.app.remove_docs") as remove_mock:
for project_name in ["upload", "claim", " Delete ", "help"]:
for project_name in ["upload", "claim", " Delete ", "help", "DOC", "api"]:
response = client_with_claimed_project.post(
f"/api/{project_name}/1.0.0", files={"file": ("index.html", io.BytesIO(b"<h1>Hello World</h1>"), "plain/text")}
)
Expand Down
18 changes: 4 additions & 14 deletions web/src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createHashRouter, RouterProvider } from 'react-router-dom'
import { createBrowserRouter, RouterProvider } from 'react-router-dom'
import { ConfigDataProvider } from './data-providers/ConfigDataProvider'
import { MessageBannerProvider } from './data-providers/MessageBannerProvider'
import { ProjectDataProvider } from './data-providers/ProjectDataProvider'
Expand All @@ -7,14 +7,13 @@ import { StatsDataProvider } from './data-providers/StatsDataProvider'
import Claim from './pages/Claim'
import Delete from './pages/Delete'
import Docs from './pages/Docs'
import EscapeSlashForDocsPath from './pages/EscapeSlashForDocsPath'
import Help from './pages/Help'
import Home from './pages/Home'
import NotFound from './pages/NotFound'
import Upload from './pages/Upload'

function App(): JSX.Element {
const router = createHashRouter([
const router = createBrowserRouter([
{
path: '/',
errorElement: <NotFound />,
Expand Down Expand Up @@ -54,17 +53,8 @@ function App(): JSX.Element {
element: <Docs />
},
{
path: ':page',
children: [
{
path: '',
element: <Docs />
},
{
path: '*',
element: <EscapeSlashForDocsPath />
}
]
path: '*',
element: <Docs />
}
]
}
Expand Down
6 changes: 4 additions & 2 deletions web/src/components/DocumentControlButtons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ export default function DocumentControlButtons(props: Props): JSX.Element {
url = url.replace(props.version, 'latest')
}

if (shareModalHideUi && !url.includes('?hide-ui=true')) {
url = `${url}?hide-ui=true`
if (shareModalHideUi) {
const urlObject = new URL(url)
urlObject.search = 'hide-ui'
url = urlObject.toString()
}

return url
Expand Down
33 changes: 16 additions & 17 deletions web/src/pages/Docs.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect, useMemo, useState, useRef } from 'react'
import { useEffect, useMemo, useState, useRef } from 'react'
import ProjectRepository from '../repositories/ProjectRepository'
import type ProjectDetails from '../models/ProjectDetails'
import LoadingPage from './LoadingPage'
Expand All @@ -11,7 +11,7 @@ import IframeNotFound from './IframePageNotFound'

export default function Docs(): JSX.Element {
const params = useParams()
const searchParams = useSearchParams()[0]
const [searchParams] = useSearchParams()
const location = useLocation()
const { showMessage, clearMessages } = useMessageBanner()

Expand All @@ -20,14 +20,11 @@ export default function Docs(): JSX.Element {
const [loadingFailed, setLoadingFailed] = useState<boolean>(false)

const project = useRef(params.project ?? '')
const page = useRef(params.page ?? 'index.html')
const hash = useRef(location.hash.split('?')[0] ?? '')
const page = useRef(params['*'] ?? '')
const hash = useRef(location.hash)

const [version, setVersion] = useState<string>(params.version ?? 'latest')
const [hideUi, setHideUi] = useState<boolean>(
searchParams.get('hide-ui') === 'true' ||
location.hash.split('?')[1] === 'hide-ui=true'
)
const [hideUi, setHideUi] = useState<boolean>(searchParams.get('hide-ui') === '' || searchParams.get('hide-ui') === 'true')
const [iframeUpdateTrigger, setIframeUpdateTrigger] = useState<number>(0)

// This provides the url for the iframe.
Expand Down Expand Up @@ -91,10 +88,9 @@ export default function Docs(): JSX.Element {
})()
}, [project])

/** Encodes the url for the current page, and escapes the path part to avoid
* redirecting to escapeSlashForDocsPath.
/** Encodes the url for the current page.
* @example
* getUrl('project', 'version', 'path/to/page.html', '#hash', false) -> '#/project/version/path%2Fto%2Fpage.html#hash'
* getUrl('project', 'version', 'path/to/page.html', '#hash', false) -> '/project/version/path/to/page.html#hash'
*/
const getUrl = (
project: string,
Expand All @@ -103,7 +99,7 @@ export default function Docs(): JSX.Element {
hash: string,
hideUi: boolean
): string => {
return `#/${project}/${version}/${encodeURIComponent(page)}${hash}${hideUi ? '?hide-ui=true' : ''}`
return `/${project}/${version}/${encodeURI(page)}${hash}${hideUi ? '?hide-ui' : ''}`
}

const updateUrl = (newVersion: string, hideUi: boolean): void => {
Expand All @@ -121,6 +117,11 @@ export default function Docs(): JSX.Element {
document.title = newTitle
}

// Keep compatibility with encoded page path
useEffect(() => {
updateUrl(version, hideUi)
}, [])

const iFramePageChanged = (urlPage: string, urlHash: string, title?: string): void => {
if (title != null && title !== document.title) {
updateTitle(title)
Expand Down Expand Up @@ -156,11 +157,9 @@ export default function Docs(): JSX.Element {
useEffect(() => {
const urlProject = params.project ?? ''
const urlVersion = params.version ?? 'latest'
const urlPage = params.page ?? 'index.html'
const urlHash = location.hash.split('?')[0] ?? ''
const urlHideUi =
searchParams.get('hide-ui') === 'true' ||
location.hash.split('?')[1] === 'hide-ui=true'
const urlPage = params['*'] ?? ''
const urlHash = location.hash
const urlHideUi = searchParams.get('hide-ui') === '' || searchParams.get('hide-ui') === 'true'

// update the state to the url params on first loadon
if (urlProject !== project.current) {
Expand Down
22 changes: 0 additions & 22 deletions web/src/pages/EscapeSlashForDocsPath.tsx

This file was deleted.

21 changes: 7 additions & 14 deletions web/src/pages/Home.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { useEffect, useState } from 'react';
import { useNavigate } from 'react-router';

import { Delete, ErrorOutline, FileUpload, KeyboardArrowDown, Lock } from '@mui/icons-material';
import { useLocation } from 'react-router';
import { useProjects } from '../data-providers/ProjectDataProvider';
import { useSearch } from '../data-providers/SearchProvider';
import { type Project } from '../models/ProjectsResponse';
Expand All @@ -20,26 +20,19 @@ import styles from './../style/pages/Home.module.css';


export default function Home(): JSX.Element {
const navigate = useNavigate()
const { loadingFailed } = useProjects()
const { stats, loadingFailed: statsLoadingFailed } = useStats()
const { filteredProjects: projects, query, setQuery } = useSearch()
const { filteredProjects: projects, query } = useSearch()
const [showAll, setShowAll] = useState(false);
const [favoriteProjects, setFavoriteProjects] = useState<Project[]>([])

const location = useLocation()

document.title = 'Home | docat'

// insert # into the url if it's missing
useEffect(() => {
const nonHostPart = window.location.href.replace(window.location.origin, '')

if (nonHostPart.startsWith('#') || nonHostPart.startsWith('/#')) {
return
}

window.location.replace(`/#${nonHostPart}`)
}, [location, setQuery, projects])
// Keep compatibility with hash-based URI
if (location.hash.startsWith('#/')) {
navigate(location.hash.replace('#', ''), { replace: true })
}

const updateFavorites = (): void => {
if (projects == null) return
Expand Down
2 changes: 1 addition & 1 deletion web/src/pages/IframePageNotFound.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ interface Props {
}

export default function IframeNotFound(props: Props): JSX.Element {
const link = `/${props.project}/${props.version}${props.hideUi ? '?hide-ui=true' : ''}`
const link = `/${props.project}/${props.version}${props.hideUi ? '?hide-ui' : ''}`

return (
<div className={styles['iframe-page-not-found']}>
Expand Down
29 changes: 0 additions & 29 deletions web/src/repositories/ProjectRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,34 +4,6 @@ import { type Project } from '../models/ProjectsResponse'

const RESOURCE = 'doc'

/**
* Escapes all slashes in a url to the docs page from the point between the version and the path.
* This is necessary because react-router thinks that the slashes are path separators.
* The slashes are escaped to %2F and reverted back to slashes by react-router.
* Example:
* /doc/project/1.0.0/path/to/page -> /doc/project/1.0.0/path%2Fto%2Fpage
* @param pathname useLocation().pathname
* @param search useLocation().search
* @param hash useLocation().hash
* @returns a url with escaped slashes
*/
function escapeSlashesInUrl(
pathname: string,
search: string,
hash: string
): string {
const url = pathname + hash + search
const projectAndVersion = url.split('/', 3).join('/')
let path = url.substring(projectAndVersion.length + 1)
path = path.replaceAll('/', '%2F')

if (path.length === 0) {
return projectAndVersion
}

return projectAndVersion + '/' + path
}

function dateTimeReviver(key: string, value: any) {
if (key === 'timestamp') {
return new Date(value)
Expand Down Expand Up @@ -273,7 +245,6 @@ function setFavorite(projectName: string, shouldBeFavorite: boolean): void {
}

const exp = {
escapeSlashesInUrl,
getVersions,
getLatestVersion,
filterHiddenVersions,
Expand Down
51 changes: 0 additions & 51 deletions web/src/tests/repositories/ProjectRepository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -473,54 +473,3 @@ describe('getLatestVersion', () => {
expect(latestVersion).toStrictEqual(versions[0])
})
})

describe('escapeSlashesInUrl', () => {
test('should ignore version and project name', () => {
const url = '/project/1.0.0'

expect(ProjectRepository.escapeSlashesInUrl(url, '', '')).toBe(url)
})

test('should ignore trailing slash', () => {
const given = '/project/1.0.0/'
const expected = '/project/1.0.0'

expect(ProjectRepository.escapeSlashesInUrl(given, '', '')).toBe(expected)
})

test('should escape slashes in path', () => {
const given = '/project/1.0.0/path/with/slashes'
const expected = '/project/1.0.0/path%2Fwith%2Fslashes'

expect(ProjectRepository.escapeSlashesInUrl(given, '', '')).toBe(expected)
})

test('should work with query parameters', () => {
const given = '/project/1.0.0/path/with/slashes'
const query = '?param=value'
const expected = '/project/1.0.0/path%2Fwith%2Fslashes?param=value'

expect(ProjectRepository.escapeSlashesInUrl(given, query, '')).toBe(
expected
)
})

test('should work with hash', () => {
const given = '/project/1.0.0/path/with/slashes'
const hash = '#hash'
const expected = '/project/1.0.0/path%2Fwith%2Fslashes#hash'

expect(ProjectRepository.escapeSlashesInUrl(given, '', hash)).toBe(expected)
})

test('should work with query parameters and hash', () => {
const given = '/project/1.0.0/path/with/slashes'
const query = '?param=value'
const hash = '#hash'
const expected = '/project/1.0.0/path%2Fwith%2Fslashes#hash?param=value'

expect(ProjectRepository.escapeSlashesInUrl(given, query, hash)).toBe(
expected
)
})
})