gsadmin/pad
1
0
Fork 0
mirror of https://github.com/PerpetualSoftware/pad.git synced 2026-09-21 01:53:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(store): hide item_links pointing to soft-deleted items (BUG-734)

Browse Source 
Item-link queries that JOIN against `items` now also filter on
`deleted_at IS NULL` for both source and target. This prevents
`pad item related`, the lineage breadcrumb, and dashboard enrichment
from surfacing dangling endpoints when one side has been archived.

Affected queries in internal/store/items.go:
- GetItemLinks  (powers `pad item related`, lineage, dashboard)
- GetItemLink   (singular; fixed for consistency)
- GetParentForItem (breadcrumb / lineage; archived parent reads as none)

Other item_links queries already filtered on deleted_at; export.go
deliberately keeps all rows for backup correctness — left unchanged.

The link rows themselves are preserved on disk, so restoring a
soft-deleted item resurrects its relationships automatically.

Tests:
- TestItemLinks_HidesSoftDeletedEndpoints — delete + restore round-trip
  on both source-side and target-side
- TestGetParentForItem_HidesSoftDeletedParent — parent breadcrumb path

Manually verified: PLAN + TASK with `implements` link, soft-delete the
TASK, `pad item related <PLAN>` correctly returns no implementers.
This commit is contained in:
xarmian
2026-04-27 13:44:33 +00:00
parent 96b3f68b5a
commit 1eb7fff757
2 changed files with 131 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/store/items.go
+15 -6
View File
@@ -997,14 +997,15 @@ func (s *Store) getItemLink(id string) (*models.ItemLink, error) {
srcStatus := s.dialect.JSONExtractText("s.fields", "status")
tgtStatus := s.dialect.JSONExtractText("t.fields", "status")
// Filter out links pointing to or from soft-deleted items — see BUG-734.
err := s.db.QueryRow(s.q(fmt.Sprintf(`
SELECT l.id, l.workspace_id, l.source_id, l.target_id, l.link_type, l.created_by, l.created_at,
s.title, t.title, s.slug, t.slug, sc.slug, tc.slug, sc.prefix, tc.prefix,
s.item_number, t.item_number,
%s, %s
FROM item_links l
JOIN items s ON s.id = l.source_id
JOIN items t ON t.id = l.target_id
JOIN items s ON s.id = l.source_id AND s.deleted_at IS NULL
JOIN items t ON t.id = l.target_id AND t.deleted_at IS NULL
JOIN collections sc ON sc.id = s.collection_id
JOIN collections tc ON tc.id = t.collection_id
WHERE l.id = ?
@@ -1040,6 +1041,12 @@ func (s *Store) getItemLink(id string) (*models.ItemLink, error) {
return &link, nil
}
// GetItemLinks returns links where the given item is either source or target.
// Links pointing to or from soft-deleted items are filtered out so callers (e.g.
// `pad item related`, the lineage panel, the dashboard enrichment pass) don't
// surface dangling endpoints. The link rows themselves are preserved on disk —
// restoring a soft-deleted item resurrects its relationships automatically. See
// BUG-734.
func (s *Store) GetItemLinks(itemID string) ([]models.ItemLink, error) {
srcStatusExpr := s.dialect.JSONExtractText("s.fields", "status")
tgtStatusExpr := s.dialect.JSONExtractText("t.fields", "status")
@@ -1049,8 +1056,8 @@ func (s *Store) GetItemLinks(itemID string) ([]models.ItemLink, error) {
s.item_number, t.item_number,
%s, %s
FROM item_links l
JOIN items s ON s.id = l.source_id
JOIN items t ON t.id = l.target_id
JOIN items s ON s.id = l.source_id AND s.deleted_at IS NULL
JOIN items t ON t.id = l.target_id AND t.deleted_at IS NULL
JOIN collections sc ON sc.id = s.collection_id
JOIN collections tc ON tc.id = t.collection_id
WHERE l.source_id = ? OR l.target_id = ?
@@ -1213,6 +1220,8 @@ func (s *Store) ClearParentLink(itemID string) error {
}
// GetParentForItem returns the parent link for an item, or nil if it has no parent.
// A parent link pointing to a soft-deleted item is treated as no parent — the
// breadcrumb / lineage UI shouldn't show a deleted ancestor. See BUG-734.
func (s *Store) GetParentForItem(itemID string) (*models.ItemLink, error) {
sStatusExpr := s.dialect.JSONExtractText("s.fields", "status")
tStatusExpr := s.dialect.JSONExtractText("t.fields", "status")
@@ -1222,8 +1231,8 @@ func (s *Store) GetParentForItem(itemID string) (*models.ItemLink, error) {
s.item_number, t.item_number,
%s, %s
FROM item_links l
JOIN items s ON s.id = l.source_id
JOIN items t ON t.id = l.target_id
JOIN items s ON s.id = l.source_id AND s.deleted_at IS NULL
JOIN items t ON t.id = l.target_id AND t.deleted_at IS NULL
JOIN collections sc ON sc.id = s.collection_id
JOIN collections tc ON tc.id = t.collection_id
WHERE l.source_id = ? AND l.link_type IN (%s)
internal/store/items_test.go
+116
View File
@@ -954,6 +954,122 @@ func TestItemLinks(t *testing.T) {
}
}
// TestItemLinks_HidesSoftDeletedEndpoints exercises BUG-734: when an item that
// is the source or target of a link gets soft-deleted, GetItemLinks should not
// surface the link from the surviving endpoint's perspective. Restoring the
// deleted item should resurrect the link automatically — the row is preserved
// on disk; only the query layer filters it.
func TestItemLinks_HidesSoftDeletedEndpoints(t *testing.T) {
s := testStore(t)
ws := createTestWorkspace(t, s, "Test")
col := createTestCollection(t, s, ws.ID, "Tasks")
plan := createTestItem(t, s, ws.ID, col.ID, "Plan", "")
implementer := createTestItem(t, s, ws.ID, col.ID, "Implementer task", "")
// implementer --implements--> plan
if _, err := s.CreateItemLink(ws.ID, models.ItemLinkCreate{
TargetID: plan.ID,
LinkType: "implements",
}, implementer.ID); err != nil {
t.Fatalf("CreateItemLink: %v", err)
}
// Sanity: link visible from both endpoints.
if links, _ := s.GetItemLinks(plan.ID); len(links) != 1 {
t.Fatalf("expected 1 link from plan side before delete, got %d", len(links))
}
if links, _ := s.GetItemLinks(implementer.ID); len(links) != 1 {
t.Fatalf("expected 1 link from implementer side before delete, got %d", len(links))
}
// Soft-delete the implementer (the BUG-734 scenario: source side gone).
if err := s.DeleteItem(implementer.ID); err != nil {
t.Fatalf("DeleteItem: %v", err)
}
// From the plan's perspective, the dangling implementer must not surface.
links, err := s.GetItemLinks(plan.ID)
if err != nil {
t.Fatalf("GetItemLinks after delete: %v", err)
}
if len(links) != 0 {
t.Errorf("expected 0 links from plan side after implementer deleted, got %d (orphan leak — BUG-734)", len(links))
}
// Restore the implementer — the link row was never deleted, so the
// relationship should reappear automatically.
if _, err := s.RestoreItem(implementer.ID); err != nil {
t.Fatalf("RestoreItem: %v", err)
}
links, err = s.GetItemLinks(plan.ID)
if err != nil {
t.Fatalf("GetItemLinks after restore: %v", err)
}
if len(links) != 1 {
t.Errorf("expected 1 link from plan side after restore, got %d (link should be preserved across soft-delete/restore)", len(links))
}
// Now soft-delete the plan side instead (target side gone) and verify the
// implementer's view also drops the dangling link.
if err := s.DeleteItem(plan.ID); err != nil {
t.Fatalf("DeleteItem plan: %v", err)
}
links, err = s.GetItemLinks(implementer.ID)
if err != nil {
t.Fatalf("GetItemLinks after target delete: %v", err)
}
if len(links) != 0 {
t.Errorf("expected 0 links from implementer side after plan deleted, got %d (target-side orphan leak)", len(links))
}
}
// TestGetParentForItem_HidesSoftDeletedParent ensures lineage / breadcrumb
// queries don't surface a soft-deleted ancestor. See BUG-734.
func TestGetParentForItem_HidesSoftDeletedParent(t *testing.T) {
s := testStore(t)
ws := createTestWorkspace(t, s, "Test")
col := createTestCollection(t, s, ws.ID, "Tasks")
parent := createTestItem(t, s, ws.ID, col.ID, "Parent", "")
child := createTestItem(t, s, ws.ID, col.ID, "Child", "")
if _, err := s.SetParentLink(ws.ID, child.ID, parent.ID, "user"); err != nil {
t.Fatalf("SetParentLink: %v", err)
}
// Before delete: parent visible.
if link, err := s.GetParentForItem(child.ID); err != nil {
t.Fatalf("GetParentForItem: %v", err)
} else if link == nil {
t.Fatal("expected parent link before delete, got nil")
}
// Soft-delete parent.
if err := s.DeleteItem(parent.ID); err != nil {
t.Fatalf("DeleteItem: %v", err)
}
// After delete: must read as no parent (don't render a deleted breadcrumb).
link, err := s.GetParentForItem(child.ID)
if err != nil {
t.Fatalf("GetParentForItem after delete: %v", err)
}
if link != nil {
t.Errorf("expected nil parent link after soft-delete, got %+v", link)
}
// After restore: parent visible again.
if _, err := s.RestoreItem(parent.ID); err != nil {
t.Fatalf("RestoreItem: %v", err)
}
if link, err := s.GetParentForItem(child.ID); err != nil {
t.Fatalf("GetParentForItem after restore: %v", err)
} else if link == nil {
t.Error("expected parent link to reappear after restore")
}
}
func TestItemLinkDefaultType(t *testing.T) {
s := testStore(t)
ws := createTestWorkspace(t, s, "Test")