Connector delete after endpoint deletion shows misleading "board has been deleted" overlay #117

Open
opened 2026-04-30 09:54:13 +00:00 by AhmedHanafy725 · 3 comments
Member

Summary

Delete an object that has a connector attached → that connector is cascade-deleted on the server but stays visible in the client. Click the orphan connector and press Delete → the page shows the "This board has been deleted by its owner" overlay even though the board is alive.

Steps to reproduce

  1. Add two objects A and B and connect them with a connector C.
  2. Delete object A.
  3. Connector C still appears on the canvas (orphan) — click it and press Delete.

Expected

Connector C is removed silently. The board stays open.

Actual

The board-deleted overlay is shown, the editor is torn down, and the user has to navigate back. The board is not actually deleted.

Root cause

crates/hero_whiteboard_server/src/migrations/001_core_schema.sql:93 declares from_id and to_id on connectors with ON DELETE CASCADE, so SQLite cascade-deletes the connector row the moment its endpoint object is deleted.

crates/hero_whiteboard_server/src/handlers/connector.rs::delete calls is_connector_board_live(db, id):

"SELECT 1 FROM connectors c JOIN boards b ON b.id = c.board_id
 WHERE c.id = ?1 AND b.deleted_at IS NULL"

That returns false in two unrelated cases:

  • the board is soft-deleted (intended check),
  • the connector row no longer exists (because of the FK cascade above).

The handler then bails with the literal error string "board has been deleted". crates/hero_whiteboard_ui/static/web/js/whiteboard/rpc.js matches that substring and calls WhiteboardSync.markBoardDeleted(), which surfaces the deletion overlay. So a missing-connector delete is wrongly reported as a board deletion.

is_comment_board_live has the same pattern — comment delete on a stale id would behave the same way, though the cascade trigger is different (a comment is only deleted by the user, not by FK cascade).

Fix scope

  • Server (primary): make connector.delete (and comment.delete) idempotent. If the row is missing, return {deleted: 0} with no error. If the board is genuinely deleted (row exists, boards.deleted_at IS NOT NULL), keep returning the board-deleted error.
  • Client (related): when WhiteboardObjects.deleteObject(id) runs, also remove any local connectors whose fromId === id || toId === id and broadcast / persist their deletion. This eliminates the orphan-connector window entirely. Once the server is idempotent, the client cleanup is purely UX polish but worth doing in the same pass since the symptoms are entangled.
## Summary Delete an object that has a connector attached → that connector is cascade-deleted on the server but stays visible in the client. Click the orphan connector and press Delete → the page shows the **"This board has been deleted by its owner"** overlay even though the board is alive. ## Steps to reproduce 1. Add two objects A and B and connect them with a connector C. 2. Delete object A. 3. Connector C still appears on the canvas (orphan) — click it and press Delete. ## Expected Connector C is removed silently. The board stays open. ## Actual The board-deleted overlay is shown, the editor is torn down, and the user has to navigate back. The board is not actually deleted. ## Root cause `crates/hero_whiteboard_server/src/migrations/001_core_schema.sql:93` declares `from_id` and `to_id` on connectors with `ON DELETE CASCADE`, so SQLite cascade-deletes the connector row the moment its endpoint object is deleted. `crates/hero_whiteboard_server/src/handlers/connector.rs::delete` calls `is_connector_board_live(db, id)`: ```rust "SELECT 1 FROM connectors c JOIN boards b ON b.id = c.board_id WHERE c.id = ?1 AND b.deleted_at IS NULL" ``` That returns `false` in two unrelated cases: - the board is soft-deleted (intended check), - **the connector row no longer exists** (because of the FK cascade above). The handler then bails with the literal error string `"board has been deleted"`. `crates/hero_whiteboard_ui/static/web/js/whiteboard/rpc.js` matches that substring and calls `WhiteboardSync.markBoardDeleted()`, which surfaces the deletion overlay. So a missing-connector delete is wrongly reported as a board deletion. `is_comment_board_live` has the same pattern — comment delete on a stale id would behave the same way, though the cascade trigger is different (a comment is only deleted by the user, not by FK cascade). ## Fix scope - **Server (primary):** make `connector.delete` (and `comment.delete`) idempotent. If the row is missing, return `{deleted: 0}` with no error. If the board is genuinely deleted (row exists, `boards.deleted_at IS NOT NULL`), keep returning the board-deleted error. - **Client (related):** when `WhiteboardObjects.deleteObject(id)` runs, also remove any local connectors whose `fromId === id || toId === id` and broadcast / persist their deletion. This eliminates the orphan-connector window entirely. Once the server is idempotent, the client cleanup is purely UX polish but worth doing in the same pass since the symptoms are entangled.
Author
Member

Implementation Spec for Issue #117

Objective

Stop showing the board-deleted overlay when the user deletes a connector whose endpoint object was already removed (and the connector therefore cascade-deleted on the server). The board is alive — the missing connector should be a silent no-op, not a misleading error.

Root cause recap

  • connectors.from_id / to_id use ON DELETE CASCADE, so SQLite cascade-deletes connectors when an endpoint object is deleted.
  • is_connector_board_live returns false both when the board is soft-deleted and when the connector row is gone. The handler bails with the literal string "board has been deleted", and the rpc client matches that substring to trigger the deletion overlay.

Requirements

  • Deleting a connector that no longer exists returns a successful no-op ({ deleted: 0 }) instead of the board-deleted error.
  • Soft-deleted boards still produce the genuine board-deleted error.
  • Same idempotency for comment.delete.
  • Client-side: deleting an object also clears any local connectors that referenced it, so the orphan never appears in the first place.
  • No regression on the rpc.js board-deleted path for actual board deletion.

Files to Modify

  • crates/hero_whiteboard_server/src/db/queries.rs — replace is_connector_board_live and is_comment_board_live with a tri-state status helper, or add a parallel one. Keep the existing helpers if other callers rely on them.
  • crates/hero_whiteboard_server/src/handlers/connector.rsdelete uses the new status helper; "missing" → { deleted: 0 }, "board deleted" → bail, "live" → delete and return rows affected.
  • crates/hero_whiteboard_server/src/handlers/comment.rs — same change to delete.
  • crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.jsdeleteObject(id) walks WhiteboardConnectors.getConnectors() and calls WhiteboardConnectors.deleteConnector(connId) for each connector whose fromId === id || toId === id.

Implementation Plan

Step 1: Server — tri-state status helpers

File: crates/hero_whiteboard_server/src/db/queries.rs

Add an enum and two helpers:

pub enum BoardLinkStatus {
    /// The row no longer exists (already cascaded or never created).
    Missing,
    /// The row exists but its board has been soft-deleted.
    BoardDeleted,
    /// Row and board are both live.
    Live,
}

pub fn connector_board_status(conn: &Connection, id: u64) -> rusqlite::Result<BoardLinkStatus> {
    let mut stmt = conn.prepare(
        "SELECT b.deleted_at FROM connectors c JOIN boards b ON b.id = c.board_id
         WHERE c.id = ?1",
    )?;
    let mut rows = stmt.query(params![id])?;
    Ok(match rows.next()? {
        Some(row) => {
            let deleted_at: Option<String> = row.get(0)?;
            if deleted_at.is_some() { BoardLinkStatus::BoardDeleted } else { BoardLinkStatus::Live }
        }
        None => BoardLinkStatus::Missing,
    })
}

pub fn comment_board_status(conn: &Connection, id: u64) -> rusqlite::Result<BoardLinkStatus> { /* same shape */ }

Keep the existing is_*_board_live functions in place (other callers — connector.create / update, comment.create / update etc. — still want the boolean check; "row missing" for those write paths is genuinely an error, but they look up by board_id not by row id, so they aren't affected by the cascade bug).

Dependencies: none.

Step 2: Server — connector.delete + comment.delete switch on status

Files: crates/hero_whiteboard_server/src/handlers/connector.rs, crates/hero_whiteboard_server/src/handlers/comment.rs

Replace the existing two-line check:

if !queries::is_connector_board_live(&db, id)? {
    anyhow::bail!("board has been deleted");
}
let deleted = queries::delete_connector(&db, id)?;
Ok(serde_json::json!({ "deleted": deleted }))

with:

match queries::connector_board_status(&db, id)? {
    queries::BoardLinkStatus::Missing => Ok(serde_json::json!({ "deleted": 0 })),
    queries::BoardLinkStatus::BoardDeleted => anyhow::bail!("board has been deleted"),
    queries::BoardLinkStatus::Live => {
        let deleted = queries::delete_connector(&db, id)?;
        Ok(serde_json::json!({ "deleted": deleted }))
    }
}

Same change in comment::delete using comment_board_status.

Dependencies: Step 1.

Step 3: Client — cleanup orphan connectors on object delete

File: crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js

In deleteObject(id), before destroying the object, iterate WhiteboardConnectors.getConnectors() and for each connector with fromId === id || toId === id, call WhiteboardConnectors.deleteConnector(connId). The connector module already broadcasts connector.deleted, persists, and removes from the canvas — so we just delegate.

Defensive: wrap in typeof WhiteboardConnectors !== 'undefined' and copy the keys before iterating (since we're mutating the map).

Dependencies: none. Independent of server changes.

Acceptance Criteria

  • Delete an object connected to another → connector vanishes immediately on the local canvas. No orphan visible.
  • In a second tab open on the same board, the connector also vanishes (existing connector.deleted broadcast path handles this).
  • If a stale orphan connector somehow survives (older code path / second tab without the client fix), deleting it on the server returns { deleted: 0 } and does not trigger the board-deleted overlay.
  • Genuine board deletion still triggers the overlay everywhere.
  • Same behavior for comment.delete: deleting a comment whose row was already removed (e.g. cross-tab race) returns { deleted: 0 }.
  • cargo fmt, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace --lib clean.

Notes

  • Why idempotent rather than a different error: REST/DELETE convention is that "delete X that doesn't exist" should succeed as a no-op. It avoids needing to teach the rpc client a new error string and matches what the user expects (the connector is gone whether we just deleted it or it was already gone).
  • Why not also for object.delete: objects.parent_id cascades to NULL, not to row delete, so an object delete won't disappear out from under the client. The rest of the write paths look up by board_id, not by row id, so they don't hit this conflation.
  • Tests: consider adding a small unit test in crates/hero_whiteboard_server/src/handlers/connector.rs::tests that creates a connector, deletes one of its endpoint objects (cascade fires), then calls connector::delete on the now-missing row and asserts { deleted: 0 } and no error. Mirror for comment if straightforward.
## Implementation Spec for Issue #117 ### Objective Stop showing the board-deleted overlay when the user deletes a connector whose endpoint object was already removed (and the connector therefore cascade-deleted on the server). The board is alive — the missing connector should be a silent no-op, not a misleading error. ### Root cause recap - `connectors.from_id` / `to_id` use `ON DELETE CASCADE`, so SQLite cascade-deletes connectors when an endpoint object is deleted. - `is_connector_board_live` returns false both when the board is soft-deleted *and* when the connector row is gone. The handler bails with the literal string `"board has been deleted"`, and the rpc client matches that substring to trigger the deletion overlay. ### Requirements - Deleting a connector that no longer exists returns a successful no-op (`{ deleted: 0 }`) instead of the board-deleted error. - Soft-deleted boards still produce the genuine board-deleted error. - Same idempotency for `comment.delete`. - Client-side: deleting an object also clears any local connectors that referenced it, so the orphan never appears in the first place. - No regression on the rpc.js board-deleted path for actual board deletion. ### Files to Modify - `crates/hero_whiteboard_server/src/db/queries.rs` — replace `is_connector_board_live` and `is_comment_board_live` with a tri-state status helper, or add a parallel one. Keep the existing helpers if other callers rely on them. - `crates/hero_whiteboard_server/src/handlers/connector.rs` — `delete` uses the new status helper; "missing" → `{ deleted: 0 }`, "board deleted" → bail, "live" → delete and return rows affected. - `crates/hero_whiteboard_server/src/handlers/comment.rs` — same change to `delete`. - `crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js` — `deleteObject(id)` walks `WhiteboardConnectors.getConnectors()` and calls `WhiteboardConnectors.deleteConnector(connId)` for each connector whose `fromId === id || toId === id`. ### Implementation Plan #### Step 1: Server — tri-state status helpers File: `crates/hero_whiteboard_server/src/db/queries.rs` Add an enum and two helpers: ```rust pub enum BoardLinkStatus { /// The row no longer exists (already cascaded or never created). Missing, /// The row exists but its board has been soft-deleted. BoardDeleted, /// Row and board are both live. Live, } pub fn connector_board_status(conn: &Connection, id: u64) -> rusqlite::Result<BoardLinkStatus> { let mut stmt = conn.prepare( "SELECT b.deleted_at FROM connectors c JOIN boards b ON b.id = c.board_id WHERE c.id = ?1", )?; let mut rows = stmt.query(params![id])?; Ok(match rows.next()? { Some(row) => { let deleted_at: Option<String> = row.get(0)?; if deleted_at.is_some() { BoardLinkStatus::BoardDeleted } else { BoardLinkStatus::Live } } None => BoardLinkStatus::Missing, }) } pub fn comment_board_status(conn: &Connection, id: u64) -> rusqlite::Result<BoardLinkStatus> { /* same shape */ } ``` Keep the existing `is_*_board_live` functions in place (other callers — `connector.create` / `update`, `comment.create` / `update` etc. — still want the boolean check; "row missing" for those write paths is genuinely an error, but they look up by `board_id` not by row id, so they aren't affected by the cascade bug). Dependencies: none. #### Step 2: Server — connector.delete + comment.delete switch on status Files: `crates/hero_whiteboard_server/src/handlers/connector.rs`, `crates/hero_whiteboard_server/src/handlers/comment.rs` Replace the existing two-line check: ```rust if !queries::is_connector_board_live(&db, id)? { anyhow::bail!("board has been deleted"); } let deleted = queries::delete_connector(&db, id)?; Ok(serde_json::json!({ "deleted": deleted })) ``` with: ```rust match queries::connector_board_status(&db, id)? { queries::BoardLinkStatus::Missing => Ok(serde_json::json!({ "deleted": 0 })), queries::BoardLinkStatus::BoardDeleted => anyhow::bail!("board has been deleted"), queries::BoardLinkStatus::Live => { let deleted = queries::delete_connector(&db, id)?; Ok(serde_json::json!({ "deleted": deleted })) } } ``` Same change in `comment::delete` using `comment_board_status`. Dependencies: Step 1. #### Step 3: Client — cleanup orphan connectors on object delete File: `crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js` In `deleteObject(id)`, before destroying the object, iterate `WhiteboardConnectors.getConnectors()` and for each connector with `fromId === id || toId === id`, call `WhiteboardConnectors.deleteConnector(connId)`. The connector module already broadcasts `connector.deleted`, persists, and removes from the canvas — so we just delegate. Defensive: wrap in `typeof WhiteboardConnectors !== 'undefined'` and copy the keys before iterating (since we're mutating the map). Dependencies: none. Independent of server changes. ### Acceptance Criteria - [ ] Delete an object connected to another → connector vanishes immediately on the local canvas. No orphan visible. - [ ] In a second tab open on the same board, the connector also vanishes (existing `connector.deleted` broadcast path handles this). - [ ] If a stale orphan connector somehow survives (older code path / second tab without the client fix), deleting it on the server returns `{ deleted: 0 }` and does **not** trigger the board-deleted overlay. - [ ] Genuine board deletion still triggers the overlay everywhere. - [ ] Same behavior for comment.delete: deleting a comment whose row was already removed (e.g. cross-tab race) returns `{ deleted: 0 }`. - [ ] `cargo fmt`, `cargo clippy --workspace --all-targets -- -D warnings`, `cargo test --workspace --lib` clean. ### Notes - **Why idempotent rather than a different error:** REST/DELETE convention is that "delete X that doesn't exist" should succeed as a no-op. It avoids needing to teach the rpc client a new error string and matches what the user expects (the connector is gone whether we just deleted it or it was already gone). - **Why not also for object.delete:** `objects.parent_id` cascades to NULL, not to row delete, so an object delete won't disappear out from under the client. The rest of the write paths look up by `board_id`, not by row id, so they don't hit this conflation. - **Tests:** consider adding a small unit test in `crates/hero_whiteboard_server/src/handlers/connector.rs::tests` that creates a connector, deletes one of its endpoint objects (cascade fires), then calls `connector::delete` on the now-missing row and asserts `{ deleted: 0 }` and no error. Mirror for comment if straightforward.
Author
Member

Test Results

  • cargo fmt --all -- --check — clean
  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • cargo check -p hero_whiteboard_server — clean
  • cargo test --workspace --lib — 0 failed
  • node --check objects.js — clean
## Test Results - `cargo fmt --all -- --check` — clean - `cargo clippy --workspace --all-targets -- -D warnings` — clean - `cargo check -p hero_whiteboard_server` — clean - `cargo test --workspace --lib` — 0 failed - `node --check objects.js` — clean
Author
Member

Implementation Summary

Server: connector.delete and comment.delete are now idempotent for missing rows. Client: deleteObject cleans up orphan connectors before destroying the object.

crates/hero_whiteboard_server/src/db/queries.rs

Added a tri-state enum and two helpers:

pub enum BoardLinkStatus { Missing, BoardDeleted, Live }

pub fn connector_board_status(conn, id) -> Result<BoardLinkStatus> { ... }
pub fn comment_board_status(conn, id)   -> Result<BoardLinkStatus> { ... }

Each runs SELECT b.deleted_at FROM <table> JOIN boards ... and inspects the result: no row → Missing; row with deleted_at IS NOT NULL → BoardDeleted; otherwise Live. The original is_*_board_live helpers are kept for other callers.

crates/hero_whiteboard_server/src/handlers/connector.rs and comment.rs

Both delete handlers replaced their two-line is-live check with a match on the new status:

  • MissingOk({ "deleted": 0 })
  • BoardDeletedbail!("board has been deleted")
  • Live → run the actual delete

So a connector that was cascade-deleted when its endpoint object was removed (FK ON DELETE CASCADE from connectors.from_id / to_id) is now a successful no-op rather than an error that the rpc client maps to the deletion overlay.

crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js

deleteObject(id) now walks WhiteboardConnectors.getConnectors() first and calls WhiteboardConnectors.deleteConnector(connId) for each connector with fromId === id || toId === id. The connector module already broadcasts connector.deleted and persists to server, so we just delegate. Defensive: copy the keys before iterating since deletion mutates the map.

Files Changed

  • crates/hero_whiteboard_server/src/db/queries.rs+44 / 0
  • crates/hero_whiteboard_server/src/handlers/connector.rs+12 / -3
  • crates/hero_whiteboard_server/src/handlers/comment.rs+11 / -3
  • crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js+15 / 0

Test Results

  • cargo fmt --all -- --check — clean
  • cargo clippy --workspace --all-targets -- -D warnings — clean
  • cargo check -p hero_whiteboard_server — clean
  • cargo test --workspace --lib — 0 failed
  • node --check objects.js — clean

Manual smoke

  1. Add two objects, connect them with a connector. Delete one of the objects → the connector vanishes locally and from any other tab.
  2. Repro of the original bug (older clients without the cleanup): in a tab without the JS fix, an orphan connector survives the object delete. Deleting it now returns silently — the board stays open, no overlay.
  3. Genuine board deletion still triggers the overlay everywhere (the BoardDeleted arm is unchanged).
  4. Cross-tab race: if a comment is deleted in one tab and the second tab tries to delete the same row, the second delete returns { deleted: 0 } cleanly.

Notes

  • Why idempotent rather than a new error: REST/DELETE convention is "delete X that does not exist" succeeds as a no-op. Avoids teaching the rpc client a new error string and matches user expectation (gone is gone).
  • object.delete is unaffected: objects.parent_id cascades to NULL rather than a row delete, so an object never disappears out from under a sibling write path.
  • is_*_board_live retained: write paths (*.create, *.update) still use the boolean check; for those, "row missing" is genuinely an error.
## Implementation Summary Server: `connector.delete` and `comment.delete` are now idempotent for missing rows. Client: `deleteObject` cleans up orphan connectors before destroying the object. ### `crates/hero_whiteboard_server/src/db/queries.rs` Added a tri-state enum and two helpers: ```rust pub enum BoardLinkStatus { Missing, BoardDeleted, Live } pub fn connector_board_status(conn, id) -> Result<BoardLinkStatus> { ... } pub fn comment_board_status(conn, id) -> Result<BoardLinkStatus> { ... } ``` Each runs `SELECT b.deleted_at FROM <table> JOIN boards ...` and inspects the result: no row → Missing; row with `deleted_at IS NOT NULL` → BoardDeleted; otherwise Live. The original `is_*_board_live` helpers are kept for other callers. ### `crates/hero_whiteboard_server/src/handlers/connector.rs` and `comment.rs` Both `delete` handlers replaced their two-line is-live check with a `match` on the new status: - `Missing` → `Ok({ "deleted": 0 })` - `BoardDeleted` → `bail!("board has been deleted")` - `Live` → run the actual delete So a connector that was cascade-deleted when its endpoint object was removed (FK `ON DELETE CASCADE` from `connectors.from_id` / `to_id`) is now a successful no-op rather than an error that the rpc client maps to the deletion overlay. ### `crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js` `deleteObject(id)` now walks `WhiteboardConnectors.getConnectors()` first and calls `WhiteboardConnectors.deleteConnector(connId)` for each connector with `fromId === id || toId === id`. The connector module already broadcasts `connector.deleted` and persists to server, so we just delegate. Defensive: copy the keys before iterating since deletion mutates the map. ### Files Changed - `crates/hero_whiteboard_server/src/db/queries.rs` — `+44 / 0` - `crates/hero_whiteboard_server/src/handlers/connector.rs` — `+12 / -3` - `crates/hero_whiteboard_server/src/handlers/comment.rs` — `+11 / -3` - `crates/hero_whiteboard_ui/static/web/js/whiteboard/objects.js` — `+15 / 0` ### Test Results - `cargo fmt --all -- --check` — clean - `cargo clippy --workspace --all-targets -- -D warnings` — clean - `cargo check -p hero_whiteboard_server` — clean - `cargo test --workspace --lib` — 0 failed - `node --check objects.js` — clean ### Manual smoke 1. Add two objects, connect them with a connector. Delete one of the objects → the connector vanishes locally and from any other tab. 2. Repro of the original bug (older clients without the cleanup): in a tab without the JS fix, an orphan connector survives the object delete. Deleting it now returns silently — the board stays open, no overlay. 3. Genuine board deletion still triggers the overlay everywhere (the `BoardDeleted` arm is unchanged). 4. Cross-tab race: if a comment is deleted in one tab and the second tab tries to delete the same row, the second delete returns `{ deleted: 0 }` cleanly. ### Notes - **Why idempotent rather than a new error:** REST/DELETE convention is "delete X that does not exist" succeeds as a no-op. Avoids teaching the rpc client a new error string and matches user expectation (gone is gone). - **`object.delete` is unaffected**: `objects.parent_id` cascades to NULL rather than a row delete, so an object never disappears out from under a sibling write path. - **`is_*_board_live` retained**: write paths (`*.create`, `*.update`) still use the boolean check; for those, "row missing" is genuinely an error.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lhumina_code/hero_whiteboard#117
No description provided.