Преглед на файлове

RHL-015 refactor(docs): standardize language in API documentation and enhance PDF handling details

Code_Uwe преди 4 седмици
родител
ревизия
db654f3bbe
променени са 4 файла, в които са добавени 196 реда и са изтрити 113 реда
  1. 29 53
      Docs/api.md
  2. 53 30
      Docs/frontend-api-usage.md
  3. 74 24
      Docs/frontend-ui.md
  4. 40 6
      Docs/storage.md

+ 29 - 53
Docs/api.md

@@ -1,10 +1,10 @@
 <!-- --------------------------------------------------------------------------- -->
 
-<!-- Ordner: Docs -->
+<!-- Folder: Docs -->
 
-<!-- Datei: api.md -->
+<!-- File: api.md -->
 
-<!-- Relativer Pfad: Docs/api.md -->
+<!-- Relative Path: Docs/api.md -->
 
 <!-- --------------------------------------------------------------------------- -->
 
@@ -352,26 +352,14 @@ Destroy the current session by clearing the cookie.
 
 **Purpose**
 
-Return the current session identity for frontend consumers.
+Provide the current session identity for frontend consumers.
 
-Rationale:
+Semantics (frontend-friendly):
 
-- Frontends should not use `401` as basic control flow to determine “am I logged in?”.
-- This endpoint provides a stable, frontend-friendly session check.
+- 200 with `{ user: null }` when unauthenticated
+- 200 with `{ user: { userId, role, branchId } }` when authenticated
 
-**Authentication**: not required.
-
-**Response 200 (unauthenticated)**
-
-```json
-{ "user": null }
-```
-
-**Response 200 (authenticated)**
-
-```json
-{ "user": { "userId": "...", "role": "branch|admin|dev", "branchId": "NL01" } }
-```
+This avoids using 401 as control-flow for basic "am I logged in?" checks.
 
 ---
 
@@ -422,14 +410,6 @@ Example: `/api/branches/NL01/years`
 { "branch": "NL01", "years": ["2023", "2024"] }
 ```
 
-**Error responses (common)**
-
-- `401` → `AUTH_UNAUTHENTICATED`
-- `403` → `AUTH_FORBIDDEN_BRANCH`
-- `400` → `VALIDATION_MISSING_PARAM`
-- `404` → `FS_NOT_FOUND`
-- `500` → `FS_STORAGE_ERROR` / `INTERNAL_SERVER_ERROR`
-
 ---
 
 ### 4.7 `GET /api/branches/[branch]/[year]/months`
@@ -444,14 +424,6 @@ Example: `/api/branches/NL01/2024/months`
 { "branch": "NL01", "year": "2024", "months": ["01", "02", "10"] }
 ```
 
-**Error responses (common)**
-
-- `401` → `AUTH_UNAUTHENTICATED`
-- `403` → `AUTH_FORBIDDEN_BRANCH`
-- `400` → `VALIDATION_MISSING_PARAM`
-- `404` → `FS_NOT_FOUND`
-- `500` → `FS_STORAGE_ERROR` / `INTERNAL_SERVER_ERROR`
-
 ---
 
 ### 4.8 `GET /api/branches/[branch]/[year]/[month]/days`
@@ -466,14 +438,6 @@ Example: `/api/branches/NL01/2024/10/days`
 { "branch": "NL01", "year": "2024", "month": "10", "days": ["01", "23"] }
 ```
 
-**Error responses (common)**
-
-- `401` → `AUTH_UNAUTHENTICATED`
-- `403` → `AUTH_FORBIDDEN_BRANCH`
-- `400` → `VALIDATION_MISSING_PARAM`
-- `404` → `FS_NOT_FOUND`
-- `500` → `FS_STORAGE_ERROR` / `INTERNAL_SERVER_ERROR`
-
 ---
 
 ### 4.9 `GET /api/files?branch=&year=&month=&day=`
@@ -498,14 +462,6 @@ Example:
 }
 ```
 
-**Error responses (common)**
-
-- `401` → `AUTH_UNAUTHENTICATED`
-- `403` → `AUTH_FORBIDDEN_BRANCH`
-- `400` → `VALIDATION_MISSING_QUERY`
-- `404` → `FS_NOT_FOUND`
-- `500` → `FS_STORAGE_ERROR` / `INTERNAL_SERVER_ERROR`
-
 ---
 
 ### 4.10 `GET /api/files/:branch/:year/:month/:day/:filename`
@@ -542,11 +498,31 @@ Stream (or download) a single PDF file from the NAS while enforcing authenticati
 - Headers (example):
 
   - `Content-Type: application/pdf`
-  - `Content-Disposition: inline; filename="<filename>"` (or `attachment` when `download=1`)
+
   - `Cache-Control: no-store`
 
+  - `Content-Disposition`:
+
+    - Default: `inline`
+    - When `download=1`: `attachment`
+
+    For filename handling (Unicode-safe):
+
+    - `filename="..."` is an ASCII fallback (safe for header values)
+    - `filename*=UTF-8''...` contains the UTF-8 encoded original name (RFC 5987)
+
+    Example:
+
+    ```text
+    Content-Disposition: inline; filename="Euro _.pdf"; filename*=UTF-8''Euro%20%E2%82%AC.pdf
+    ```
+
 **Error responses (JSON)**
 
+Even though the happy path is binary, error responses remain standardized JSON.
+
+Common error codes:
+
 - `400` validation errors:
 
   - `VALIDATION_MISSING_PARAM`

+ 53 - 30
Docs/frontend-api-usage.md

@@ -17,6 +17,7 @@ Scope:
 - Stable **API v1** contracts (URLs, params, response shapes).
 - The minimal frontend `apiClient` helper layer (`lib/frontend/apiClient.js`).
 - Practical examples for building the first UI.
+- PDF streaming/opening behavior in the Explorer (RHL-023).
 
 > UI developers: For the app shell layout and frontend route scaffold (public vs protected routes, placeholder pages), see **`Docs/frontend-ui.md`**.
 >
@@ -25,11 +26,11 @@ Scope:
 Non-goals:
 
 - New major features.
-- PDF viewer UI implementation details (planned as RHL-023).
+- An in-app PDF viewer UI (beyond opening the browser’s PDF viewer in a new tab).
 
 Notes:
 
-- The backend provides a PDF stream/download endpoint (RHL-015). This document describes the **frontend calling pattern** for that endpoint.
+- The backend provides a binary PDF stream/download endpoint (RHL-015). The Explorer integrates it for “Open PDF” (RHL-023).
 
 ---
 
@@ -57,6 +58,7 @@ The core UI flow is a simple drill-down:
 5. `getMonths(branch, year)`
 6. `getDays(branch, year, month)`
 7. `getFiles(branch, year, month, day)`
+8. Open a PDF via the binary endpoint (see section 4.3)
 
 ### 1.3 Example usage (client-side)
 
@@ -347,32 +349,59 @@ Success:
 
 #### `GET /api/files/:branch/:year/:month/:day/:filename`
 
-This endpoint returns **binary PDF data** (not JSON).
+This endpoint returns **binary PDF data** on the happy path (not JSON).
 
-Recommended frontend usage:
+Frontend rules:
 
-- Open inline in a new tab/window so the browser handles PDF rendering.
-- Do **not** call this endpoint via `apiClient.apiFetch()` because it is JSON-centric.
+- **Do not call this endpoint via `apiClient.apiFetch()`**.
 
-Open inline:
+  - `apiClient` is JSON-centric and will try to parse the response.
+
+- Prefer opening the endpoint URL in a **new tab** so the browser handles PDF rendering.
+
+##### 4.3.1 Centralized URL builder
+
+File:
+
+- `lib/frontend/explorer/pdfUrl.js`
+
+Exports:
+
+- `buildPdfUrl({ branch, year, month, day, filename })`
+- `buildPdfDownloadUrl({ branch, year, month, day, filename })` (adds `?download=1`)
+
+Why it exists:
+
+- Keeps URL construction consistent.
+- Ensures the `filename` segment is encoded correctly.
+
+##### 4.3.2 Recommended UI usage pattern
+
+In the Explorer table, we open PDFs via navigation:
+
+- Use a normal anchor element with `target="_blank"`.
+- This avoids popup-blocker issues and is semantically correct.
+
+Example:
 
 ```js
-const url = `/api/files/${branch}/${year}/${month}/${day}/${encodeURIComponent(
-	filename
-)}`;
-window.open(url, "_blank", "noopener,noreferrer");
+import { buildPdfUrl } from "@/lib/frontend/explorer/pdfUrl";
+
+const href = buildPdfUrl({ branch, year, month, day, filename });
+
+// In JSX:
+// <a href={href} target="_blank" rel="noopener noreferrer">Öffnen</a>
 ```
 
 Force download:
 
 ```js
-const url = `/api/files/${branch}/${year}/${month}/${day}/${encodeURIComponent(
-	filename
-)}?download=1`;
-window.open(url, "_blank", "noopener,noreferrer");
+import { buildPdfDownloadUrl } from "@/lib/frontend/explorer/pdfUrl";
+
+const href = buildPdfDownloadUrl({ branch, year, month, day, filename });
 ```
 
-Important notes:
+##### 4.3.3 Important filename and cookie notes
 
 - Use the exact `files[].name` returned by `getFiles()` (case-sensitive on Linux).
 
@@ -386,22 +415,13 @@ Important notes:
   - If you are logged in on `http://localhost:3000`, also open the PDF on `http://localhost:3000`.
   - Switching to `http://127.0.0.1:3000` will not send the cookie (different host) and results in `401`.
 
-### 4.4 Health
-
-#### `GET /api/health`
-
-Always returns `200` and reports partial system state:
-
-- database connectivity
-- NAS readability
-
 ---
 
 ## 5. Error handling
 
 ### 5.1 Standard error payload
 
-All error responses use:
+All JSON error responses use:
 
 ```json
 {
@@ -464,7 +484,8 @@ Frontend guidance:
 
 The repository contains a manual smoke test script that exercises:
 
-- happy path drill-down
+- authentication
+- drill-down navigation (branches -> years -> months -> days -> files)
 - negative cases (401/403/400/404)
 
 Script:
@@ -512,6 +533,8 @@ Rules:
 
 ## 9. Out of scope / planned additions
 
-- PDF viewer UX (RHL-023): enabling the Explorer “Open” button and providing a polished open/download experience.
-- Search UI and filters (`/:branch/search`).
-- Admin/dev branch selector and additional sidebar navigation.
+- Search UI and filters (route exists as placeholder: `/:branch/search`).
+- Optional Explorer UX polish:
+
+  - add a dedicated “Herunterladen” UI action (download variant)
+  - optional in-app PDF viewer experience (instead of a new tab)

+ 74 - 24
Docs/frontend-ui.md

@@ -8,7 +8,7 @@
 
 <!-- --------------------------------------------------------------------------- -->
 
-# Frontend UI: App Shell, Routing, Auth/RBAC, and Explorer (RHL-019 / RHL-020 / RHL-021 / RHL-022)
+# Frontend UI: App Shell, Routing, Auth/RBAC, and Explorer (RHL-019 / RHL-020 / RHL-021 / RHL-022 / RHL-023)
 
 This document describes the **frontend routing scaffold**, the **application shell layout**, and the **core navigation UI (Explorer)** for the RHL Lieferscheine app.
 
@@ -18,6 +18,7 @@ Timeline:
 - **RHL-020**: Real login flow + session handling and redirects.
 - **RHL-021**: UI-side RBAC guard + consistent Forbidden / NotFound UX.
 - **RHL-022**: Explorer v2 (Year → Month → Day → Files) + shadcn Breadcrumbs with dropdowns.
+- **RHL-023**: Explorer file action “Open PDF” using the binary PDF endpoint (opens in a new tab).
 
 > **Language policy**
 >
@@ -29,7 +30,7 @@ Timeline:
 
 ## 1. Scope
 
-### 1.1 Implemented (as of RHL-022)
+### 1.1 Implemented (as of RHL-023)
 
 - **Public** `/login` route with a functional login form (shadcn/ui primitives).
 
@@ -86,14 +87,34 @@ Timeline:
   - Error states with retry
   - FS_NOT_FOUND mapped to an Explorer “path no longer exists” card
 
+- **Explorer leaf action: Open PDF (RHL-023)**
+
+  - The file list on `/:branch/:year/:month/:day` provides an **“Öffnen”** action.
+
+  - Clicking “Öffnen” opens the selected PDF **in a new browser tab**.
+
+  - URL construction is centralized in a pure helper:
+
+    - `lib/frontend/explorer/pdfUrl.js` (`buildPdfUrl`, optional `buildPdfDownloadUrl`)
+
+  - The PDF endpoint is **binary** (`application/pdf`) and is opened via navigation (`<a target="_blank">`).
+
+  - The frontend does **not** use `apiClient.apiFetch()` for PDF opening (JSON-centric).
+
 ### 1.2 Still out of scope / planned
 
 - Search UI (route exists as placeholder: `/:branch/search`).
-- PDF viewer / streaming endpoint integration (planned as RHL-023). File “Open” stays disabled for now.
+
 - Admin/dev branch selector in the sidebar.
-- Performance polish:
 
-  - smoother navigation via client-side caching / prefetching
+- Optional Explorer improvements:
+
+  - “Herunterladen” action (download variant) next to “Öffnen”
+  - a dedicated in-app PDF viewer UI (instead of a new tab)
+
+- Perceived performance polish:
+
+  - client-side caching/prefetch
   - skeleton/layout shift reduction
 
 ---
@@ -185,6 +206,7 @@ File: `components/auth/AuthProvider.jsx`
 Behavior:
 
 1. On mount, call `apiClient.getMe()`.
+
 2. If `{ user: { ... } }`:
 
    - set auth state to `authenticated`
@@ -219,12 +241,14 @@ Files:
 Flow:
 
 1. Login page parses query params using `parseLoginParams(...)`.
+
 2. If `reason` is present:
 
    - `expired` → show “session expired” banner (German)
    - `logged-out` → show “logged out” banner (German)
 
 3. On submit, the form calls `apiClient.login({ username, password })`.
+
 4. On success:
 
    - redirect to `next` if present
@@ -348,7 +372,7 @@ Where NotFound is shown:
 
 ---
 
-## 6. Explorer v2 (RHL-022)
+## 6. Explorer v2 (RHL-022) + PDF Open (RHL-023)
 
 ### 6.1 UI goal
 
@@ -368,7 +392,7 @@ Routes and components:
 ### 6.3 Data fetching strategy
 
 - All Explorer pages are **Client Components**.
-- All API calls go through `lib/frontend/apiClient.js`.
+- All JSON API calls go through `lib/frontend/apiClient.js`.
 - A small hook provides consistent query state:
 
   - `lib/frontend/hooks/useExplorerQuery.js`
@@ -424,7 +448,13 @@ Error mapping:
   - `FS_NOT_FOUND` → ExplorerNotFound
   - other errors → ExplorerError + retry
 
-### 6.6 Files list
+### 6.6 Files list (leaf route) and “Open PDF”
+
+Leaf route:
+
+- `/:branch/:year/:month/:day`
+
+Files list behavior:
 
 - Uses shadcn/ui `Table`.
 - Shows:
@@ -432,9 +462,25 @@ Error mapping:
   - file name
   - relative path (desktop column + mobile secondary line)
 
-- Primary action:
+Primary file action:
+
+- “Öffnen” opens the PDF in a **new browser tab** via the binary PDF endpoint:
+
+  - `GET /api/files/:branch/:year/:month/:day/:filename`
 
-  - “Öffnen” button remains disabled until the PDF endpoint/viewer ticket (RHL-023).
+Implementation notes:
+
+- URL construction is centralized in:
+
+  - `lib/frontend/explorer/pdfUrl.js`
+
+- The PDF endpoint is binary (`application/pdf`). The UI uses navigation (`<a target="_blank">`) instead of `apiClient`.
+
+- The filename segment must be URL-encoded (handled by `buildPdfUrl(...)`).
+
+Accessibility:
+
+- The “Öffnen” action uses an `aria-label` like `PDF öffnen: <filename>`.
 
 ---
 
@@ -496,6 +542,7 @@ Explorer helper tests:
 - `lib/frontend/explorer/errorMapping.test.js`
 - `lib/frontend/explorer/formatters.test.js`
 - `lib/frontend/explorer/sorters.test.js`
+- `lib/frontend/explorer/pdfUrl.test.js` (RHL-023)
 
 Component SSR smoke test:
 
@@ -559,24 +606,17 @@ Explorer checks:
 - `/:branch/:year` shows months
 - `/:branch/:year/:month` shows days
 - `/:branch/:year/:month/:day` shows files
-- Breadcrumb dropdowns:
-
-  - year dropdown exists on month/day/files levels
-  - month dropdown exists on day/files levels
-  - day dropdown exists on files level
 
-### 10.2 Server
+PDF open (RHL-023):
 
-Deploy and verify on the server URL.
+- On `/:branch/:year/:month/:day`, click **“Öffnen”** on multiple files
 
-Important cookie note:
+  - Expected: opens the PDF in a new tab
+  - Expected: works for filenames with spaces and special characters (URL-encoding)
 
-- Browsers reject `Secure` cookies over HTTP.
-- Therefore the server `.env.server` must set:
+### 10.2 Server
 
-```env
-SESSION_COOKIE_SECURE=false
-```
+Deploy and verify on the server URL.
 
 Verify flows:
 
@@ -589,13 +629,23 @@ Admin/dev checks:
 - existing branches render
 - non-existing branch (e.g. `/NL9999`) shows NotFound (existence validation)
 
+PDF open (RHL-023):
+
+- Repeat the local PDF open checks against real NAS data
+
 ---
 
 ## 11. Planned follow-ups
 
 - Search UI and filters (`/:branch/search`).
-- PDF open/view experience (RHL-023).
+
+- Optional Explorer UI enhancements:
+
+  - add a secondary action “Herunterladen” (download variant)
+  - optional in-app PDF viewer experience (instead of a new tab)
+
 - Admin/dev branch selector in the sidebar.
+
 - Smooth navigation / perceived performance improvements:
 
   - reduce skeleton/layout shift

+ 40 - 6
Docs/storage.md

@@ -1,10 +1,10 @@
 <!-- --------------------------------------------------------------------------- -->
 
-<!-- Ordner: Docs -->
+<!-- Folder: Docs -->
 
-<!-- Datei: storage.md -->
+<!-- File: storage.md -->
 
-<!-- Relativer Pfad: Docs/storage.md -->
+<!-- Relative Path: Docs/storage.md -->
 
 <!-- --------------------------------------------------------------------------- -->
 
@@ -169,7 +169,7 @@ The NAS content can change at any time (new scans). To reduce filesystem load wh
 
 ## 7. File streaming endpoints (PDF delivery)
 
-The storage module currently focuses on **listing** directory contents.
+The storage module focuses on **listing** directory contents.
 
 For endpoints that must return **binary file data** (PDF streaming/download), a direct stream approach is preferred:
 
@@ -198,6 +198,8 @@ Rules:
 - Validate filename:
 
   - must be a simple file name (no `/`, `\`, or `..` segments)
+  - must not contain control characters (`\r`, `\n`, `\t`)
+  - must not contain quotes (`"`) to keep headers predictable
   - only `.pdf` is allowed
 
 - Apply a root containment check:
@@ -225,9 +227,40 @@ Recommended approach:
 For PDF streaming:
 
 - `Content-Type: application/pdf`
-- `Content-Disposition: inline; filename="..."` (default)
-- `Content-Disposition: attachment; filename="..."` (when `download=1`)
 - `Cache-Control: no-store`
+- `X-Content-Type-Options: nosniff`
+
+**Content-Disposition**
+
+Use `Content-Disposition` to control inline viewing vs download:
+
+- Default: `inline`
+- When `download=1`: `attachment`
+
+Important: Unicode-safe filename handling
+
+- Many Web `Response` implementations require header values to be byte-safe.
+- Raw Unicode characters (e.g. `€`) in `filename="..."` can crash response creation.
+
+Recommended pattern (RFC 5987):
+
+- Provide an ASCII fallback via `filename="..."` (safe in headers)
+- Provide the real UTF-8 name via `filename*=UTF-8''...`
+
+Example:
+
+```text
+Content-Disposition: inline; filename="Euro _.pdf"; filename*=UTF-8''Euro%20%E2%82%AC.pdf
+```
+
+### 7.4 Frontend calling pattern
+
+Because streaming endpoints are binary:
+
+- Frontends should not call them via JSON-centric clients.
+- Prefer opening the URL in a new tab/window and let the browser render the PDF.
+
+For the current UI implementation, the Explorer uses a pure URL builder helper and opens the endpoint via navigation (see `Docs/frontend-api-usage.md`).
 
 ---
 
@@ -240,6 +273,7 @@ Potential follow-ups for the storage layer:
   - strict validation
   - safe path construction
   - `stat()` + stream creation
+  - Unicode-safe `Content-Disposition` handling
   - consistent error details
 
 This is optional; the current v1 design keeps `lib/storage` focused on listing operations.