From 5bf8e752e75d15451628d609e1d8e7864ce83dee Mon Sep 17 00:00:00 2001 From: Andras Schmelczer Date: Sun, 31 May 2026 09:39:34 +0100 Subject: [PATCH] Reveiw --- backend/src/life_towers/auth.py | 29 ++++++++++--------- backend/src/life_towers/limits.py | 9 ++---- backend/src/life_towers/logging.py | 10 +++---- backend/src/life_towers/main.py | 4 +-- .../life_towers/migrations/001_initial.sql | 6 ++-- backend/tests/test_api.py | 11 ------- frontend/angular.json | 1 - .../components/modal/block-edit.component.ts | 25 +++++++++------- .../components/modal/settings.component.ts | 10 ++++--- .../modal/tower-settings.component.ts | 8 +++-- .../src/app/components/page/page.component.ts | 11 ++++--- .../app/components/pages/pages.component.scss | 6 ---- .../app/components/pages/pages.component.ts | 8 +++-- .../color-picker/color-picker.component.ts | 5 ++-- .../double-slider/double-slider.component.ts | 4 +-- .../app/components/tower/tower.component.ts | 9 +++--- .../components/welcome/welcome.component.ts | 12 ++++---- .../src/app/services/api.service.vitest.ts | 17 ----------- frontend/src/app/utils/hash.ts | 2 +- frontend/tsconfig.app.json | 2 +- frontend/tsconfig.spec.json | 2 +- frontend/vitest.config.ts | 2 +- 22 files changed, 81 insertions(+), 112 deletions(-) diff --git a/backend/src/life_towers/auth.py b/backend/src/life_towers/auth.py index 8f7e01f..45f396a 100644 --- a/backend/src/life_towers/auth.py +++ b/backend/src/life_towers/auth.py @@ -2,11 +2,10 @@ from __future__ import annotations -import uuid - from fastapi import HTTPException, Request from .db import db_connection +from .models import _canonical_uuidv4 # Single generic detail used for ALL 401 responses. Per spec, the response # must not distinguish between missing / malformed / unknown tokens — that @@ -21,26 +20,28 @@ def _unauthorized() -> HTTPException: ) -def get_current_user(request: Request) -> str: - """Dependency that extracts and validates a Bearer token, returns user_id.""" +def extract_bearer_token(request: Request) -> str | None: + """Return the raw Bearer token from the Authorization header, or None.""" auth_header = request.headers.get("Authorization") or request.headers.get( "authorization" ) if not auth_header: - raise _unauthorized() - + return None parts = auth_header.split() - if len(parts) != 2 or parts[0].lower() != "bearer": - raise _unauthorized() + if len(parts) == 2 and parts[0].lower() == "bearer": + return parts[1] + return None - token = parts[1] + +def get_current_user(request: Request) -> str: + """Dependency that extracts and validates a Bearer token, returns user_id.""" + token = extract_bearer_token(request) + if token is None: + raise _unauthorized() try: - u = uuid.UUID(token) - if u.version != 4: - raise ValueError("Not v4") - token = str(u) - except (ValueError, AttributeError): + token = _canonical_uuidv4(token) + except ValueError: raise _unauthorized() with db_connection() as conn: diff --git a/backend/src/life_towers/limits.py b/backend/src/life_towers/limits.py index b566d84..bc6f90d 100644 --- a/backend/src/life_towers/limits.py +++ b/backend/src/life_towers/limits.py @@ -8,6 +8,8 @@ from fastapi import Request, Response from slowapi import Limiter from slowapi.util import get_remote_address +from .auth import extract_bearer_token + PAYLOAD_LIMIT_BYTES = 2 * 1024 * 1024 # 2 MiB _TOO_LARGE_BODY = json.dumps( @@ -20,12 +22,7 @@ _TOO_LARGE_BODY = json.dumps( def _get_token_or_ip(request: Request) -> str: """Key function for rate limiting: use Bearer token if present, else IP.""" - auth = request.headers.get("Authorization") or request.headers.get("authorization") - if auth: - parts = auth.split() - if len(parts) == 2 and parts[0].lower() == "bearer": - return parts[1] - return get_remote_address(request) + return extract_bearer_token(request) or get_remote_address(request) limiter = Limiter(key_func=_get_token_or_ip, default_limits=[]) diff --git a/backend/src/life_towers/logging.py b/backend/src/life_towers/logging.py index 1cb9288..539b01b 100644 --- a/backend/src/life_towers/logging.py +++ b/backend/src/life_towers/logging.py @@ -9,6 +9,8 @@ from hashlib import sha256 import structlog from fastapi import Request, Response +from .auth import extract_bearer_token + def configure_logging() -> None: """Configure structlog for JSON output.""" @@ -38,12 +40,8 @@ async def request_logging_middleware(request: Request, call_next) -> Response: structlog.contextvars.bind_contextvars(request_id=request_id) # Extract user_id from Authorization header for logging (no DB call here) - auth = request.headers.get("Authorization") or request.headers.get("authorization") - user_id: str | None = None - if auth: - parts = auth.split() - if len(parts) == 2 and parts[0].lower() == "bearer": - user_id = token_log_id(parts[1]) + token = extract_bearer_token(request) + user_id = token_log_id(token) if token else None start = time.monotonic() response = await call_next(request) diff --git a/backend/src/life_towers/main.py b/backend/src/life_towers/main.py index 0e2a29a..b9be419 100644 --- a/backend/src/life_towers/main.py +++ b/backend/src/life_towers/main.py @@ -98,7 +98,7 @@ def create_app() -> FastAPI: if field ) if fields: - detail_str = "Validation failed for: " + ", ".join(f for f in fields if f) + detail_str = "Validation failed for: " + ", ".join(fields) else: detail_str = "Validation failed" return JSONResponse( @@ -116,7 +116,7 @@ def create_app() -> FastAPI: code = STATUS_CODE_MAP.get(exc.status_code, "server_error") detail = {"error": code, "detail": str(exc.detail)} - headers = getattr(exc, "headers", None) or {} + headers = exc.headers or {} return JSONResponse(status_code=exc.status_code, content=detail, headers=headers) # Generic 500 handler diff --git a/backend/src/life_towers/migrations/001_initial.sql b/backend/src/life_towers/migrations/001_initial.sql index b8d3761..372c02f 100644 --- a/backend/src/life_towers/migrations/001_initial.sql +++ b/backend/src/life_towers/migrations/001_initial.sql @@ -1,10 +1,8 @@ -- Life Towers v4 initial schema. --- SQLite with WAL mode and foreign keys enabled at connection time. +-- WAL mode, foreign keys, and busy_timeout are applied per-connection in +-- db._apply_pragmas(), so they are not (and need not be) set here. -- All timestamps are unix epoch seconds (INTEGER). -PRAGMA journal_mode = WAL; -PRAGMA foreign_keys = ON; - CREATE TABLE IF NOT EXISTS users ( id TEXT PRIMARY KEY, created_at INTEGER NOT NULL, diff --git a/backend/tests/test_api.py b/backend/tests/test_api.py index 404fa09..a4f8538 100644 --- a/backend/tests/test_api.py +++ b/backend/tests/test_api.py @@ -51,17 +51,6 @@ async def client(tmp_path: Path) -> AsyncGenerator[AsyncClient, None]: db_module._DB_PATH = None -# --------------------------------------------------------------------------- -# Health -# --------------------------------------------------------------------------- - -@pytest.mark.asyncio -async def test_health(client: AsyncClient) -> None: - resp = await client.get("/api/v1/health") - assert resp.status_code == 200 - assert resp.json() == {"status": "ok"} - - @pytest.mark.asyncio async def test_spa_index_injects_absolute_open_graph_urls( tmp_path: Path, monkeypatch: pytest.MonkeyPatch diff --git a/frontend/angular.json b/frontend/angular.json index 2639328..e8f741a 100644 --- a/frontend/angular.json +++ b/frontend/angular.json @@ -8,7 +8,6 @@ ], "analytics": false }, - "newProjectRoot": "projects", "projects": { "frontend": { "projectType": "application", diff --git a/frontend/src/app/components/modal/block-edit.component.ts b/frontend/src/app/components/modal/block-edit.component.ts index 64e73e1..51cb5eb 100644 --- a/frontend/src/app/components/modal/block-edit.component.ts +++ b/frontend/src/app/components/modal/block-edit.component.ts @@ -4,7 +4,6 @@ import { input, output, signal, - computed, effect, viewChild, ElementRef, @@ -614,10 +613,12 @@ export class BlockEditComponent implements AfterViewInit { const t = this.tags(); untracked(() => { const cur = this.newValue(); - if (!cur.tag && t.length > 0) { - this.newValue.set({ ...cur, tag: t[0], is_done: this.defaultDone() }); - } else if (!cur.tag) { - this.newValue.set({ ...cur, is_done: this.defaultDone() }); + if (!cur.tag) { + this.newValue.set({ + ...cur, + tag: t.length > 0 ? t[0] : '', + is_done: this.defaultDone(), + }); } }); }); @@ -648,19 +649,21 @@ export class BlockEditComponent implements AfterViewInit { ); } + private colorOfTag(tag: string): string { + return tag ? getColorOfTag(tag, this.baseColor()) : 'transparent'; + } + colorOfTagForBlock(id: string): string { - const v = this.editedFor(id); - return v.tag ? getColorOfTag(v.tag, this.baseColor()) : 'transparent'; + return this.colorOfTag(this.editedFor(id).tag); } tagPlaceholder(fallback: string): string { return this.tags().length === 0 ? 'No tags yet. Open to create one.' : fallback; } - colorOfNewTag = computed(() => { - const t = this.newValue().tag; - return t ? getColorOfTag(t, this.baseColor()) : 'transparent'; - }); + colorOfNewTag(): string { + return this.colorOfTag(this.newValue().tag); + } formatDate(ts: number, compact = false): string { const d = new Date(ts * 1000); diff --git a/frontend/src/app/components/modal/settings.component.ts b/frontend/src/app/components/modal/settings.component.ts index 289e5e9..9d044ce 100644 --- a/frontend/src/app/components/modal/settings.component.ts +++ b/frontend/src/app/components/modal/settings.component.ts @@ -16,9 +16,6 @@ export interface UpdatePagePayload { keep_tasks_open: boolean; } -const UUIDV4_RE = - /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; - @Component({ selector: 'lt-settings', standalone: true, @@ -255,10 +252,15 @@ export class SettingsComponent { readonly hideCreateTowerButton = signal(false); readonly keepTasksOpen = signal(false); + private static readonly UUIDV4_RE = + /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; + // Token-switch state readonly tokenInput = signal(''); readonly tokenInputTouched = signal(false); - readonly isValidToken = computed(() => UUIDV4_RE.test(this.tokenInput())); + readonly isValidToken = computed(() => + SettingsComponent.UUIDV4_RE.test(this.tokenInput()), + ); constructor() { effect(() => { diff --git a/frontend/src/app/components/modal/tower-settings.component.ts b/frontend/src/app/components/modal/tower-settings.component.ts index 71389f6..7443e40 100644 --- a/frontend/src/app/components/modal/tower-settings.component.ts +++ b/frontend/src/app/components/modal/tower-settings.component.ts @@ -138,12 +138,14 @@ export class TowerSettingsComponent implements OnInit { onSubmit(): void { // Only the create flow reaches here via its Submit button; edit mode // auto-saves (and Enter on the single field is a harmless redundant save). - if (this.form.invalid) return; - this.emitSave(); + this.tryEmitSave(); } - /** Emit a save only when the form is valid (skips e.g. an empty name). */ private autoSave(): void { + this.tryEmitSave(); + } + + private tryEmitSave(): void { if (this.form.invalid) return; this.emitSave(); } diff --git a/frontend/src/app/components/page/page.component.ts b/frontend/src/app/components/page/page.component.ts index 5b0d990..544ed13 100644 --- a/frontend/src/app/components/page/page.component.ts +++ b/frontend/src/app/components/page/page.component.ts @@ -214,13 +214,16 @@ export class PageComponent { onTrashEnter(): void { this.nearTrashcan = true; - const preview = document.querySelector('.cdk-drag-preview'); - if (preview) preview.classList.add('trash-highlight'); + this.dragPreview()?.classList.add('trash-highlight'); } onTrashLeave(): void { this.nearTrashcan = false; - const preview = document.querySelector('.cdk-drag-preview'); - if (preview) preview.classList.remove('trash-highlight'); + this.dragPreview()?.classList.remove('trash-highlight'); + } + + /** The CDK drag preview currently in flight, if any. Matches legacy DOM-driven trash highlight. */ + private dragPreview(): Element | null { + return document.querySelector('.cdk-drag-preview'); } } diff --git a/frontend/src/app/components/pages/pages.component.scss b/frontend/src/app/components/pages/pages.component.scss index 13f10bb..cdb7480 100644 --- a/frontend/src/app/components/pages/pages.component.scss +++ b/frontend/src/app/components/pages/pages.component.scss @@ -71,12 +71,6 @@ } } - p { - strong { - font-weight: bold; - } - } - .confirm-buttons { display: flex; justify-content: center; diff --git a/frontend/src/app/components/pages/pages.component.ts b/frontend/src/app/components/pages/pages.component.ts index cbce254..46e01b7 100644 --- a/frontend/src/app/components/pages/pages.component.ts +++ b/frontend/src/app/components/pages/pages.component.ts @@ -38,9 +38,10 @@ export class PagesComponent implements OnDestroy { constructor() { effect(() => { - if (!this.store.loading() && this.store.pages().length === 0) { + const pages = this.store.pages(); + if (!this.store.loading() && pages.length === 0) { this.showWelcome.set(true); - } else if (this.store.pages().length > 0) { + } else if (pages.length > 0) { this.showWelcome.set(false); } }); @@ -89,9 +90,10 @@ export class PagesComponent implements OnDestroy { }); readonly selectedPageIndex = computed(() => { + const pages = this.store.pages(); const page = this.selectedPage(); if (!page) return -1; - return this.store.pages().findIndex((p) => p.id === page.id); + return pages.findIndex((p) => p.id === page.id); }); onSelectPage(index: number): void { diff --git a/frontend/src/app/components/shared/color-picker/color-picker.component.ts b/frontend/src/app/components/shared/color-picker/color-picker.component.ts index 29292c3..e12ee89 100644 --- a/frontend/src/app/components/shared/color-picker/color-picker.component.ts +++ b/frontend/src/app/components/shared/color-picker/color-picker.component.ts @@ -185,9 +185,8 @@ export class ColorPickerComponent { return `hsl(${h}, 70%, 55%)`; } - toCss(c: HslColor): string { - return toCss(c); - } + /** Re-exported so the template can call the utility directly. */ + readonly toCss = toCss; pickHue(h: number): void { this.colorChange.emit({ h: h / 360, s: FIXED_S, l: FIXED_L }); diff --git a/frontend/src/app/components/shared/double-slider/double-slider.component.ts b/frontend/src/app/components/shared/double-slider/double-slider.component.ts index 892e935..f4db1e4 100644 --- a/frontend/src/app/components/shared/double-slider/double-slider.component.ts +++ b/frontend/src/app/components/shared/double-slider/double-slider.component.ts @@ -18,7 +18,7 @@ export interface DoubleSliderRange { * Two-thumb range slider — legacy "double-slider". * Hands an indexed range over an arbitrary values array; emits the * underlying values on each change. Labels magnetically lift as a thumb - * approaches them (rotated -45°), per the legacy. + * approaches them (rotated -30°), per the legacy. */ @Component({ selector: 'lt-double-slider', @@ -228,7 +228,7 @@ export class DoubleSliderComponent { /** * Magnetic label position: returns a CSS `transform` that lifts the label - * upward and rotates -45° as a thumb approaches. + * upward and rotates -30° as a thumb approaches. */ getOffset(index: number): string { const labelIndex = index / Math.max(1, this.drawnLabels().length - 1); diff --git a/frontend/src/app/components/tower/tower.component.ts b/frontend/src/app/components/tower/tower.component.ts index a6422c9..72d59d1 100644 --- a/frontend/src/app/components/tower/tower.component.ts +++ b/frontend/src/app/components/tower/tower.component.ts @@ -695,6 +695,8 @@ export class TowerComponent implements AfterViewInit, OnDestroy { if (this.isFirstRun && animateInitialStack && maxVisibleBlocks === null) { this._visibleBlocks.set([]); this.hiddenBlockCount.set(0); + this.prevDoneIds = allDone.map((b) => b.id); + this.isFirstRun = false; return; } @@ -708,9 +710,6 @@ export class TowerComponent implements AfterViewInit, OnDestroy { newIds.length === 1 && prev.every((id) => ids.includes(id)); // no IDs disappeared - const inRange = (b: Block) => - !range || (b.created_at >= range.from && b.created_at <= range.to); - const styled: StyledBlock[] = []; for (const b of allDone) { if (range && b.created_at < range.from) { @@ -755,7 +754,7 @@ export class TowerComponent implements AfterViewInit, OnDestroy { this.hiddenBlockCount.set(hiddenCount); if (this.isFirstRun && animateInitialStack) { - const initialBlocks = visibleStyled.filter((b) => b._opacity === '1' && inRange(b)); + const initialBlocks = visibleStyled.filter((b) => b._opacity === '1'); if (initialBlocks.length > 0) { this.startDescendAnimation(visibleStyled, initialBlocks); this.prevDoneIds = ids; @@ -767,7 +766,7 @@ export class TowerComponent implements AfterViewInit, OnDestroy { if (grewByOne) { const newId = newIds[0]; const newBlock = visibleStyled.find((b) => b.id === newId); - if (newBlock && inRange(newBlock)) { + if (newBlock) { // Snap newly-added in-range block to start position, then on the next // paint flip it back to rest — that's what makes it visibly fall. this.startDescendAnimation(visibleStyled, [newBlock]); diff --git a/frontend/src/app/components/welcome/welcome.component.ts b/frontend/src/app/components/welcome/welcome.component.ts index eefa8de..b571af2 100644 --- a/frontend/src/app/components/welcome/welcome.component.ts +++ b/frontend/src/app/components/welcome/welcome.component.ts @@ -333,20 +333,20 @@ import { A11yModule } from '@angular/cdk/a11y'; min-width: 0; } - .basic__label { + .basic__label, + .basic__text { font-family: inherit; font-weight: inherit; - color: $text-color; font-size: inherit; line-height: inherit; } + .basic__label { + color: $text-color; + } + .basic__text { - font-family: inherit; - font-weight: inherit; color: rgba($text-color, 0.82); - font-size: inherit; - line-height: inherit; } .actions { diff --git a/frontend/src/app/services/api.service.vitest.ts b/frontend/src/app/services/api.service.vitest.ts index e9cf798..4b9b4e3 100644 --- a/frontend/src/app/services/api.service.vitest.ts +++ b/frontend/src/app/services/api.service.vitest.ts @@ -21,23 +21,6 @@ describe('ApiService', () => { http.verify(); }); - it('gets health', async () => { - const promise = service.health(); - const req = http.expectOne('/api/v1/health'); - expect(req.request.method).toBe('GET'); - req.flush({ status: 'ok' }); - await expect(promise).resolves.toEqual({ status: 'ok' }); - }); - - it('registers a token', async () => { - const promise = service.register('token-1'); - const req = http.expectOne('/api/v1/register'); - expect(req.request.method).toBe('POST'); - expect(req.request.body).toEqual({ token: 'token-1' }); - req.flush({ user_id: 'token-1' }); - await expect(promise).resolves.toEqual({ user_id: 'token-1' }); - }); - it('gets data with a bearer token', async () => { const tree: TreeDto = { pages: [] }; const promise = service.getData('token-1'); diff --git a/frontend/src/app/utils/hash.ts b/frontend/src/app/utils/hash.ts index fffe090..d95c6da 100644 --- a/frontend/src/app/utils/hash.ts +++ b/frontend/src/app/utils/hash.ts @@ -16,5 +16,5 @@ export function hash(s: string): number { h = ((h << 5) - h + s.charCodeAt(i)) | 0; } // Map the signed int32 to [0, 1) — same formula as legacy - return h / (Math.pow(2, 32) - 2) + 0.5; + return h / (2 ** 32 - 2) + 0.5; } diff --git a/frontend/tsconfig.app.json b/frontend/tsconfig.app.json index 264f459..bec29af 100644 --- a/frontend/tsconfig.app.json +++ b/frontend/tsconfig.app.json @@ -10,6 +10,6 @@ "src/**/*.ts" ], "exclude": [ - "src/**/*.spec.ts" + "src/**/*.vitest.ts" ] } diff --git a/frontend/tsconfig.spec.json b/frontend/tsconfig.spec.json index d383706..6883a29 100644 --- a/frontend/tsconfig.spec.json +++ b/frontend/tsconfig.spec.json @@ -10,6 +10,6 @@ }, "include": [ "src/**/*.d.ts", - "src/**/*.spec.ts" + "src/**/*.vitest.ts" ] } diff --git a/frontend/vitest.config.ts b/frontend/vitest.config.ts index af0b622..8dad81b 100644 --- a/frontend/vitest.config.ts +++ b/frontend/vitest.config.ts @@ -12,7 +12,7 @@ export default defineConfig({ test: { globals: true, environment: 'jsdom', - include: ['src/**/*.vitest.ts', 'src/**/*.spec.vitest.ts'], + include: ['src/**/*.vitest.ts'], setupFiles: ['./vitest.setup.ts'], coverage: { provider: 'v8',