Merged in bugfix/delete-client-cascade (pull request #220)

fix cascading delete issue * test passing
2026-04-08 10:54:32 +00:00
parent b71a28d3c0
commit ad7b21f3a2
8 changed files with 229 additions and 0 deletions
@@ -96,6 +96,8 @@ type clientDependencyDelete struct {
 // deleteClientDependencies removes collector data, batch uploads, and sync records
 // for a client within an existing transaction. Must be called after documents and
 // folders are already deleted. Executes deletes in FK dependency order.
+// When batch_uploads are deleted, batch_document_outcomes are CASCADE-deleted by the
+// database FK constraint.
 func deleteClientDependencies(ctx context.Context, q *repository.Queries, clientID string) error {
 	deletes := []clientDependencyDelete{
 		{"collector min clean versions", q.DeleteCollectorMinCleanVersions},
@@ -104,6 +104,72 @@ func TestHardDeleteClient(t *testing.T) {
 		require.Error(t, err)
 	})

+	t.Run("deletes client with batch outcomes", func(t *testing.T) {
+		clientID := "test-client-batch-del"
+		test.CreateTestClient(t, cfg, clientID, "Test Client Batch Del")
+
+		// Get root folder
+		rootFolder, err := q.GetFolderByPath(ctx, &repository.GetFolderByPathParams{
+			Clientid: clientID,
+			Path:     "/",
+		})
+		require.NoError(t, err)
+
+		// Create batch upload for client
+		batchID, err := q.CreateBatchUpload(ctx, &repository.CreateBatchUploadParams{
+			ClientID:         clientID,
+			OriginalFilename: "test.zip",
+			TotalDocuments:   1,
+		})
+		require.NoError(t, err)
+
+		// Create a document linked to the batch
+		filename := "resolved.pdf"
+		docID := createDocWithEntryForClient(t, cfg, clientID, &rootFolder.ID, "batch-client-doc")
+
+		// Update the document to have the batch_id by creating via params
+		// (the helper doesn't set batch_id, so create a second doc with batch_id)
+		docID2, err := q.CreateDocument(ctx, &repository.CreateDocumentParams{
+			Clientid: clientID,
+			Hash:     "batch-client-doc2",
+			Filename: &filename,
+			Folderid: &rootFolder.ID,
+			BatchID:  &batchID,
+		})
+		require.NoError(t, err)
+		_ = docID
+
+		// Insert resolved outcome (with document_id set)
+		err = q.InsertBatchDocumentOutcome(ctx, &repository.InsertBatchDocumentOutcomeParams{
+			BatchID:    batchID,
+			Filename:   "resolved.pdf",
+			Column3:    repository.BatchOutcomeStatusInitComplete,
+			DocumentID: &docID2,
+		})
+		require.NoError(t, err)
+
+		// Insert unresolved outcome (document_id NULL)
+		err = q.InsertBatchDocumentOutcome(ctx, &repository.InsertBatchDocumentOutcomeParams{
+			BatchID:  batchID,
+			Filename: "unresolved.pdf",
+			Column3:  repository.BatchOutcomeStatusSubmitted,
+		})
+		require.NoError(t, err)
+
+		// Hard delete the client -- should fail before FK fix
+		_, err = svc.HardDelete(ctx, clientID)
+		require.NoError(t, err)
+
+		// Verify client is gone
+		_, err = q.GetClient(ctx, clientID)
+		require.Error(t, err)
+
+		// Verify batch_document_outcomes are gone (cascade from batch_uploads deletion)
+		rows, err := q.ListBatchDocumentOutcomes(ctx, batchID)
+		require.NoError(t, err)
+		assert.Empty(t, rows)
+	})
+
 	t.Run("returns NotFoundError for non-existent client", func(t *testing.T) {
 		_, err := svc.HardDelete(ctx, "non-existent-client")
 		require.Error(t, err)
@@ -0,0 +1,15 @@
+-- Revert to original FK constraints (NO ACTION).
+
+ALTER TABLE batch_document_outcomes
+    DROP CONSTRAINT fk_batch_doc_outcomes_document_id;
+
+ALTER TABLE batch_document_outcomes
+    DROP CONSTRAINT fk_batch_doc_outcomes_batch_id;
+
+ALTER TABLE batch_document_outcomes
+    ADD CONSTRAINT batch_document_outcomes_document_id_fkey
+        FOREIGN KEY (document_id) REFERENCES documents(id);
+
+ALTER TABLE batch_document_outcomes
+    ADD CONSTRAINT batch_document_outcomes_batch_id_fkey
+        FOREIGN KEY (batch_id) REFERENCES batch_uploads(id);
@@ -0,0 +1,17 @@
+-- Fix batch_document_outcomes FK constraints to support cascading deletes.
+-- document_id: SET NULL preserves batch outcome history when a document is hard-deleted.
+-- batch_id: CASCADE removes outcome rows when their owning batch_upload is deleted.
+
+ALTER TABLE batch_document_outcomes
+    DROP CONSTRAINT batch_document_outcomes_document_id_fkey;
+
+ALTER TABLE batch_document_outcomes
+    DROP CONSTRAINT batch_document_outcomes_batch_id_fkey;
+
+ALTER TABLE batch_document_outcomes
+    ADD CONSTRAINT fk_batch_doc_outcomes_document_id
+        FOREIGN KEY (document_id) REFERENCES documents(id) ON DELETE SET NULL;
+
+ALTER TABLE batch_document_outcomes
+    ADD CONSTRAINT fk_batch_doc_outcomes_batch_id
+        FOREIGN KEY (batch_id) REFERENCES batch_uploads(id) ON DELETE CASCADE;
@@ -17,6 +17,9 @@ import (
 //   - document entries (S3 references)
 //   - the document row itself (documentLabels auto-cascade via ON DELETE CASCADE)
 //
+// batch_document_outcomes.document_id is SET NULL by the database FK constraint,
+// preserving batch outcome history when a document is removed.
+//
 // S3 source files are intentionally NOT deleted. Their paths are collected before
 // the transaction, logged at INFO level, and returned for optional verbose responses.
 //
@@ -144,6 +144,64 @@ func TestHardDeleteDocument(t *testing.T) {
 		assert.True(t, errors.As(err, &notFoundErr))
 	})

+	t.Run("deletes document that has batch outcome rows", func(t *testing.T) {
+		q := cfg.GetDBQueries()
+
+		// Use a unique client to avoid collisions with other subtests
+		batchClientID := "test-doc-batch-del"
+		test.CreateTestClient(t, cfg, batchClientID, "Test Doc Batch Del")
+
+		rootFolder, err := q.GetFolderByPath(ctx, &repository.GetFolderByPathParams{
+			Clientid: batchClientID,
+			Path:     "/",
+		})
+		require.NoError(t, err)
+
+		// Create a batch upload for the client
+		batchID, err := q.CreateBatchUpload(ctx, &repository.CreateBatchUploadParams{
+			ClientID:         batchClientID,
+			OriginalFilename: "test.zip",
+			TotalDocuments:   1,
+		})
+		require.NoError(t, err)
+
+		// Create a document linked to the batch
+		filename := "test.pdf"
+		docID, err := q.CreateDocument(ctx, &repository.CreateDocumentParams{
+			Clientid: batchClientID,
+			Hash:     "batch-doc-del-test",
+			Filename: &filename,
+			Folderid: &rootFolder.ID,
+			BatchID:  &batchID,
+		})
+		require.NoError(t, err)
+
+		// Insert a submitted outcome referencing this document
+		err = q.InsertBatchDocumentOutcome(ctx, &repository.InsertBatchDocumentOutcomeParams{
+			BatchID:    batchID,
+			Filename:   "test.pdf",
+			Column3:    repository.BatchOutcomeStatusSubmitted,
+			DocumentID: &docID,
+		})
+		require.NoError(t, err)
+
+		// Hard delete the document -- should fail before FK fix
+		s3Paths, err := svc.HardDelete(ctx, docID)
+		require.NoError(t, err)
+
+		// Verify document is gone
+		_ = s3Paths
+		_, err = q.GetDocumentSummary(ctx, docID)
+		require.Error(t, err)
+
+		// Verify the batch outcome row still exists but document_id is NULL
+		// (history preserved via SET NULL on FK)
+		rows, err := q.ListBatchDocumentOutcomes(ctx, batchID)
+		require.NoError(t, err)
+		require.Len(t, rows, 1)
+		assert.Nil(t, rows[0].DocumentID)
+	})
+
 	t.Run("returns empty S3 paths when document has no entries", func(t *testing.T) {
 		q := cfg.GetDBQueries()

@@ -21,6 +21,8 @@ import (
 // When includeDocuments is true, all documents in the folder tree are cascade-deleted
 // (using the same document cascade as document.HardDelete), then documentUploads and
 // folder rows are removed. The entire operation executes in a single database transaction.
+// batch_document_outcomes are handled by database FK actions: document_id is SET NULL
+// when a document is deleted, and rows CASCADE-delete when their batch_upload is removed.
 //
 // S3 source files are intentionally NOT deleted. Their paths are collected, logged at
 // INFO level, and returned for optional verbose responses.
@@ -137,6 +137,72 @@ func TestHardDeleteFolder(t *testing.T) {
 		require.Error(t, err)
 	})

+	t.Run("deletes folder with batch-backed documents and outcomes", func(t *testing.T) {
+		// Use a unique client to avoid collisions
+		batchClientID := "test-folder-batch-del"
+		test.CreateTestClient(t, cfg, batchClientID, "Test Folder Batch Del")
+
+		batchRootFolder, err := q.GetFolderByPath(ctx, &repository.GetFolderByPathParams{
+			Clientid: batchClientID,
+			Path:     "/",
+		})
+		require.NoError(t, err)
+
+		batchSvc := folder.New(cfg)
+
+		// Create a subfolder under root
+		subFolder, err := batchSvc.CreateFolder(ctx, "/batch-del-sub", &batchRootFolder.ID, batchClientID, "testuser")
+		require.NoError(t, err)
+
+		// Create a batch upload for the client
+		batchID, err := q.CreateBatchUpload(ctx, &repository.CreateBatchUploadParams{
+			ClientID:         batchClientID,
+			OriginalFilename: "test.zip",
+			TotalDocuments:   1,
+		})
+		require.NoError(t, err)
+
+		// Create a document in the subfolder with batch_id
+		filename := "batch-folder-doc.pdf"
+		docID, err := q.CreateDocument(ctx, &repository.CreateDocumentParams{
+			Clientid: batchClientID,
+			Hash:     "batch-folder-doc-hash",
+			Filename: &filename,
+			Folderid: &subFolder.ID,
+			BatchID:  &batchID,
+		})
+		require.NoError(t, err)
+
+		// Add a document entry (S3 ref)
+		err = q.AddDocumentEntry(ctx, &repository.AddDocumentEntryParams{
+			Documentid: docID,
+			Bucket:     "test-bucket",
+			Key:        "test-key/batch-folder-doc-hash",
+		})
+		require.NoError(t, err)
+
+		// Insert a resolved outcome with document_id set
+		err = q.InsertBatchDocumentOutcome(ctx, &repository.InsertBatchDocumentOutcomeParams{
+			BatchID:    batchID,
+			Filename:   "batch-folder-doc.pdf",
+			Column3:    repository.BatchOutcomeStatusInitComplete,
+			DocumentID: &docID,
+		})
+		require.NoError(t, err)
+
+		// Hard delete the folder with includeDocuments=true -- should fail before FK fix
+		_, err = batchSvc.HardDelete(ctx, subFolder.ID, true)
+		require.NoError(t, err)
+
+		// Verify folder is gone
+		_, err = q.GetFolderByID(ctx, subFolder.ID)
+		require.Error(t, err)
+
+		// Verify document is gone
+		_, err = q.GetDocumentSummary(ctx, docID)
+		require.Error(t, err)
+	})
+
 	t.Run("counts documents in nested folder tree correctly", func(t *testing.T) {
 		parent, err := svc.CreateFolder(ctx, "/del-count", &rootFolder.ID, clientID, "testuser")
 		require.NoError(t, err)