From f451e94c0b44903abe78aeecbcf68f0fb2e8d05e Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Wed, 29 Jul 2026 15:01:37 +0000 Subject: [PATCH] Clear document mappings on soft delete Document soft delete cleared generated-document references but left control, risk, and measure junction rows pointing at deleted documents. That blocked risk deletion and made unlinkRisk fail when the document was already gone. Remove entity mappings in SoftDelete and BulkSoftDelete, drop mappings before deleting a risk, tolerate missing documents when unlinking, and backfill orphaned junction rows for soft-deleted documents. Signed-off-by: Cursor Agent Co-authored-by: Bryan FRIMIN --- e2e/console/mapping_test.go | 100 +++++++++++++++++++ pkg/coredata/migrations/20260729T150130Z.sql | 43 ++++++++ pkg/coredata/risk_document.go | 27 +++++ pkg/probo/control_service.go | 8 +- pkg/probo/document_service.go | 65 +++++++----- pkg/probo/measure_service.go | 8 +- pkg/probo/risk_service.go | 13 ++- 7 files changed, 229 insertions(+), 35 deletions(-) create mode 100644 pkg/coredata/migrations/20260729T150130Z.sql diff --git a/e2e/console/mapping_test.go b/e2e/console/mapping_test.go index 64af13b81..422a0d186 100644 --- a/e2e/console/mapping_test.go +++ b/e2e/console/mapping_test.go @@ -728,6 +728,106 @@ func TestRiskDocumentMapping_CreateDelete(t *testing.T) { }) } +func TestRiskDocumentMapping_DeleteDocumentClearsMappingAndAllowsRiskDelete(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + + riskID := factory.NewRisk(owner). + WithName("Risk linked to deleted document"). + Create() + documentID := factory.NewDocument(owner). + WithTitle("Document linked to risk"). + Create() + + _, err := owner.Do(` + mutation($input: CreateRiskDocumentMappingInput!) { + createRiskDocumentMapping(input: $input) { + riskEdge { node { id } } + } + } + `, map[string]any{ + "input": map[string]any{ + "riskId": riskID, + "documentId": documentID, + }, + }) + require.NoError(t, err) + + _, err = owner.Do(` + mutation DeleteDocument($input: DeleteDocumentInput!) { + deleteDocument(input: $input) { + deletedDocumentId + } + } + `, map[string]any{ + "input": map[string]any{"documentId": documentID}, + }) + require.NoError(t, err) + + _, err = owner.Do(` + mutation($input: DeleteRiskDocumentMappingInput!) { + deleteRiskDocumentMapping(input: $input) { + deletedRiskId + deletedDocumentId + } + } + `, map[string]any{ + "input": map[string]any{ + "riskId": riskID, + "documentId": documentID, + }, + }) + require.NoError(t, err) + + _, err = owner.Do(` + mutation DeleteRisk($input: DeleteRiskInput!) { + deleteRisk(input: $input) { + deletedRiskId + } + } + `, map[string]any{ + "input": map[string]any{"riskId": riskID}, + }) + require.NoError(t, err) +} + +func TestRiskDocumentMapping_DeleteRiskWithActiveDocumentMapping(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + + riskID := factory.NewRisk(owner). + WithName("Risk with active document link"). + Create() + documentID := factory.NewDocument(owner). + WithTitle("Document still linked to risk"). + Create() + + _, err := owner.Do(` + mutation($input: CreateRiskDocumentMappingInput!) { + createRiskDocumentMapping(input: $input) { + riskEdge { node { id } } + } + } + `, map[string]any{ + "input": map[string]any{ + "riskId": riskID, + "documentId": documentID, + }, + }) + require.NoError(t, err) + + _, err = owner.Do(` + mutation DeleteRisk($input: DeleteRiskInput!) { + deleteRisk(input: $input) { + deletedRiskId + } + } + `, map[string]any{ + "input": map[string]any{"riskId": riskID}, + }) + require.NoError(t, err) +} + func TestRiskObligationMapping_CreateDelete(t *testing.T) { t.Parallel() owner := testutil.NewClient(t, testutil.RoleOwner) diff --git a/pkg/coredata/migrations/20260729T150130Z.sql b/pkg/coredata/migrations/20260729T150130Z.sql new file mode 100644 index 000000000..1e03be1a8 --- /dev/null +++ b/pkg/coredata/migrations/20260729T150130Z.sql @@ -0,0 +1,43 @@ +-- Copyright (c) 2026 Probo Inc . +-- +-- Permission is hereby granted, free of charge, to any person obtaining a copy +-- of this software and associated documentation files (the "Software"), to deal +-- in the Software without restriction, including without limitation the rights +-- to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +-- copies of the Software, and to permit persons to whom the Software is +-- furnished to do so, subject to the following conditions: +-- +-- The above copyright notice and this permission notice shall be included in +-- all copies or substantial portions of the Software. +-- +-- THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +-- IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +-- FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +-- AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +-- LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +-- OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +-- SOFTWARE. + +DELETE FROM risks_documents rd +WHERE EXISTS ( + SELECT 1 + FROM documents d + WHERE d.id = rd.document_id + AND d.deleted_at IS NOT NULL +); + +DELETE FROM controls_documents cd +WHERE EXISTS ( + SELECT 1 + FROM documents d + WHERE d.id = cd.document_id + AND d.deleted_at IS NOT NULL +); + +DELETE FROM measures_documents md +WHERE EXISTS ( + SELECT 1 + FROM documents d + WHERE d.id = md.document_id + AND d.deleted_at IS NOT NULL +); diff --git a/pkg/coredata/risk_document.go b/pkg/coredata/risk_document.go index 17b9c6428..f2acb93f0 100644 --- a/pkg/coredata/risk_document.go +++ b/pkg/coredata/risk_document.go @@ -108,6 +108,33 @@ WHERE return err } +func (rp RiskDocument) DeleteByRiskID( + ctx context.Context, + conn pg.Tx, + scope Scoper, + riskID gid.GID, +) error { + q := ` +DELETE +FROM + risks_documents +WHERE + %s + AND risk_id = @risk_id; +` + + q = fmt.Sprintf(q, scope.SQLFragment()) + + args := pgx.StrictNamedArgs{ + "risk_id": riskID, + } + maps.Copy(args, scope.SQLArguments()) + + _, err := conn.Exec(ctx, q, args) + + return err +} + func (rp RiskDocument) DeleteByDocumentIDs( ctx context.Context, conn pg.Tx, diff --git a/pkg/probo/control_service.go b/pkg/probo/control_service.go index e56e6d217..47889e3ea 100644 --- a/pkg/probo/control_service.go +++ b/pkg/probo/control_service.go @@ -501,7 +501,7 @@ func (s ControlService) DeleteDocumentMapping( documentID gid.GID, ) (*coredata.Control, *coredata.Document, error) { control := &coredata.Control{} - document := &coredata.Document{} + document := &coredata.Document{ID: documentID} err := s.svc.pg.WithTx( ctx, @@ -511,11 +511,13 @@ func (s ControlService) DeleteDocumentMapping( } if err := document.LoadByID(ctx, tx, scope, documentID); err != nil { - return fmt.Errorf("cannot load document: %w", err) + if !errors.Is(err, coredata.ErrResourceNotFound) { + return fmt.Errorf("cannot load document: %w", err) + } } controlDocument := &coredata.ControlDocument{} - if err := controlDocument.Delete(ctx, tx, scope, control.ID, document.ID); err != nil { + if err := controlDocument.Delete(ctx, tx, scope, control.ID, documentID); err != nil { return fmt.Errorf("cannot delete control document mapping: %w", err) } diff --git a/pkg/probo/document_service.go b/pkg/probo/document_service.go index aeb49793e..e39c1f3a4 100644 --- a/pkg/probo/document_service.go +++ b/pkg/probo/document_service.go @@ -1598,6 +1598,10 @@ func (s *DocumentService) SoftDelete( return fmt.Errorf("cannot emit document deleted webhook: %w", err) } + if err := s.deleteDocumentEntityMappings(ctx, scope, tx, []gid.GID{documentID}); err != nil { + return err + } + if err := s.clearDocumentReferences(ctx, scope, tx, []gid.GID{documentID}); err != nil { return err } @@ -1624,6 +1628,10 @@ func (s *DocumentService) BulkSoftDelete( return fmt.Errorf("cannot emit document deleted webhooks: %w", err) } + if err := s.deleteDocumentEntityMappings(ctx, scope, tx, documentIDs); err != nil { + return err + } + if err := s.clearDocumentReferences(ctx, scope, tx, documentIDs); err != nil { return err } @@ -1652,19 +1660,8 @@ func (s *DocumentService) BulkArchive( } } - controlDocument := coredata.ControlDocument{} - if err := controlDocument.DeleteByDocumentIDs(ctx, tx, scope, documentIDs); err != nil { - return fmt.Errorf("cannot delete control mappings: %w", err) - } - - riskDocument := coredata.RiskDocument{} - if err := riskDocument.DeleteByDocumentIDs(ctx, tx, scope, documentIDs); err != nil { - return fmt.Errorf("cannot delete risk mappings: %w", err) - } - - measureDocument := coredata.MeasureDocument{} - if err := measureDocument.DeleteByDocumentIDs(ctx, tx, scope, documentIDs); err != nil { - return fmt.Errorf("cannot delete measure mappings: %w", err) + if err := s.deleteDocumentEntityMappings(ctx, scope, tx, documentIDs); err != nil { + return err } if err := s.clearDocumentReferences(ctx, scope, tx, documentIDs); err != nil { @@ -1716,6 +1713,33 @@ func (s *DocumentService) BulkUnarchive( ) } +func (s *DocumentService) deleteDocumentEntityMappings( + ctx context.Context, scope coredata.Scoper, + tx pg.Tx, + documentIDs []gid.GID, +) error { + if len(documentIDs) == 0 { + return nil + } + + controlDocument := coredata.ControlDocument{} + if err := controlDocument.DeleteByDocumentIDs(ctx, tx, scope, documentIDs); err != nil { + return fmt.Errorf("cannot delete control mappings: %w", err) + } + + riskDocument := coredata.RiskDocument{} + if err := riskDocument.DeleteByDocumentIDs(ctx, tx, scope, documentIDs); err != nil { + return fmt.Errorf("cannot delete risk mappings: %w", err) + } + + measureDocument := coredata.MeasureDocument{} + if err := measureDocument.DeleteByDocumentIDs(ctx, tx, scope, documentIDs); err != nil { + return fmt.Errorf("cannot delete measure mappings: %w", err) + } + + return nil +} + // clearDocumentReferences nullifies references to the given document IDs in // generated_documents and statements_of_applicability. This must be called // inside a transaction before soft-deleting or archiving documents, because @@ -2476,19 +2500,8 @@ func (s *DocumentService) Archive( return err } - controlDocument := coredata.ControlDocument{} - if err := controlDocument.DeleteByDocumentIDs(ctx, tx, scope, []gid.GID{documentID}); err != nil { - return fmt.Errorf("cannot delete control mappings: %w", err) - } - - riskDocument := coredata.RiskDocument{} - if err := riskDocument.DeleteByDocumentIDs(ctx, tx, scope, []gid.GID{documentID}); err != nil { - return fmt.Errorf("cannot delete risk mappings: %w", err) - } - - measureDocument := coredata.MeasureDocument{} - if err := measureDocument.DeleteByDocumentIDs(ctx, tx, scope, []gid.GID{documentID}); err != nil { - return fmt.Errorf("cannot delete measure mappings: %w", err) + if err := s.deleteDocumentEntityMappings(ctx, scope, tx, []gid.GID{documentID}); err != nil { + return err } if err := s.clearDocumentReferences(ctx, scope, tx, []gid.GID{documentID}); err != nil { diff --git a/pkg/probo/measure_service.go b/pkg/probo/measure_service.go index 2aa1d997b..d071e9bb4 100644 --- a/pkg/probo/measure_service.go +++ b/pkg/probo/measure_service.go @@ -789,7 +789,7 @@ func (s MeasureService) DeleteDocumentMapping( documentID gid.GID, ) (*coredata.Measure, *coredata.Document, error) { measure := &coredata.Measure{} - document := &coredata.Document{} + document := &coredata.Document{ID: documentID} err := s.svc.pg.WithTx( ctx, @@ -799,11 +799,13 @@ func (s MeasureService) DeleteDocumentMapping( } if err := document.LoadByID(ctx, tx, scope, documentID); err != nil { - return fmt.Errorf("cannot load document: %w", err) + if !errors.Is(err, coredata.ErrResourceNotFound) { + return fmt.Errorf("cannot load document: %w", err) + } } measureDocument := &coredata.MeasureDocument{} - if err := measureDocument.Delete(ctx, tx, scope, measure.ID, document.ID); err != nil { + if err := measureDocument.Delete(ctx, tx, scope, measure.ID, documentID); err != nil { return fmt.Errorf("cannot delete measure document mapping: %w", err) } diff --git a/pkg/probo/risk_service.go b/pkg/probo/risk_service.go index 76f335cc2..d892359e1 100644 --- a/pkg/probo/risk_service.go +++ b/pkg/probo/risk_service.go @@ -249,7 +249,7 @@ func (s RiskService) DeleteDocumentMapping( ) (*coredata.Risk, *coredata.Document, error) { riskDocument := &coredata.RiskDocument{} risk := &coredata.Risk{} - document := &coredata.Document{} + document := &coredata.Document{ID: documentID} err := s.svc.pg.WithTx( ctx, @@ -259,10 +259,12 @@ func (s RiskService) DeleteDocumentMapping( } if err := document.LoadByID(ctx, tx, scope, documentID); err != nil { - return fmt.Errorf("cannot load document: %w", err) + if !errors.Is(err, coredata.ErrResourceNotFound) { + return fmt.Errorf("cannot load document: %w", err) + } } - return riskDocument.Delete(ctx, tx, scope, risk.ID, document.ID) + return riskDocument.Delete(ctx, tx, scope, risk.ID, documentID) }, ) if err != nil { @@ -610,10 +612,15 @@ func (s RiskService) Delete( riskID gid.GID, ) error { risk := &coredata.Risk{} + riskDocument := &coredata.RiskDocument{} return s.svc.pg.WithTx( ctx, func(ctx context.Context, tx pg.Tx) error { + if err := riskDocument.DeleteByRiskID(ctx, tx, scope, riskID); err != nil { + return fmt.Errorf("cannot delete risk document mappings: %w", err) + } + return risk.Delete(ctx, tx, scope, riskID) }, )