From 936162d5c770e08e2661ea0192449e487ea445a1 Mon Sep 17 00:00:00 2001 From: Sacha Al Himdani Date: Tue, 7 Jul 2026 19:03:53 +0200 Subject: [PATCH] Resolve document publish approvers from defaults The MCP publishDocument tool required callers to pass approver_ids and to distinguish an omitted list (rejected) from an empty one (direct publish), a null-vs-empty subtlety that is awkward for an LLM to get right. Drop approver_ids from the MCP tool and resolve a major publish's approvers from the document's default approvers instead: an approval is requested when the document has default approvers, otherwise the version is published directly. Default approvers are configured with addDocument or updateDocument. Replace a document's default approvers on every major publish that supplies an explicit list, even when the list is empty, so a direct publish through the GraphQL API clears stale approvers instead of leaving them behind (previously the empty case skipped the update). Expose the default-approver behaviour as a separate entry point, PublishVersionWithDefaultApprovers, that loads the defaults and delegates to PublishVersion. PublishVersion keeps its explicit-approver contract for the GraphQL API. Require only the publish permission to publish a version, whether or not it opens an approval quorum, and drop the now-unused request-approval action. Fold the publish steps into the publishMinor and publishMajor primitives shared by both the single and bulk publish paths, and drop the redundant InTx suffix from RequestApproval and emitDocumentEvent, which already take a transaction argument. Signed-off-by: Sacha Al Himdani --- e2e/console/document_version_test.go | 39 ++ e2e/mcp/document_test.go | 68 ++++ pkg/probo/actions.go | 21 +- pkg/probo/document_approval_service.go | 67 ++-- pkg/probo/document_service.go | 360 +++++++++++------- pkg/probo/generated_document_service.go | 13 +- pkg/probo/oauth2_scopes.go | 1 - .../api/console/v1/document_resolvers.go | 7 +- pkg/server/api/mcp/v1/schema.resolvers.go | 16 +- pkg/server/api/mcp/v1/specification.yaml | 9 +- 10 files changed, 396 insertions(+), 205 deletions(-) diff --git a/e2e/console/document_version_test.go b/e2e/console/document_version_test.go index d65a80ca0..0663e92cf 100644 --- a/e2e/console/document_version_test.go +++ b/e2e/console/document_version_test.go @@ -1688,6 +1688,45 @@ func TestDocument_DefaultApprovers(t *testing.T) { assert.Empty(t, result.UpdateDocument.Document.DefaultApprovers) }) + + t.Run("major publish with explicit approvers updates default approvers, even when empty", func(t *testing.T) { + t.Parallel() + + docID := createTestDocumentWithApprovers(t, owner, []string{approverID}) + + var result struct { + PublishDocument struct { + Document struct { + DefaultApprovers []struct { + ID string `json:"id"` + } `json:"defaultApprovers"` + } `json:"document"` + DocumentVersion struct { + Status string `json:"status"` + } `json:"documentVersion"` + } `json:"publishDocument"` + } + + err := owner.Execute(` + mutation($input: PublishDocumentInput!) { + publishDocument(input: $input) { + document { defaultApprovers { id } } + documentVersion { status } + } + } + `, map[string]any{ + "input": map[string]any{ + "minor": false, + "documentId": docID, + "approverIds": []string{}, + "changelog": "Direct publish clearing approvers", + }, + }, &result) + require.NoError(t, err) + + assert.Equal(t, "PUBLISHED", result.PublishDocument.DocumentVersion.Status) + assert.Empty(t, result.PublishDocument.Document.DefaultApprovers) + }) } func TestDocumentVersion_DeleteDraft(t *testing.T) { diff --git a/e2e/mcp/document_test.go b/e2e/mcp/document_test.go index 16171d214..80005f2d3 100644 --- a/e2e/mcp/document_test.go +++ b/e2e/mcp/document_test.go @@ -93,3 +93,71 @@ func TestMCP_Document_CRUD(t *testing.T) { }, &deleteResult) assert.Equal(t, addResult.Document.ID, deleteResult.DeletedDocumentID) } + +func TestMCP_Document_PublishUsesDefaultApprovers(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + mc := testutil.NewMCPClient(t, owner) + orgID := owner.GetOrganizationID().String() + approverID := owner.GetProfileID().String() + + addDocument := func(defaultApproverIDs []string) string { + input := map[string]any{ + "organizationId": orgID, + "title": factory.SafeName("Document"), + "content": "Body content", + "classification": "INTERNAL", + "documentType": "POLICY", + } + if defaultApproverIDs != nil { + input["defaultApproverIds"] = defaultApproverIDs + } + + var addResult struct { + Document struct { + ID string `json:"id"` + } `json:"document"` + } + mc.CallToolInto("addDocument", input, &addResult) + require.NotEmpty(t, addResult.Document.ID) + + return addResult.Document.ID + } + + type publishResult struct { + DocumentVersion struct { + Status string `json:"status"` + } `json:"documentVersion"` + ApprovalQuorum *struct { + ID string `json:"id"` + } `json:"approvalQuorum"` + } + + t.Run("requests approval from default approvers", func(t *testing.T) { + docID := addDocument([]string{approverID}) + + var result publishResult + mc.CallToolInto("publishDocument", map[string]any{ + "documentId": docID, + "minor": false, + "changelog": "Initial major", + }, &result) + + require.NotNil(t, result.ApprovalQuorum) + assert.Equal(t, "PENDING_APPROVAL", result.DocumentVersion.Status) + }) + + t.Run("publishes directly without default approvers", func(t *testing.T) { + docID := addDocument(nil) + + var result publishResult + mc.CallToolInto("publishDocument", map[string]any{ + "documentId": docID, + "minor": false, + "changelog": "Initial major", + }, &result) + + assert.Nil(t, result.ApprovalQuorum) + assert.Equal(t, "PUBLISHED", result.DocumentVersion.Status) + }) +} diff --git a/pkg/probo/actions.go b/pkg/probo/actions.go index 44ebe7caf..f81981673 100644 --- a/pkg/probo/actions.go +++ b/pkg/probo/actions.go @@ -200,17 +200,16 @@ const ( ActionDocumentDeleteDraft = "core:document:delete-draft" // DocumentVersion actions - ActionDocumentVersionGet = "core:document-version:get" - ActionDocumentVersionList = "core:document-version:list" - ActionDocumentVersionExportPDF = "core:document-version:export-pdf" - ActionDocumentVersionSign = "core:document-version:sign" - ActionDocumentVersionRequestApproval = "core:document-version:request-approval" - ActionDocumentVersionVoidApproval = "core:document-version:void-approval" - ActionDocumentVersionApprove = "core:document-version:approve" - ActionDocumentVersionReject = "core:document-version:reject" - ActionDocumentVersionApprovalList = "core:document-version:approval-list" - ActionDocumentVersionPublish = "core:document-version:publish" - ActionDocumentVersionExport = "core:document-version:export" + ActionDocumentVersionGet = "core:document-version:get" + ActionDocumentVersionList = "core:document-version:list" + ActionDocumentVersionExportPDF = "core:document-version:export-pdf" + ActionDocumentVersionSign = "core:document-version:sign" + ActionDocumentVersionVoidApproval = "core:document-version:void-approval" + ActionDocumentVersionApprove = "core:document-version:approve" + ActionDocumentVersionReject = "core:document-version:reject" + ActionDocumentVersionApprovalList = "core:document-version:approval-list" + ActionDocumentVersionPublish = "core:document-version:publish" + ActionDocumentVersionExport = "core:document-version:export" // EmployeeDocument actions ActionEmployeeDocumentGet = "core:employee-document:get" diff --git a/pkg/probo/document_approval_service.go b/pkg/probo/document_approval_service.go index 9e4177386..1ff0d1b09 100644 --- a/pkg/probo/document_approval_service.go +++ b/pkg/probo/document_approval_service.go @@ -75,8 +75,9 @@ func (e ErrApprovalDecisionAlreadyMade) Error() string { return "approval decision has already been made" } -func (s *DocumentApprovalService) RequestApprovalInTx( - ctx context.Context, scope coredata.Scoper, +func (s *DocumentApprovalService) RequestApproval( + ctx context.Context, + scope coredata.Scoper, tx pg.Tx, document *coredata.Document, documentVersion *coredata.DocumentVersion, @@ -134,7 +135,8 @@ func (s *DocumentApprovalService) RequestApprovalInTx( // an approval is requested for it; otherwise it is published as a major // bump. Documents with no draft (or already pending approval) are skipped. func (s *DocumentApprovalService) BulkPublishVersions( - ctx context.Context, scope coredata.Scoper, + ctx context.Context, + scope coredata.Scoper, req BulkPublishVersionsRequest, ) ([]*coredata.DocumentVersion, []*coredata.Document, error) { var ( @@ -183,7 +185,14 @@ func (s *DocumentApprovalService) BulkPublishVersions( if req.Minor { var err error - document, dv, err = s.svc.Documents.publishMinorVersionInTx(ctx, scope, tx, documentID, &req.Changelog, true) + document, dv, err = s.svc.Documents.publishMinor( + ctx, + scope, + tx, + documentID, + &req.Changelog, + true, + ) if err != nil { return fmt.Errorf("cannot publish document %q: %w", documentID, err) } @@ -199,7 +208,15 @@ func (s *DocumentApprovalService) BulkPublishVersions( approverIDs[i] = a.ApproverProfileID } - quorum, err := s.RequestApprovalInTx(ctx, scope, tx, document, dv, approverIDs, &req.Changelog) + quorum, err := s.RequestApproval( + ctx, + scope, + tx, + document, + dv, + approverIDs, + &req.Changelog, + ) if err != nil { return fmt.Errorf("cannot request approval for %q: %w", documentID, err) } @@ -208,7 +225,14 @@ func (s *DocumentApprovalService) BulkPublishVersions( } else { var err error - document, dv, err = s.svc.Documents.publishMajorVersionInTx(ctx, scope, tx, documentID, &req.Changelog, true) + document, dv, err = s.svc.Documents.publishMajor( + ctx, + scope, + tx, + documentID, + &req.Changelog, + true, + ) if err != nil { return fmt.Errorf("cannot publish document %q: %w", documentID, err) } @@ -218,8 +242,11 @@ func (s *DocumentApprovalService) BulkPublishVersions( publishedVersions = append(publishedVersions, dv) updatedDocuments = append(updatedDocuments, document) + // A direct publish emits the version-published webhook inside + // publishMinor/publishMajor; only the approval-quorum case needs + // its event emitted here. if requestedQuorum != nil { - if err := s.svc.Documents.emitDocumentEventInTx( + if err := s.svc.Documents.emitDocumentEvent( ctx, scope, tx, @@ -232,20 +259,6 @@ func (s *DocumentApprovalService) BulkPublishVersions( ); err != nil { return fmt.Errorf("cannot emit approval quorum requested webhook: %w", err) } - } else { - if err := s.svc.Documents.emitDocumentEventInTx( - ctx, - scope, - tx, - dv.DocumentID, - coredata.WebhookEventTypeDocumentVersionPublished, - dv, - nil, - nil, - nil, - ); err != nil { - return fmt.Errorf("cannot emit version published webhook: %w", err) - } } } @@ -453,7 +466,7 @@ func (s *DocumentApprovalService) Approve( return nil } - if err := s.svc.Documents.emitDocumentEventInTx( + if err := s.svc.Documents.emitDocumentEvent( ctx, scope, tx, @@ -552,7 +565,7 @@ func (s *DocumentApprovalService) Reject( return fmt.Errorf("cannot update document version status: %w", err) } - if err := s.svc.Documents.emitDocumentEventInTx( + if err := s.svc.Documents.emitDocumentEvent( ctx, scope, tx, @@ -566,7 +579,7 @@ func (s *DocumentApprovalService) Reject( return fmt.Errorf("cannot emit approval quorum rejected webhook: %w", err) } - if err := s.svc.Documents.emitDocumentEventInTx( + if err := s.svc.Documents.emitDocumentEvent( ctx, scope, tx, @@ -658,7 +671,7 @@ func (s *DocumentApprovalService) VoidApproval( return fmt.Errorf("cannot update document version status: %w", err) } - if err := s.svc.Documents.emitDocumentEventInTx( + if err := s.svc.Documents.emitDocumentEvent( ctx, scope, tx, @@ -1022,7 +1035,7 @@ func (s *DocumentApprovalService) maybeApproveQuorum( return fmt.Errorf("cannot publish version: %w", err) } - if err := s.svc.Documents.emitDocumentEventInTx( + if err := s.svc.Documents.emitDocumentEvent( ctx, scope, tx, @@ -1036,7 +1049,7 @@ func (s *DocumentApprovalService) maybeApproveQuorum( return fmt.Errorf("cannot emit approval quorum approved webhook: %w", err) } - if err := s.svc.Documents.emitDocumentEventInTx( + if err := s.svc.Documents.emitDocumentEvent( ctx, scope, tx, diff --git a/pkg/probo/document_service.go b/pkg/probo/document_service.go index c701ba669..0027998ea 100644 --- a/pkg/probo/document_service.go +++ b/pkg/probo/document_service.go @@ -580,140 +580,36 @@ func (s DocumentService) generateChangelog( return &text, nil } -// PublishVersion is the single entry point for publishing a document -// version. The behaviour depends on req.Minor and req.ApproverIDs: -// - Minor=true: publish the existing draft as a minor version. ApproverIDs -// are ignored. -// - Minor=false with ApproverIDs: open an approval quorum on the draft as -// a pending major bump (currentMajor+1.0). Result.Quorum is set. -// - Minor=false without ApproverIDs: publish the draft immediately as a -// major bump (currentMajor+1.0). -func (s *DocumentService) PublishVersion( - ctx context.Context, scope coredata.Scoper, +func (s *DocumentService) PublishVersionWithDefaultApprovers( + ctx context.Context, + scope coredata.Scoper, req PublishDocumentRequest, ) (*PublishDocumentResult, error) { - if err := req.Validate(); err != nil { - return nil, err - } - - result := &PublishDocumentResult{} + var result *PublishDocumentResult err := s.svc.pg.WithTx( ctx, func(ctx context.Context, tx pg.Tx) error { - dv := &coredata.DocumentVersion{} - if err := dv.LoadLatestVersion(ctx, tx, scope, req.DocumentID); err != nil { - return fmt.Errorf("cannot load latest version: %w", err) - } - - if dv.Status == coredata.DocumentVersionStatusPendingApproval { - return &ErrDocumentVersionPendingApproval{} - } - - if req.Minor { - document, version, err := s.publishMinorVersionInTx(ctx, scope, tx, req.DocumentID, &req.Changelog, false) - if err != nil { - return fmt.Errorf("cannot publish minor version: %w", err) + if !req.Minor { + defaultApprovers := &coredata.DocumentDefaultApprovers{} + if err := defaultApprovers.LoadByDocumentID(ctx, tx, scope, req.DocumentID); err != nil { + return fmt.Errorf("cannot load default approvers: %w", err) } - result.Document = document - result.Version = version - - if err := s.emitDocumentEventInTx( - ctx, - scope, - tx, - version.DocumentID, - coredata.WebhookEventTypeDocumentVersionPublished, - version, - nil, - nil, - nil, - ); err != nil { - return fmt.Errorf("cannot emit document version published webhook: %w", err) + approverIDs := make([]gid.GID, len(*defaultApprovers)) + for i, a := range *defaultApprovers { + approverIDs[i] = a.ApproverProfileID } - return nil + req.ApproverIDs = approverIDs } - if len(req.ApproverIDs) == 0 { - document, version, err := s.publishMajorVersionInTx(ctx, scope, tx, req.DocumentID, &req.Changelog, false) - if err != nil { - return fmt.Errorf("cannot publish major version: %w", err) - } - - result.Document = document - result.Version = version - - if err := s.emitDocumentEventInTx( - ctx, - scope, - tx, - version.DocumentID, - coredata.WebhookEventTypeDocumentVersionPublished, - version, - nil, - nil, - nil, - ); err != nil { - return fmt.Errorf("cannot emit document version published webhook: %w", err) - } - - return nil - } - - profiles := &coredata.MembershipProfiles{} - if err := profiles.LoadByIDs(ctx, tx, scope, req.ApproverIDs); err != nil { - return fmt.Errorf("cannot load approver profiles: %w", err) - } - - now := time.Now() - for _, p := range *profiles { - if p.ContractEndDate != nil && p.ContractEndDate.Before(now) { - return &ErrProfileContractEnded{ProfileID: p.ID} - } - } - - document := &coredata.Document{} - if err := document.LoadByID(ctx, tx, scope, req.DocumentID); err != nil { - return fmt.Errorf("cannot load document: %w", err) - } - - if document.ArchivedAt != nil { - return &ErrDocumentArchived{} - } - - if dv.Status != coredata.DocumentVersionStatusDraft { - return &ErrDocumentVersionNotDraft{} - } - - quorum, err := s.svc.DocumentApprovals.RequestApprovalInTx(ctx, scope, tx, document, dv, req.ApproverIDs, &req.Changelog) + publishResult, err := s.publish(ctx, scope, tx, req) if err != nil { - return fmt.Errorf("cannot request approval: %w", err) + return fmt.Errorf("cannot publish version: %w", err) } - defaultApprovers := &coredata.DocumentDefaultApprovers{} - if err := defaultApprovers.MergeByDocumentID(ctx, tx, scope, req.DocumentID, document.OrganizationID, req.ApproverIDs); err != nil { - return fmt.Errorf("cannot update default approvers: %w", err) - } - - result.Document = document - result.Version = dv - result.Quorum = quorum - - if err := s.emitDocumentEventInTx( - ctx, - scope, - tx, - dv.DocumentID, - coredata.WebhookEventTypeDocumentVersionApprovalQuorumRequested, - dv, - nil, - &quorum.ID, - nil, - ); err != nil { - return fmt.Errorf("cannot emit approval quorum requested webhook: %w", err) - } + result = publishResult return nil }, @@ -725,6 +621,161 @@ func (s *DocumentService) PublishVersion( return result, nil } +// PublishVersion publishes a document version. The behaviour depends on +// req.Minor and req.ApproverIDs: +// - Minor=true: publish the existing draft as a minor version. ApproverIDs +// are ignored. +// - Minor=false with ApproverIDs: open an approval quorum on the draft as +// a pending major bump (currentMajor+1.0). Result.Quorum is set. +// - Minor=false without ApproverIDs: publish the draft immediately as a +// major bump (currentMajor+1.0). +// +// A major publish also replaces the document's default approvers with +// req.ApproverIDs, even when the list is empty. +func (s *DocumentService) PublishVersion( + ctx context.Context, + scope coredata.Scoper, + req PublishDocumentRequest, +) (*PublishDocumentResult, error) { + var result *PublishDocumentResult + + err := s.svc.pg.WithTx( + ctx, + func(ctx context.Context, tx pg.Tx) error { + publishResult, err := s.publish(ctx, scope, tx, req) + if err != nil { + return fmt.Errorf("cannot publish version: %w", err) + } + + result = publishResult + + return nil + }, + ) + if err != nil { + return nil, err + } + + return result, nil +} + +func (s *DocumentService) publish( + ctx context.Context, + scope coredata.Scoper, + tx pg.Tx, + req PublishDocumentRequest, +) (*PublishDocumentResult, error) { + if err := req.Validate(); err != nil { + return nil, err + } + + result := &PublishDocumentResult{} + + dv := &coredata.DocumentVersion{} + if err := dv.LoadLatestVersion(ctx, tx, scope, req.DocumentID); err != nil { + return nil, fmt.Errorf("cannot load latest version: %w", err) + } + + if dv.Status == coredata.DocumentVersionStatusPendingApproval { + return nil, &ErrDocumentVersionPendingApproval{} + } + + if req.Minor { + document, version, err := s.publishMinor(ctx, scope, tx, req.DocumentID, &req.Changelog, false) + if err != nil { + return nil, fmt.Errorf("cannot publish minor version: %w", err) + } + + result.Document = document + result.Version = version + + return result, nil + } + + document := &coredata.Document{} + if err := document.LoadByID(ctx, tx, scope, req.DocumentID); err != nil { + return nil, fmt.Errorf("cannot load document: %w", err) + } + + if document.ArchivedAt != nil { + return nil, &ErrDocumentArchived{} + } + + defaultApprovers := &coredata.DocumentDefaultApprovers{} + if err := defaultApprovers.MergeByDocumentID(ctx, tx, scope, req.DocumentID, document.OrganizationID, req.ApproverIDs); err != nil { + return nil, fmt.Errorf("cannot update default approvers: %w", err) + } + + if len(req.ApproverIDs) == 0 { + publishedDocument, version, err := s.publishMajor(ctx, scope, tx, req.DocumentID, &req.Changelog, false) + if err != nil { + return nil, fmt.Errorf("cannot publish major version: %w", err) + } + + result.Document = publishedDocument + result.Version = version + + return result, nil + } + + if err := s.requestMajorApproval(ctx, scope, tx, req, document, dv, result); err != nil { + return nil, fmt.Errorf("cannot request major approval: %w", err) + } + + return result, nil +} + +func (s *DocumentService) requestMajorApproval( + ctx context.Context, + scope coredata.Scoper, + tx pg.Tx, + req PublishDocumentRequest, + document *coredata.Document, + dv *coredata.DocumentVersion, + result *PublishDocumentResult, +) error { + profiles := &coredata.MembershipProfiles{} + if err := profiles.LoadByIDs(ctx, tx, scope, req.ApproverIDs); err != nil { + return fmt.Errorf("cannot load approver profiles: %w", err) + } + + now := time.Now() + for _, p := range *profiles { + if p.ContractEndDate != nil && p.ContractEndDate.Before(now) { + return &ErrProfileContractEnded{ProfileID: p.ID} + } + } + + if dv.Status != coredata.DocumentVersionStatusDraft { + return &ErrDocumentVersionNotDraft{} + } + + quorum, err := s.svc.DocumentApprovals.RequestApproval(ctx, scope, tx, document, dv, req.ApproverIDs, &req.Changelog) + if err != nil { + return fmt.Errorf("cannot request approval: %w", err) + } + + result.Document = document + result.Version = dv + result.Quorum = quorum + + if err := s.emitDocumentEvent( + ctx, + scope, + tx, + dv.DocumentID, + coredata.WebhookEventTypeDocumentVersionApprovalQuorumRequested, + dv, + nil, + &quorum.ID, + nil, + ); err != nil { + return fmt.Errorf("cannot emit approval quorum requested webhook: %w", err) + } + + return nil +} + func (s *DocumentService) Create( ctx context.Context, scope coredata.Scoper, req CreateDocumentRequest, @@ -808,11 +859,11 @@ func (s *DocumentService) Create( } } - if err := s.emitDocumentEventInTx(ctx, scope, conn, documentID, coredata.WebhookEventTypeDocumentCreated, nil, nil, nil, nil); err != nil { + if err := s.emitDocumentEvent(ctx, scope, conn, documentID, coredata.WebhookEventTypeDocumentCreated, nil, nil, nil, nil); err != nil { return fmt.Errorf("cannot emit document created webhook: %w", err) } - if err := s.emitDocumentEventInTx( + if err := s.emitDocumentEvent( ctx, scope, conn, @@ -988,7 +1039,7 @@ func (s *DocumentService) SignDocumentVersionByIdentity( return fmt.Errorf("cannot update document version signature: %w", err) } - if err := s.emitDocumentEventInTx( + if err := s.emitDocumentEvent( ctx, scope, tx, @@ -1255,7 +1306,7 @@ func (s *DocumentService) RequestSignature( return nil } - if err := s.emitDocumentEventInTx( + if err := s.emitDocumentEvent( ctx, scope, tx, @@ -1377,7 +1428,7 @@ func (s *DocumentService) deleteDraftInTx( // For deletion events this must be called before the document is soft-deleted, // since Document.LoadByID filters out soft-deleted rows. -func (s *DocumentService) emitDocumentEventInTx( +func (s *DocumentService) emitDocumentEvent( ctx context.Context, scope coredata.Scoper, tx pg.Tx, documentID gid.GID, @@ -1543,7 +1594,7 @@ func (s *DocumentService) SoftDelete( return s.svc.pg.WithTx( ctx, func(ctx context.Context, tx pg.Tx) error { - if err := s.emitDocumentEventInTx(ctx, scope, tx, documentID, coredata.WebhookEventTypeDocumentDeleted, nil, nil, nil, nil); err != nil { + if err := s.emitDocumentEvent(ctx, scope, tx, documentID, coredata.WebhookEventTypeDocumentDeleted, nil, nil, nil, nil); err != nil { return fmt.Errorf("cannot emit document deleted webhook: %w", err) } @@ -2239,7 +2290,7 @@ func (s *DocumentService) Update( } if versionDeleted { - if err := s.emitDocumentEventInTx( + if err := s.emitDocumentEvent( ctx, scope, tx, @@ -2259,14 +2310,14 @@ func (s *DocumentService) Update( versionPrevious = nil } - if err := s.emitDocumentEventInTx(ctx, scope, tx, resultVersion.DocumentID, versionEvent, resultVersion, nil, nil, versionPrevious); err != nil { + if err := s.emitDocumentEvent(ctx, scope, tx, resultVersion.DocumentID, versionEvent, resultVersion, nil, nil, versionPrevious); err != nil { return fmt.Errorf("cannot emit document version webhook: %w", err) } } } if docLevelChanged { - if err := s.emitDocumentEventInTx(ctx, scope, tx, req.DocumentID, coredata.WebhookEventTypeDocumentUpdated, nil, nil, nil, webhooktypes.NewDocument(&previousDocument)); err != nil { + if err := s.emitDocumentEvent(ctx, scope, tx, req.DocumentID, coredata.WebhookEventTypeDocumentUpdated, nil, nil, nil, webhooktypes.NewDocument(&previousDocument)); err != nil { return fmt.Errorf("cannot emit document updated webhook: %w", err) } } @@ -2315,7 +2366,7 @@ func (s *DocumentService) DeleteDraft( return err } - if err := s.emitDocumentEventInTx( + if err := s.emitDocumentEvent( ctx, scope, tx, @@ -2385,7 +2436,7 @@ func (s *DocumentService) Archive( return fmt.Errorf("cannot archive document: %w", err) } - if err := s.emitDocumentEventInTx(ctx, scope, tx, documentID, coredata.WebhookEventTypeDocumentArchived, nil, nil, nil, nil); err != nil { + if err := s.emitDocumentEvent(ctx, scope, tx, documentID, coredata.WebhookEventTypeDocumentArchived, nil, nil, nil, nil); err != nil { return fmt.Errorf("cannot emit document archived webhook: %w", err) } @@ -2425,7 +2476,7 @@ func (s *DocumentService) Unarchive( return fmt.Errorf("cannot unarchive document: %w", err) } - if err := s.emitDocumentEventInTx(ctx, scope, tx, documentID, coredata.WebhookEventTypeDocumentUnarchived, nil, nil, nil, nil); err != nil { + if err := s.emitDocumentEvent(ctx, scope, tx, documentID, coredata.WebhookEventTypeDocumentUnarchived, nil, nil, nil, nil); err != nil { return fmt.Errorf("cannot emit document unarchived webhook: %w", err) } @@ -2477,7 +2528,7 @@ func (s *DocumentService) CancelSignatureRequest( return fmt.Errorf("cannot delete document version signature: %w", err) } - if err := s.emitDocumentEventInTx( + if err := s.emitDocumentEvent( ctx, scope, tx, @@ -3199,8 +3250,13 @@ func (s *DocumentService) finalizePublish( return nil } -func (s *DocumentService) publishMajorVersionInTx( - ctx context.Context, scope coredata.Scoper, +// publishMajor publishes the document's draft as a new major version +// (currentMajor+1.0) and emits the version-published webhook. ignoreExisting is +// set by bulk publish to treat an already-published version as an idempotent +// no-op. +func (s *DocumentService) publishMajor( + ctx context.Context, + scope coredata.Scoper, tx pg.Tx, documentID gid.GID, changelog *string, @@ -3238,6 +3294,20 @@ func (s *DocumentService) publishMajorVersionInTx( return nil, nil, err } + if err := s.emitDocumentEvent( + ctx, + scope, + tx, + documentVersion.DocumentID, + coredata.WebhookEventTypeDocumentVersionPublished, + documentVersion, + nil, + nil, + nil, + ); err != nil { + return nil, nil, fmt.Errorf("cannot emit document version published webhook: %w", err) + } + return document, documentVersion, nil } @@ -3260,8 +3330,12 @@ func (s *DocumentService) cancelPreviousMajorSignatureRequestsInTx( return nil } -func (s *DocumentService) publishMinorVersionInTx( - ctx context.Context, scope coredata.Scoper, +// publishMinor publishes the document's draft as a minor version and emits the +// version-published webhook. ignoreExisting is set by bulk publish to treat an +// already-published version as an idempotent no-op. +func (s *DocumentService) publishMinor( + ctx context.Context, + scope coredata.Scoper, tx pg.Tx, documentID gid.GID, changelog *string, @@ -3287,6 +3361,20 @@ func (s *DocumentService) publishMinorVersionInTx( return nil, nil, err } + if err := s.emitDocumentEvent( + ctx, + scope, + tx, + documentVersion.DocumentID, + coredata.WebhookEventTypeDocumentVersionPublished, + documentVersion, + nil, + nil, + nil, + ); err != nil { + return nil, nil, fmt.Errorf("cannot emit document version published webhook: %w", err) + } + return document, documentVersion, nil } diff --git a/pkg/probo/generated_document_service.go b/pkg/probo/generated_document_service.go index 229ba2263..9b2c4cbcb 100644 --- a/pkg/probo/generated_document_service.go +++ b/pkg/probo/generated_document_service.go @@ -3209,7 +3209,8 @@ func formatRiskTreatment(t coredata.RiskTreatment) string { // minor is false a non-empty approverIDs triggers an approval request at // (currentMajor+1).0; otherwise the version is published at (currentMajor+1).0. func (s *GeneratedDocumentService) publishOrRequestApproval( - ctx context.Context, scope coredata.Scoper, + ctx context.Context, + scope coredata.Scoper, tx pg.Tx, document *coredata.Document, version *coredata.DocumentVersion, @@ -3290,18 +3291,18 @@ func (s *GeneratedDocumentService) publishOrRequestApproval( return fmt.Errorf("cannot save default approvers: %w", err) } - quorum, err := s.svc.DocumentApprovals.RequestApprovalInTx(ctx, scope, tx, document, version, approverIDs, nil) + quorum, err := s.svc.DocumentApprovals.RequestApproval(ctx, scope, tx, document, version, approverIDs, nil) if err != nil { return fmt.Errorf("cannot request approval: %w", err) } if isFirstVersion { - if err := s.svc.Documents.emitDocumentEventInTx(ctx, scope, tx, document.ID, coredata.WebhookEventTypeDocumentCreated, nil, nil, nil, nil); err != nil { + if err := s.svc.Documents.emitDocumentEvent(ctx, scope, tx, document.ID, coredata.WebhookEventTypeDocumentCreated, nil, nil, nil, nil); err != nil { return fmt.Errorf("cannot emit document created webhook: %w", err) } } - if err := s.svc.Documents.emitDocumentEventInTx( + if err := s.svc.Documents.emitDocumentEvent( ctx, scope, tx, @@ -3333,12 +3334,12 @@ func (s *GeneratedDocumentService) publishOrRequestApproval( } if isFirstVersion { - if err := s.svc.Documents.emitDocumentEventInTx(ctx, scope, tx, document.ID, coredata.WebhookEventTypeDocumentCreated, nil, nil, nil, nil); err != nil { + if err := s.svc.Documents.emitDocumentEvent(ctx, scope, tx, document.ID, coredata.WebhookEventTypeDocumentCreated, nil, nil, nil, nil); err != nil { return fmt.Errorf("cannot emit document created webhook: %w", err) } } - if err := s.svc.Documents.emitDocumentEventInTx( + if err := s.svc.Documents.emitDocumentEvent( ctx, scope, tx, diff --git a/pkg/probo/oauth2_scopes.go b/pkg/probo/oauth2_scopes.go index 866afb69b..ddb77586d 100644 --- a/pkg/probo/oauth2_scopes.go +++ b/pkg/probo/oauth2_scopes.go @@ -307,7 +307,6 @@ var OAuth2ScopeMappings = map[coredata.OAuth2Scope][]string{ ActionDocumentUnarchive, ActionDocumentDeleteDraft, ActionDocumentVersionSign, - ActionDocumentVersionRequestApproval, ActionDocumentVersionVoidApproval, ActionDocumentVersionApprove, ActionDocumentVersionReject, diff --git a/pkg/server/api/console/v1/document_resolvers.go b/pkg/server/api/console/v1/document_resolvers.go index b8c77fd49..b9b8a08da 100644 --- a/pkg/server/api/console/v1/document_resolvers.go +++ b/pkg/server/api/console/v1/document_resolvers.go @@ -1036,12 +1036,7 @@ func (r *mutationResolver) DeleteDocument(ctx context.Context, input types.Delet // PublishDocument is the resolver for the publishDocument field. func (r *mutationResolver) PublishDocument(ctx context.Context, input types.PublishDocumentInput) (*types.PublishDocumentPayload, error) { - action := probo.ActionDocumentVersionPublish - if !input.Minor && len(input.ApproverIds) > 0 { - action = probo.ActionDocumentVersionRequestApproval - } - - scope, err := r.authorize(ctx, input.DocumentID, action) + scope, err := r.authorize(ctx, input.DocumentID, probo.ActionDocumentVersionPublish) if err != nil { return nil, err } diff --git a/pkg/server/api/mcp/v1/schema.resolvers.go b/pkg/server/api/mcp/v1/schema.resolvers.go index d0c166382..4e7239f2b 100644 --- a/pkg/server/api/mcp/v1/schema.resolvers.go +++ b/pkg/server/api/mcp/v1/schema.resolvers.go @@ -6084,23 +6084,17 @@ func (r *Resolver) ListSCIMEventsTool(ctx context.Context, req *mcp.CallToolRequ } func (r *Resolver) PublishDocumentTool(ctx context.Context, req *mcp.CallToolRequest, input *types.PublishDocumentInput) (*mcp.CallToolResult, types.PublishDocumentOutput, error) { - action := probo.ActionDocumentVersionPublish - if !input.Minor && len(input.ApproverIds) > 0 { - action = probo.ActionDocumentVersionRequestApproval - } - - scope, err := r.Authorize(ctx, input.DocumentID, action) + scope, err := r.Authorize(ctx, input.DocumentID, probo.ActionDocumentVersionPublish) if err != nil { return nil, types.PublishDocumentOutput{}, err } svc := r.proboSvc - result, err := svc.Documents.PublishVersion(ctx, scope, probo.PublishDocumentRequest{ - DocumentID: input.DocumentID, - Minor: input.Minor, - ApproverIDs: input.ApproverIds, - Changelog: input.Changelog, + result, err := svc.Documents.PublishVersionWithDefaultApprovers(ctx, scope, probo.PublishDocumentRequest{ + DocumentID: input.DocumentID, + Minor: input.Minor, + Changelog: input.Changelog, }) if err != nil { panic(fmt.Errorf("cannot publish document: %w", err)) diff --git a/pkg/server/api/mcp/v1/specification.yaml b/pkg/server/api/mcp/v1/specification.yaml index 2a7280f50..f522b27cf 100644 --- a/pkg/server/api/mcp/v1/specification.yaml +++ b/pkg/server/api/mcp/v1/specification.yaml @@ -6305,12 +6305,7 @@ components: description: Document ID minor: type: boolean - description: When true, publish the draft as a minor version; approver_ids must be omitted. When false, publish as a new major version; approver_ids is required (a non-empty list requests approval, an empty list publishes directly without approval). - approver_ids: - type: array - items: - $ref: "#/components/schemas/GID" - description: "Approver profile IDs. Required when minor is false: pass a non-empty list to request approval (approvers receive an email notification), or an empty list to publish directly without approval. Must be omitted when minor is true." + description: When true, publish the draft as a minor version (the document must already have a published major version). When false, publish a new major version using the document's default approvers; if the document has default approvers an approval is requested from them (they receive an email notification), otherwise the version is published directly. Configure a document's default approvers with updateDocument. changelog: type: string description: Changelog for this version @@ -6328,7 +6323,7 @@ components: $ref: "#/components/schemas/DocumentVersion" approval_quorum: $ref: "#/components/schemas/DocumentVersionApprovalQuorum" - description: Set when an approval was requested instead of publishing. + description: Set when an approval was requested from the document's default approvers instead of publishing directly. DeleteDocumentInput: type: object