Add slug rename support to note update API

forgejo_admin commented

2026-02-25 23:20:40 +00:00

Owner

Summary

Add slug rename capability to the PUT /notes/{slug} endpoint so notes can be renamed via the API
Returns 409 Conflict when the target slug is already taken by another note

Changes

src/pal_e_docs/schemas.py: Added slug: str | None = None field to NoteUpdate schema
src/pal_e_docs/routes/notes.py: Added slug rename logic in update_note — checks for conflicts before renaming
tests/test_crud.py: Added 3 tests — rename succeeds, rename conflict returns 409, rename to same slug is a no-op

Test Plan

Tests pass locally (PALDOCS_DATABASE_PATH=:memory: python -m pytest — 14/14 passed)
Rename to new slug works and note is accessible at new slug, 404 at old slug
Rename to existing slug returns 409 Conflict
Rename to same slug (no-op) succeeds with 200

Review Checklist

No secrets committed
No unnecessary file changes
Commit messages are descriptive

plan-2026-02-25-agent-profiles — phase phase-2026-02-25-3.5-claude-custom-cleanup
Forgejo issue #23: Add slug rename support to note update API

## Summary - Add slug rename capability to the `PUT /notes/{slug}` endpoint so notes can be renamed via the API - Returns 409 Conflict when the target slug is already taken by another note ## Changes - `src/pal_e_docs/schemas.py`: Added `slug: str | None = None` field to `NoteUpdate` schema - `src/pal_e_docs/routes/notes.py`: Added slug rename logic in `update_note` — checks for conflicts before renaming - `tests/test_crud.py`: Added 3 tests — rename succeeds, rename conflict returns 409, rename to same slug is a no-op ## Test Plan - [x] Tests pass locally (`PALDOCS_DATABASE_PATH=:memory: python -m pytest` — 14/14 passed) - [x] Rename to new slug works and note is accessible at new slug, 404 at old slug - [x] Rename to existing slug returns 409 Conflict - [x] Rename to same slug (no-op) succeeds with 200 ## Review Checklist - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related Notes - `plan-2026-02-25-agent-profiles` — phase `phase-2026-02-25-3.5-claude-custom-cleanup` - Forgejo issue #23: Add slug rename support to note update API

forgejo_admin added 1 commit

2026-02-25 23:20:40 +00:00

Add slug rename support to note update API (#23 )

ci/woodpecker/pr/woodpecker Pipeline failed

Details

736bc20854

Allow renaming a note's slug via PUT /notes/{slug} by including
a `slug` field in the request body. Returns 409 if the new slug
is already taken. Same-slug is a no-op.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

forgejo_admin commented

2026-02-25 23:26:20 +00:00

Author

Owner

PR #24 Review

Reviewer: QA Agent
Phase: phase-2026-02-25-3.5-claude-custom-cleanup
Plan: plan-2026-02-25-agent-profiles

BLOCKERS

None.

The slug rename logic is correct. The conflict check properly queries for an existing note with the target slug, the body.slug != slug guard prevents false self-conflicts, and the rename is placed before other field updates so it fails fast on 409. All three key test scenarios (rename succeeds, conflict returns 409, same-slug no-op) are covered.

Foreign keys (NoteLink, NoteRevision, NoteTag) all reference note_id (integer PK), not slug, so renames cannot break relationships. No injection risk -- all queries go through SQLAlchemy ORM.

NITS

No slug format validation — The new slug field on NoteUpdate accepts any string (spaces, special characters, empty string). NoteCreate.slug has the same gap, so this is pre-existing and not in scope for this PR. Consider adding a Pydantic field_validator with a regex like ^[a-z0-9]+(-[a-z0-9]+)*$ in a future PR to both NoteCreate and NoteUpdate.
TOCTOU race on conflict check — The check-then-set pattern (query existing → set slug) could race under concurrent requests. The DB UNIQUE constraint on Note.slug would catch it, but it would surface as a 500 IntegrityError instead of a clean 409. Low risk given the single-user deployment, but a try/except IntegrityError → 409 wrapper would be more robust. Not blocking.
Forgejo issue reference in Related Notes — The PR body references "Forgejo issue #23" alongside the plan slug. Current convention is that issues are tracked as pal-e-docs notes, not Forgejo issues. Very minor — this issue likely predates the convention change.

SOP COMPLIANCE

Branch named after issue number (23-slug-rename)
PR body follows template-pr-body (Summary, Changes, Test Plan, Review Checklist, Related Notes)
Related Notes references plan slug (plan-2026-02-25-agent-profiles)
Tests exist and cover acceptance criteria (3 new tests, 14/14 passing)
No secrets, .env files, or credentials committed
No unnecessary file changes (3 files, all directly relevant)
Commit messages are descriptive
No scope creep — focused on slug rename only

VERDICT: APPROVED

Clean, focused PR. The slug rename logic is correct, edge cases are handled, tests are thorough, and SOP compliance is complete. The nits (slug format validation, TOCTOU race) are pre-existing gaps that should be addressed in a separate PR, not blockers for this one.

## PR #24 Review **Reviewer:** QA Agent **Phase:** `phase-2026-02-25-3.5-claude-custom-cleanup` **Plan:** `plan-2026-02-25-agent-profiles` --- ### BLOCKERS None. The slug rename logic is correct. The conflict check properly queries for an existing note with the target slug, the `body.slug != slug` guard prevents false self-conflicts, and the rename is placed before other field updates so it fails fast on 409. All three key test scenarios (rename succeeds, conflict returns 409, same-slug no-op) are covered. Foreign keys (`NoteLink`, `NoteRevision`, `NoteTag`) all reference `note_id` (integer PK), not slug, so renames cannot break relationships. No injection risk -- all queries go through SQLAlchemy ORM. --- ### NITS 1. **No slug format validation** — The new `slug` field on `NoteUpdate` accepts any string (spaces, special characters, empty string). `NoteCreate.slug` has the same gap, so this is pre-existing and not in scope for this PR. Consider adding a Pydantic `field_validator` with a regex like `^[a-z0-9]+(-[a-z0-9]+)*$` in a future PR to both `NoteCreate` and `NoteUpdate`. 2. **TOCTOU race on conflict check** — The check-then-set pattern (`query existing` → `set slug`) could race under concurrent requests. The DB `UNIQUE` constraint on `Note.slug` would catch it, but it would surface as a 500 `IntegrityError` instead of a clean 409. Low risk given the single-user deployment, but a `try/except IntegrityError → 409` wrapper would be more robust. Not blocking. 3. **Forgejo issue reference in Related Notes** — The PR body references "Forgejo issue #23" alongside the plan slug. Current convention is that issues are tracked as pal-e-docs notes, not Forgejo issues. Very minor — this issue likely predates the convention change. --- ### SOP COMPLIANCE - [x] Branch named after issue number (`23-slug-rename`) - [x] PR body follows `template-pr-body` (Summary, Changes, Test Plan, Review Checklist, Related Notes) - [x] Related Notes references plan slug (`plan-2026-02-25-agent-profiles`) - [x] Tests exist and cover acceptance criteria (3 new tests, 14/14 passing) - [x] No secrets, .env files, or credentials committed - [x] No unnecessary file changes (3 files, all directly relevant) - [x] Commit messages are descriptive - [x] No scope creep — focused on slug rename only --- ### VERDICT: APPROVED Clean, focused PR. The slug rename logic is correct, edge cases are handled, tests are thorough, and SOP compliance is complete. The nits (slug format validation, TOCTOU race) are pre-existing gaps that should be addressed in a separate PR, not blockers for this one.