fix(store): preserve valid item references across workspace import (#1271)

2026-09-10 15:05:40 +00:00 · 2026-09-07 14:27:58 +02:00
parent e864a43644
commit a3a1d5862b
3 changed files with 173 additions and 6 deletions
@@ -162,6 +162,13 @@ pad workspace import my-workspace.json
 pad workspace import --name "imported-workspace" my-workspace.json
 ```

+Item reference numbers are preserved when the imported items have positive,
+workspace-unique numbers, including gaps left by deleted items. This keeps
+references such as `[[TASK-2]]` pointing to the same task after restoration or
+SQLite→PostgreSQL migration. Legacy archives with duplicate, missing, or invalid
+numbers retain the sequential-renumbering fallback; references in those archives
+may need manual repair.
+
 ### One case where an export is not importable

 A workspace whose stored data contains a **NUL character** exports fine and is
@@ -597,11 +597,25 @@ func (s *Store) ImportWorkspace(data *models.WorkspaceExport, newName string, ow
 		}
 	}

-	// Import items (first pass: create items, remap collection_id)
-	// Item numbers are assigned sequentially in created_at order to produce
-	// workspace-global numbering. Exported item_number values are ignored
-	// because old exports used per-collection numbering which can have
-	// duplicates within a workspace.
+	// Preserve modern archives' reference numbers, including deletion gaps and
+	// out-of-order items: content and comments still refer to those numbers.
+	// Legacy archives used per-collection numbering (or had no numbers), so
+	// retain sequential allocation when the imported set is not workspace-unique.
+	// Orphans are skipped below and must not force valid items to be renumbered.
+	preserveItemNumbers := true
+	seenItemNumbers := make(map[int]bool, len(data.Items))
+	for _, it := range data.Items {
+		if collMap[it.CollectionID] == "" {
+			continue
+		}
+		if it.ItemNumber <= 0 || seenItemNumbers[it.ItemNumber] {
+			preserveItemNumbers = false
+			break
+		}
+		seenItemNumbers[it.ItemNumber] = true
+	}
+
+	// Import items (first pass: create items, remap collection_id).
 	//
 	// IDEA-1486 + IDEA-1488: precompute each item's coerced fields/tags so
 	// the second-pass UPDATE (which re-applies fields after the ID remap)
@@ -649,6 +663,10 @@ func (s *Store) ImportWorkspace(data *models.WorkspaceExport, newName string, ow
 		parentID := resolveImportParent(it.ParentID, itemMap, insertedItems)

 		nextItemNumber++
+		itemNumber := nextItemNumber
+		if preserveItemNumbers {
+			itemNumber = it.ItemNumber
+		}
 		// IDEA-1486 + IDEA-1488: coerce empty-string / malformed
 		// fields/tags at the import boundary. After migration 056 /
 		// pgmigrations 035 hardened items.fields and items.tags to
@@ -736,7 +754,7 @@ func (s *Store) ImportWorkspace(data *models.WorkspaceExport, newName string, ow
 			INSERT INTO items (id, workspace_id, collection_id, title, slug, content, fields, tags, pinned, sort_order, parent_id, created_by, last_modified_by, source, item_number, created_at, updated_at, seq)
 			VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, NULLIF(?, ''), ?, ?, ?, ?, ?, ?, `+nextWorkspaceSeqSubquery+`)`),
 			newItemID, ws.ID, newCollID, itemTitle, itemSlug, it.Content, fieldsJSON, tagsJSON, s.dialect.BoolToInt(it.Pinned), it.SortOrder,
-			parentID, it.CreatedBy, it.LastModifiedBy, it.Source, nextItemNumber,
+			parentID, it.CreatedBy, it.LastModifiedBy, it.Source, itemNumber,
 			it.CreatedAt, it.UpdatedAt, ws.ID)
 		if err != nil {
 			return nil, fmt.Errorf("import item %s: %w", it.Title, err)
@@ -0,0 +1,142 @@
+package store
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/PerpetualSoftware/pad/internal/models"
+)
+
+func TestImportWorkspacePreservesReferenceTarget(t *testing.T) {
+	t.Parallel()
+	for _, deleteFirst := range []bool{false, true} {
+		t.Run(fmt.Sprintf("deleted_first=%v", deleteFirst), func(t *testing.T) {
+			s := testStore(t)
+			owner := createTestUser(t, s, "audit@example.com", "Audit", "password123")
+			ws := createTestWorkspace(t, s, "Reference source")
+			coll, err := s.CreateCollection(ws.ID, models.CollectionCreate{Name: "Tasks", Prefix: "TASK"})
+			if err != nil {
+				t.Fatal(err)
+			}
+			first := createTestItem(t, s, ws.ID, coll.ID, "Obsolete task", "")
+			target := createTestItem(t, s, ws.ID, coll.ID, "Deploy the release", "")
+			note := createTestItem(t, s, ws.ID, coll.ID, "Release instructions", "Follow [[TASK-2]] before continuing.")
+			comment, err := s.CreateComment(ws.ID, note.ID, "", models.CommentCreate{Body: "Review [[TASK-2]] first."})
+			if err != nil {
+				t.Fatal(err)
+			}
+			// Deterministic fixture for ordinary items created one second apart.
+			for i, item := range []*models.Item{first, target, note} {
+				_, err := s.db.Exec(s.q("UPDATE items SET created_at = ? WHERE id = ?"), fmt.Sprintf("2026-09-01T12:00:0%dZ", i), item.ID)
+				if err != nil {
+					t.Fatal(err)
+				}
+			}
+			before, err := s.ResolveItem(ws.ID, "TASK-2")
+			if err != nil || before == nil || before.ID != target.ID {
+				t.Fatalf("source control: item=%+v err=%v", before, err)
+			}
+			if deleteFirst {
+				if err := s.DeleteItem(first.ID); err != nil {
+					t.Fatal(err)
+				}
+			}
+			bundle, err := s.ExportWorkspace(ws.Slug)
+			if err != nil {
+				t.Fatal(err)
+			}
+			dst, err := s.ImportWorkspace(bundle, "Reference restored", owner.ID, "cli")
+			if err != nil {
+				t.Fatal(err)
+			}
+			after, err := s.ResolveItem(dst.ID, "TASK-2")
+			if err != nil || after == nil {
+				t.Fatalf("restored ref lookup: item=%+v err=%v", after, err)
+			}
+			importedNote, err := s.ResolveItem(dst.ID, note.Slug)
+			if err != nil || importedNote == nil {
+				t.Fatalf("restored note lookup: item=%+v err=%v", importedNote, err)
+			}
+			t.Logf("before TASK-2=%q; after TASK-2=%q; restored body=%q; export/import succeeded", before.Title, after.Title, importedNote.Content)
+			if after.Title != target.Title {
+				t.Errorf("reference changed target: want %q, got %q", target.Title, after.Title)
+			}
+			if importedNote.Content != note.Content {
+				t.Errorf("content changed: %q", importedNote.Content)
+			}
+			comments, err := s.ListComments(importedNote.ID)
+			if err != nil || len(comments) != 1 || comments[0].Body != comment.Body {
+				t.Errorf("comments did not round-trip: %+v, err=%v", comments, err)
+			}
+		})
+	}
+}
+
+func TestImportWorkspaceItemNumberCompatibility(t *testing.T) {
+	t.Parallel()
+	for _, tc := range []struct {
+		name    string
+		numbers []int
+		want    []int
+		orphan  bool
+	}{
+		{name: "out of order with gaps", numbers: []int{7, 2}, want: []int{7, 2}},
+		{name: "legacy duplicate", numbers: []int{1, 1}, want: []int{1, 2}},
+		{name: "missing number", numbers: []int{0, 2}, want: []int{1, 2}},
+		{name: "negative number", numbers: []int{-1, 2}, want: []int{1, 2}},
+		{name: "unnumbered archive", numbers: []int{0, 0}, want: []int{1, 2}},
+		{name: "orphan duplicate ignored", numbers: []int{7, 2}, want: []int{7, 2}, orphan: true},
+		{name: "empty archive"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			s := testStore(t)
+			owner := createTestUser(t, s, "import@example.com", "Importer", "password123")
+			const timestamp = "2026-09-01T12:00:00Z"
+			bundle := &models.WorkspaceExport{
+				Version:   1,
+				Workspace: models.WorkspaceExportMeta{Name: "Number archive", Slug: "number-archive"},
+				Collections: []models.CollectionExport{
+					{ID: "tasks", Name: "Tasks", Slug: "tasks", Prefix: "TASK", Schema: `{"fields":[]}`, CreatedAt: timestamp, UpdatedAt: timestamp},
+					{ID: "plans", Name: "Plans", Slug: "plans", Prefix: "PLAN", Schema: `{"fields":[]}`, CreatedAt: timestamp, UpdatedAt: timestamp},
+				},
+			}
+			for i, number := range tc.numbers {
+				slug := fmt.Sprintf("item-%d", i)
+				bundle.Items = append(bundle.Items, models.ItemExport{
+					ID: slug, CollectionID: bundle.Collections[i].ID, Title: slug, Slug: slug,
+					ItemNumber: number, CreatedAt: timestamp, UpdatedAt: timestamp,
+				})
+			}
+			if tc.orphan {
+				bundle.Items = append(bundle.Items, models.ItemExport{ID: "orphan", CollectionID: "missing", ItemNumber: 7})
+			}
+			dst, err := s.ImportWorkspace(bundle, "Imported numbers", owner.ID, "")
+			if err != nil {
+				t.Fatal(err)
+			}
+			imported, err := s.ExportWorkspace(dst.Slug)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if len(imported.Items) != len(tc.want) {
+				t.Fatalf("imported %d items, want %d", len(imported.Items), len(tc.want))
+			}
+			maxNumber := 0
+			for i, number := range tc.want {
+				item, err := s.GetItemBySlug(dst.ID, bundle.Items[i].Slug)
+				if err != nil || item == nil || item.ItemNumber == nil {
+					t.Fatalf("read imported item: %+v, err=%v", item, err)
+				}
+				if *item.ItemNumber != number {
+					t.Errorf("%s number=%d, want %d", item.Slug, *item.ItemNumber, number)
+				}
+				maxNumber = max(maxNumber, number)
+			}
+			created := createTestItem(t, s, dst.ID, imported.Collections[0].ID, "After import", "")
+			if created.ItemNumber == nil || *created.ItemNumber != maxNumber+1 {
+				t.Errorf("next number=%v, want %d", created.ItemNumber, maxNumber+1)
+			}
+		})
+	}
+}