StarPunk/docs/design/v1.0.0/delete-route-fix-summary.md

Delete Route Fix - Developer Summary

Date: 2025-11-18 Architect: StarPunk Architect Subagent Developer: Agent-Developer Status: Ready for Implementation

Quick Summary

Problem: Delete route doesn't check if note exists before deletion, always shows "success" message even for nonexistent notes.

Solution: Add existence check (4 lines) before confirmation check, return 404 with error message if note doesn't exist.

Result: Final failing test will pass (406/406 tests ✅)

Exact Implementation

File to Edit

/home/phil/Projects/starpunk/starpunk/routes/admin.py

Function to Modify

delete_note_submit() (currently lines 173-206)

Code to Add

Insert after line 192 (after docstring, before confirmation check):

    # Check if note exists first (per ADR-012)
    existing_note = get_note(id=note_id, load_content=False)
    if not existing_note:
        flash("Note not found", "error")
        return redirect(url_for("admin.dashboard")), 404

Complete Function After Change

@bp.route("/delete/<int:note_id>", methods=["POST"])
@require_auth
def delete_note_submit(note_id: int):
    """
    Handle note deletion

    Deletes a note after confirmation.
    Requires authentication.

    Args:
        note_id: Database ID of note to delete

    Form data:
        confirm: Must be 'yes' to proceed with deletion

    Returns:
        Redirect to dashboard with success/error message

    Decorator: @require_auth
    """
    # Check if note exists first (per ADR-012)  ← NEW
    existing_note = get_note(id=note_id, load_content=False)  ← NEW
    if not existing_note:  ← NEW
        flash("Note not found", "error")  ← NEW
        return redirect(url_for("admin.dashboard")), 404  ← NEW

    # Check for confirmation
    if request.form.get("confirm") != "yes":
        flash("Deletion cancelled", "info")
        return redirect(url_for("admin.dashboard"))

    try:
        delete_note(id=note_id, soft=False)
        flash("Note deleted successfully", "success")
    except ValueError as e:
        flash(f"Error deleting note: {e}", "error")
    except Exception as e:
        flash(f"Unexpected error deleting note: {e}", "error")

    return redirect(url_for("admin.dashboard"))

Why This Fix Works

1. Checks existence FIRST: Before user confirmation, before deletion
2. Returns 404: Proper HTTP status for nonexistent resource (per ADR-012)
3. Flash error message: Test expects "error" or "not found" in response
4. Consistent pattern: Matches update route implementation exactly

Testing

Run Failing Test

uv run pytest tests/test_routes_admin.py::TestAdminDeleteRoutes::test_delete_nonexistent_note_shows_error -v

Expected: PASSED ✅

Run Full Test Suite

uv run pytest

Expected: 406/406 tests passing ✅

Implementation Checklist

• Edit /home/phil/Projects/starpunk/starpunk/routes/admin.py
• Add 4 lines after line 192 (after docstring)
• Verify get_note is already imported (line 15) ✅
• Run failing test - should pass
• Run full test suite - should pass (406/406)
• Document changes in docs/reports/
• Update changelog
• Increment version per docs/standards/versioning-strategy.md
• Follow git protocol per docs/standards/git-branching-strategy.md

Architectural Rationale

Why Not Change delete_note() Function?

The delete_note() function in starpunk/notes.py is intentionally idempotent:

• Deleting nonexistent note returns success (no error)
• This is correct REST behavior for data layer
• Supports retry scenarios and multiple clients

Separation of Concerns:

• Data Layer (notes.py): Idempotent operations
• Route Layer (admin.py): HTTP semantics (404 for missing resources)

Why Check Before Confirmation?

Order matters:

1. ✅ Check existence → error if missing
2. ✅ Check confirmation → cancel if not confirmed
3. ✅ Perform deletion → success or error

Alternative (check after confirmation):

1. Check confirmation
2. Check existence → error if missing

Problem: User confirms deletion, then gets 404 (confusing UX)

Performance Impact

Added overhead: One database query (~0.1ms)

• SELECT query to check existence
• No file I/O (load_content=False)
• Acceptable for single-user CMS

References

• Root Cause Analysis: /home/phil/Projects/starpunk/docs/reports/delete-nonexistent-note-error-analysis.md
• Implementation Spec: /home/phil/Projects/starpunk/docs/reports/delete-route-implementation-spec.md
• ADR-012: HTTP Error Handling Policy (/home/phil/Projects/starpunk/docs/decisions/ADR-012-http-error-handling-policy.md)
• Similar Fix: Update route (lines 148-152 in admin.py)

What Happens After This Fix

Test Results:

• Before: 405/406 tests passing (99.75%)
• After: 406/406 tests passing (100%) ✅

Phase Status:

• Phase 4 (Web Interface): 100% complete ✅
• Ready for Phase 5 (Micropub API)

ADR-012 Compliance:

• All admin routes return 404 for nonexistent resources ✅
• All routes check existence before operations ✅
• Consistent HTTP semantics across application ✅

Developer Notes

1. Use uv: All Python commands need uv run prefix (per CLAUDE.md)
2. Git Protocol: Follow docs/standards/git-branching-strategy.md
3. Documentation: Update docs/reports/, changelog, version
4. This is the last failing test: After this fix, all tests pass!

Quick Reference

What to add: 4 lines (existence check + error handling) Where to add: After line 192, before confirmation check Pattern to follow: Same as update route (line 148-152) Test to verify: test_delete_nonexistent_note_shows_error Expected result: 406/406 tests passing ✅