mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-21 10:03:29 +00:00
fix: hide item_links pointing to soft-deleted items (BUG-734) (#259)
* fix(store): hide item_links pointing to soft-deleted items (BUG-734)
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.
* fix(store): address Codex review findings on PR #259 (BUG-734)
Three follow-ups from Codex's review of the soft-delete filter on item-link
queries:
1. MEDIUM — GetParentMap now JOINs items on both sides and filters on
deleted_at IS NULL. handlers_dashboard.go uses this map directly to
detect orphaned tasks (items not present in the map are flagged), so
without the filter a task whose parent had been soft-deleted would
silently fail to appear as orphaned.
2. LOW — Revert the deleted_at filter on getItemLink (lowercase, private).
Its only caller is the post-insert readback in CreateItemLink, which
means filtering buys nothing user-facing and introduces a delete-race
window where a successful INSERT returns nil. SetParentLink's readback
was switched from GetItemLinks to getItemLink for the same reason.
User-facing surfaces still go through GetItemLinks (plural) and
GetParentForItem, both of which retain the filter.
3. LOW — Add an explicit comment in export.go documenting that item_links
are exported in full (including links to soft-deleted items), and why
that intentionally diverges from the user-facing query behavior.
Tests: TestGetParentMap_ExcludesSoftDeletedEndpoints exercises the
dashboard regression path on both source-side and target-side soft-delete,
plus the restore round-trip.
* fix(store): reject soft-deleted parent in ListItems UUID parent filter (BUG-734)
Codex review on 288283b flagged that ListItems(parent=<UUID>) at items.go:534
runs an EXISTS subquery against item_links without checking whether the
target parent is soft-deleted. Slug/ref input rejects deleted parents
upstream via GetItem/GetItemBySlug, but raw-UUID input bypasses that path
and would still return active children of an archived parent.
Fix: JOIN items into the EXISTS subquery and require deleted_at IS NULL on
the parent.
Test: TestListItems_ParentFilter_RespectsSoftDeletedParent — covers the
delete + restore round-trip on the parent.
* fix(store): apply parent-filter in FTS path so search+parent enforces deleted-parent rejection (BUG-734)
Codex's 3rd review pass on PR #259 caught that listItemsFTS does not
re-apply ParentLinkID. Combining `parent=<UUID>&search=<q>` therefore
silently dropped the parent constraint — and, by extension, the
deleted-parent rejection introduced earlier in this PR.
Fix: replay the same EXISTS-with-deleted_at-IS-NULL predicate in the FTS
branch. Test: TestListItems_ParentFilter_FTS_RespectsSoftDeletedParent
covers the delete + restore round-trip on the search path.
The wider FTS filter-bypass (Tags, AssignedUserID, AgentRoleID, Fields,
ParentID are all silently dropped when search is set) is pre-existing
behavior outside BUG-734's scope; tracked as BUG-812.
This commit is contained in:
@@ -97,7 +97,13 @@ func (s *Store) ExportWorkspace(slug string) (*models.WorkspaceExport, error) {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// Item links
|
||||
// Item links — exported in full, including links whose source or target item
|
||||
// is soft-deleted. This is intentional and differs from user-facing reads
|
||||
// (GetItemLinks/GetParentForItem/GetParentMap, which all filter on
|
||||
// items.deleted_at IS NULL — see BUG-734). Backups need to round-trip the
|
||||
// raw graph so that re-importing into a workspace where the deleted items
|
||||
// are restored preserves the original relationships. The import path
|
||||
// already silently skips links whose endpoints are missing entirely.
|
||||
linkRows, err := s.db.Query(s.q(`
|
||||
SELECT id, source_id, target_id, link_type, created_by, created_at
|
||||
FROM item_links WHERE workspace_id = ?
|
||||
|
||||
+48
-19
@@ -529,9 +529,12 @@ func (s *Store) ListItems(workspaceID string, params models.ItemListParams) ([]m
|
||||
args = append(args, params.AgentRoleID, params.AgentRoleID)
|
||||
}
|
||||
|
||||
// Parent link filter via item_links
|
||||
// Parent link filter via item_links. Joins items so we ignore links pointing
|
||||
// to a soft-deleted parent — slug/ref filtering already rejects deleted
|
||||
// parents upstream, but raw-UUID input bypasses that path. See BUG-734 /
|
||||
// Codex review on PR #259.
|
||||
if params.ParentLinkID != "" {
|
||||
query += " AND EXISTS (SELECT 1 FROM item_links il WHERE il.source_id = i.id AND il.link_type = 'parent' AND il.target_id = ?)"
|
||||
query += " AND EXISTS (SELECT 1 FROM item_links il JOIN items p ON p.id = il.target_id AND p.deleted_at IS NULL WHERE il.source_id = i.id AND il.link_type = 'parent' AND il.target_id = ?)"
|
||||
args = append(args, params.ParentLinkID)
|
||||
}
|
||||
|
||||
@@ -634,6 +637,17 @@ func (s *Store) listItemsFTS(workspaceID string, params models.ItemListParams) (
|
||||
args = append(args, params.CollectionSlug)
|
||||
}
|
||||
|
||||
// Parent link filter — must mirror the non-FTS path so combining
|
||||
// `parent=<UUID>&search=<q>` doesn't silently drop the parent constraint
|
||||
// (and, by extension, the soft-deleted-parent rejection from BUG-734).
|
||||
// See Codex review on PR #259. Note: other filter kinds (tags, assignee,
|
||||
// role, fields, ParentID) are also currently ignored in the FTS path —
|
||||
// pre-existing behavior outside BUG-734's scope; tracked separately.
|
||||
if params.ParentLinkID != "" {
|
||||
query += " AND EXISTS (SELECT 1 FROM item_links il JOIN items p ON p.id = il.target_id AND p.deleted_at IS NULL WHERE il.source_id = i.id AND il.link_type = 'parent' AND il.target_id = ?)"
|
||||
args = append(args, params.ParentLinkID)
|
||||
}
|
||||
|
||||
if len(params.CollectionIDs) > 0 && len(params.ItemIDs) > 0 {
|
||||
collPlaceholders := make([]string, len(params.CollectionIDs))
|
||||
for i, id := range params.CollectionIDs {
|
||||
@@ -987,6 +1001,13 @@ func (s *Store) CreateItemLink(workspaceID string, input models.ItemLinkCreate,
|
||||
return s.getItemLink(id)
|
||||
}
|
||||
|
||||
// getItemLink is the unfiltered post-insert readback used by CreateItemLink to
|
||||
// hydrate the freshly-inserted row with collection/source/target metadata. It
|
||||
// intentionally does NOT filter on items.deleted_at IS NULL: the only caller
|
||||
// is the immediate readback after INSERT, and a delete race against either
|
||||
// endpoint would otherwise cause the just-successful insert to return nil
|
||||
// (Codex review on PR #259). User-facing surfaces all read links via
|
||||
// GetItemLinks (plural) or GetParentForItem, both of which DO filter.
|
||||
func (s *Store) getItemLink(id string) (*models.ItemLink, error) {
|
||||
var link models.ItemLink
|
||||
var createdAt string
|
||||
@@ -1040,6 +1061,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 +1076,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 = ?
|
||||
@@ -1165,17 +1192,10 @@ func (s *Store) SetParentLink(workspaceID, itemID, parentID, createdBy string) (
|
||||
return nil, fmt.Errorf("commit parent link: %w", err)
|
||||
}
|
||||
|
||||
// Return the full link with enriched fields
|
||||
links, err := s.GetItemLinks(itemID)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
for _, link := range links {
|
||||
if link.ID == id {
|
||||
return &link, nil
|
||||
}
|
||||
}
|
||||
return nil, fmt.Errorf("parent link created but not found")
|
||||
// Return the full link with enriched fields. Use the unfiltered readback
|
||||
// helper so that a delete race against either endpoint between commit and
|
||||
// readback doesn't cause the successful insert to surface as nil.
|
||||
return s.getItemLink(id)
|
||||
}
|
||||
|
||||
// checkParentCycle walks the ancestor chain from parentID and returns an error
|
||||
@@ -1213,6 +1233,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 +1244,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)
|
||||
@@ -1272,10 +1294,17 @@ func (s *Store) GetParentForItem(itemID string) (*models.ItemLink, error) {
|
||||
|
||||
// GetParentMap returns a map of item ID -> parent item ID for all parent links
|
||||
// in a workspace. Used for efficient batch lookups (e.g., dashboard, list enrichment).
|
||||
//
|
||||
// Links whose source or target item is soft-deleted are excluded so that
|
||||
// dashboard orphan-detection (handlers_dashboard.go) and similar enrichment
|
||||
// passes don't treat a task whose parent has been archived as still parented.
|
||||
// See BUG-734.
|
||||
func (s *Store) GetParentMap(workspaceID string) (map[string]string, error) {
|
||||
rows, err := s.db.Query(s.q(fmt.Sprintf(`
|
||||
SELECT source_id, target_id FROM item_links
|
||||
WHERE workspace_id = ? AND link_type IN (%s)
|
||||
SELECT il.source_id, il.target_id FROM item_links il
|
||||
JOIN items s ON s.id = il.source_id AND s.deleted_at IS NULL
|
||||
JOIN items t ON t.id = il.target_id AND t.deleted_at IS NULL
|
||||
WHERE il.workspace_id = ? AND il.link_type IN (%s)
|
||||
`, childLinkTypeSQL())), workspaceID)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("get parent map: %w", err)
|
||||
|
||||
@@ -954,6 +954,297 @@ 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")
|
||||
}
|
||||
}
|
||||
|
||||
// TestGetParentMap_ExcludesSoftDeletedEndpoints covers the dashboard
|
||||
// orphan-detection path: a task whose parent has been soft-deleted should
|
||||
// NOT appear in GetParentMap, so handlers_dashboard.go correctly flags the
|
||||
// task as orphaned. See BUG-734 / Codex review on PR #259.
|
||||
func TestGetParentMap_ExcludesSoftDeletedEndpoints(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)
|
||||
}
|
||||
|
||||
// Sanity: child→parent mapping present.
|
||||
m, err := s.GetParentMap(ws.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetParentMap: %v", err)
|
||||
}
|
||||
if m[child.ID] != parent.ID {
|
||||
t.Fatalf("expected parent map %s→%s, got %s→%s", child.ID, parent.ID, child.ID, m[child.ID])
|
||||
}
|
||||
|
||||
// Soft-delete the parent. The child must now look "parentless" so the
|
||||
// dashboard orphan detector flags it.
|
||||
if err := s.DeleteItem(parent.ID); err != nil {
|
||||
t.Fatalf("DeleteItem: %v", err)
|
||||
}
|
||||
m, err = s.GetParentMap(ws.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetParentMap after parent delete: %v", err)
|
||||
}
|
||||
if _, hasEntry := m[child.ID]; hasEntry {
|
||||
t.Errorf("expected child to drop from parent map after parent soft-deleted (orphan-detection regression)")
|
||||
}
|
||||
|
||||
// Restoring the parent should bring the mapping back.
|
||||
if _, err := s.RestoreItem(parent.ID); err != nil {
|
||||
t.Fatalf("RestoreItem: %v", err)
|
||||
}
|
||||
m, err = s.GetParentMap(ws.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetParentMap after parent restore: %v", err)
|
||||
}
|
||||
if m[child.ID] != parent.ID {
|
||||
t.Errorf("expected parent map to be restored to %s→%s, got %s→%s", child.ID, parent.ID, child.ID, m[child.ID])
|
||||
}
|
||||
|
||||
// Soft-deleting the child side should also drop the entry.
|
||||
if err := s.DeleteItem(child.ID); err != nil {
|
||||
t.Fatalf("DeleteItem child: %v", err)
|
||||
}
|
||||
m, err = s.GetParentMap(ws.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("GetParentMap after child delete: %v", err)
|
||||
}
|
||||
if _, hasEntry := m[child.ID]; hasEntry {
|
||||
t.Errorf("expected child to drop from parent map after the child itself was soft-deleted")
|
||||
}
|
||||
}
|
||||
|
||||
// TestListItems_ParentFilter_FTS_RespectsSoftDeletedParent covers the
|
||||
// `parent=<UUID>&search=<q>` combination. The search path routes through
|
||||
// listItemsFTS, which the non-FTS parent filter doesn't touch; the FTS
|
||||
// path needs to enforce the same deleted-parent rejection. See BUG-734 /
|
||||
// Codex review on PR #259 (3rd pass).
|
||||
func TestListItems_ParentFilter_FTS_RespectsSoftDeletedParent(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", "")
|
||||
// Use a distinctive title so the FTS match is unambiguous.
|
||||
child := createTestItem(t, s, ws.ID, col.ID, "Distinctivekeyword child", "")
|
||||
if _, err := s.SetParentLink(ws.ID, child.ID, parent.ID, "user"); err != nil {
|
||||
t.Fatalf("SetParentLink: %v", err)
|
||||
}
|
||||
|
||||
// Sanity: search + parent finds the child while parent is live.
|
||||
items, err := s.ListItems(ws.ID, models.ItemListParams{
|
||||
ParentLinkID: parent.ID,
|
||||
Search: "Distinctivekeyword",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("ListItems (FTS+parent): %v", err)
|
||||
}
|
||||
if len(items) != 1 || items[0].ID != child.ID {
|
||||
t.Fatalf("expected to find 1 child via FTS+parent before delete, got %d", len(items))
|
||||
}
|
||||
|
||||
// Soft-delete the parent. The FTS path must also reject the now-deleted
|
||||
// parent, otherwise `?parent=<deleted-uuid>&search=foo` continues to leak
|
||||
// active children of an archived parent (the gap Codex flagged).
|
||||
if err := s.DeleteItem(parent.ID); err != nil {
|
||||
t.Fatalf("DeleteItem: %v", err)
|
||||
}
|
||||
items, err = s.ListItems(ws.ID, models.ItemListParams{
|
||||
ParentLinkID: parent.ID,
|
||||
Search: "Distinctivekeyword",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("ListItems (FTS+parent) after delete: %v", err)
|
||||
}
|
||||
if len(items) != 0 {
|
||||
t.Errorf("expected 0 children via FTS+parent after parent soft-deleted, got %d (FTS-path parent-filter regression)", len(items))
|
||||
}
|
||||
|
||||
// Restore brings the child back through the FTS+parent path.
|
||||
if _, err := s.RestoreItem(parent.ID); err != nil {
|
||||
t.Fatalf("RestoreItem: %v", err)
|
||||
}
|
||||
items, err = s.ListItems(ws.ID, models.ItemListParams{
|
||||
ParentLinkID: parent.ID,
|
||||
Search: "Distinctivekeyword",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("ListItems (FTS+parent) after restore: %v", err)
|
||||
}
|
||||
if len(items) != 1 || items[0].ID != child.ID {
|
||||
t.Errorf("expected child to reappear via FTS+parent after restoring parent, got %d", len(items))
|
||||
}
|
||||
}
|
||||
|
||||
// TestListItems_ParentFilter_RespectsSoftDeletedParent ensures the
|
||||
// `parent=<UUID>` query filter doesn't return children of a soft-deleted
|
||||
// parent. Slug/ref filters already reject deleted parents upstream via
|
||||
// GetItem/GetItemBySlug, but raw-UUID input bypasses that path. See
|
||||
// BUG-734 / Codex review on PR #259.
|
||||
func TestListItems_ParentFilter_RespectsSoftDeletedParent(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)
|
||||
}
|
||||
|
||||
// Sanity: child is reachable via the parent filter.
|
||||
items, err := s.ListItems(ws.ID, models.ItemListParams{ParentLinkID: parent.ID})
|
||||
if err != nil {
|
||||
t.Fatalf("ListItems: %v", err)
|
||||
}
|
||||
if len(items) != 1 || items[0].ID != child.ID {
|
||||
t.Fatalf("expected to find 1 child via parent filter before delete, got %+v", items)
|
||||
}
|
||||
|
||||
// Soft-delete the parent. Filter must now return no children — no
|
||||
// caller should be able to list children of a deleted parent by UUID.
|
||||
if err := s.DeleteItem(parent.ID); err != nil {
|
||||
t.Fatalf("DeleteItem: %v", err)
|
||||
}
|
||||
items, err = s.ListItems(ws.ID, models.ItemListParams{ParentLinkID: parent.ID})
|
||||
if err != nil {
|
||||
t.Fatalf("ListItems after parent delete: %v", err)
|
||||
}
|
||||
if len(items) != 0 {
|
||||
t.Errorf("expected 0 children after parent soft-deleted, got %d (parent-filter regression)", len(items))
|
||||
}
|
||||
|
||||
// Restoring the parent should bring the child back into the filter.
|
||||
if _, err := s.RestoreItem(parent.ID); err != nil {
|
||||
t.Fatalf("RestoreItem: %v", err)
|
||||
}
|
||||
items, err = s.ListItems(ws.ID, models.ItemListParams{ParentLinkID: parent.ID})
|
||||
if err != nil {
|
||||
t.Fatalf("ListItems after restore: %v", err)
|
||||
}
|
||||
if len(items) != 1 || items[0].ID != child.ID {
|
||||
t.Errorf("expected 1 child after restoring parent, got %d", len(items))
|
||||
}
|
||||
}
|
||||
|
||||
func TestItemLinkDefaultType(t *testing.T) {
|
||||
s := testStore(t)
|
||||
ws := createTestWorkspace(t, s, "Test")
|
||||
|
||||
Reference in New Issue
Block a user