diff --git a/apps/console/src/pages/organizations/documents/_components/PublishDialog.tsx b/apps/console/src/pages/organizations/documents/_components/PublishDialog.tsx index 1b979954a..7ba319f9a 100644 --- a/apps/console/src/pages/organizations/documents/_components/PublishDialog.tsx +++ b/apps/console/src/pages/organizations/documents/_components/PublishDialog.tsx @@ -153,7 +153,7 @@ export function PublishDialog({ input: { documentId, minor, - approverIds: minor ? [] : data.approverIds, + approverIds: minor ? null : data.approverIds, changelog: data.changelog, }, }, diff --git a/e2e/console/document_version_test.go b/e2e/console/document_version_test.go index f8685e47b..22764a061 100644 --- a/e2e/console/document_version_test.go +++ b/e2e/console/document_version_test.go @@ -190,9 +190,10 @@ func publishMajorDocumentVersion(t *testing.T, owner *testutil.Client, docID str } `, map[string]any{ "input": map[string]any{ - "minor": false, - "documentId": docID, - "changelog": "Major release", + "minor": false, + "documentId": docID, + "approverIds": []string{}, + "changelog": "Major release", }, }, &result) require.NoError(t, err) @@ -1425,9 +1426,10 @@ func TestDocumentVersion_PublishBlockedWhenPendingApproval(t *testing.T) { } `, map[string]any{ "input": map[string]any{ - "minor": false, - "documentId": docID, - "changelog": "Major release", + "minor": false, + "documentId": docID, + "approverIds": []string{}, + "changelog": "Major release", }, }) require.Error(t, err) @@ -1453,6 +1455,128 @@ func TestDocumentVersion_PublishBlockedWhenPendingApproval(t *testing.T) { }) } +// TestDocumentVersion_PublishApproverIDsContract verifies that approver_ids must +// be an explicit choice when publishing: required for a major version (empty +// list publishes directly, non-empty requests approval) and rejected for a minor +// version (which ignores approvers). +func TestDocumentVersion_PublishApproverIDsContract(t *testing.T) { + t.Parallel() + + const publishMutation = ` + mutation($input: PublishDocumentInput!) { + publishDocument(input: $input) { + documentVersion { status } + approvalQuorum { id } + } + } + ` + + type publishResult struct { + PublishDocument struct { + DocumentVersion struct { + Status string `json:"status"` + } `json:"documentVersion"` + ApprovalQuorum *struct { + ID string `json:"id"` + } `json:"approvalQuorum"` + } `json:"publishDocument"` + } + + t.Run("major publish without approver_ids is rejected", func(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + docID, _ := createTestDocument(t, owner) + + _, err := owner.Do(publishMutation, map[string]any{ + "input": map[string]any{ + "minor": false, + "documentId": docID, + "changelog": "Major release", + }, + }) + require.Error(t, err) + }) + + t.Run("major publish with empty approver_ids publishes directly", func(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + docID, _ := createTestDocument(t, owner) + + var result publishResult + + err := owner.Execute(publishMutation, map[string]any{ + "input": map[string]any{ + "minor": false, + "documentId": docID, + "approverIds": []string{}, + "changelog": "Direct major release", + }, + }, &result) + require.NoError(t, err) + assert.Equal(t, "PUBLISHED", result.PublishDocument.DocumentVersion.Status) + assert.Nil(t, result.PublishDocument.ApprovalQuorum) + }) + + t.Run("major publish with approver_ids requests approval", func(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + docID, _ := createTestDocument(t, owner) + + var result publishResult + + err := owner.Execute(publishMutation, map[string]any{ + "input": map[string]any{ + "minor": false, + "documentId": docID, + "approverIds": []string{getOwnerProfileID(t, owner)}, + "changelog": "Major release needing approval", + }, + }, &result) + require.NoError(t, err) + assert.Equal(t, "PENDING_APPROVAL", result.PublishDocument.DocumentVersion.Status) + require.NotNil(t, result.PublishDocument.ApprovalQuorum) + }) + + t.Run("minor publish without approver_ids is allowed", func(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + docID, _ := createTestDocument(t, owner) + publishMajorDocumentVersion(t, owner, docID) + updateDocumentContent(t, owner, docID, "Updated content for minor") + + var result publishResult + + err := owner.Execute(publishMutation, map[string]any{ + "input": map[string]any{ + "minor": true, + "documentId": docID, + "changelog": "Minor release", + }, + }, &result) + require.NoError(t, err) + assert.Equal(t, "PUBLISHED", result.PublishDocument.DocumentVersion.Status) + assert.Nil(t, result.PublishDocument.ApprovalQuorum) + }) + + t.Run("minor publish with approver_ids is rejected", func(t *testing.T) { + t.Parallel() + owner := testutil.NewClient(t, testutil.RoleOwner) + docID, _ := createTestDocument(t, owner) + publishMajorDocumentVersion(t, owner, docID) + updateDocumentContent(t, owner, docID, "Updated content for rejected minor") + + _, err := owner.Do(publishMutation, map[string]any{ + "input": map[string]any{ + "minor": true, + "documentId": docID, + "approverIds": []string{getOwnerProfileID(t, owner)}, + "changelog": "Minor release", + }, + }) + require.Error(t, err) + }) +} + func TestDocument_DefaultApprovers(t *testing.T) { t.Parallel() owner := testutil.NewClient(t, testutil.RoleOwner) diff --git a/packages/n8n-node/nodes/Probo/actions/document/publish.operation.ts b/packages/n8n-node/nodes/Probo/actions/document/publish.operation.ts index 1b1979c6b..31c5115d8 100644 --- a/packages/n8n-node/nodes/Probo/actions/document/publish.operation.ts +++ b/packages/n8n-node/nodes/Probo/actions/document/publish.operation.ts @@ -55,7 +55,7 @@ export const description: INodeProperties[] = [ }, }, default: '', - description: 'Comma-separated list of approver profile IDs. When provided, an approval is requested instead of publishing immediately.', + description: 'Comma-separated list of approver profile IDs. Provide IDs to request approval; leave empty to publish the major version directly without approval.', }, { displayName: 'Changelog', @@ -122,12 +122,14 @@ export async function execute( `; const input: Record = { documentId, minor, changelog }; - if (!minor && approverIdsRaw) { - const approverIds = approverIdsRaw + if (!minor) { + // A major publish must set approverIds explicitly: an empty list publishes + // directly without approval, a non-empty list requests approval. It must be + // omitted for a minor publish, which ignores approvers. + input.approverIds = approverIdsRaw .split(',') .map(id => id.trim()) .filter(Boolean); - if (approverIds.length > 0) input.approverIds = approverIds; } const responseData = await proboApiRequest.call(this, query, { input }); diff --git a/pkg/probo/document_service.go b/pkg/probo/document_service.go index b8932a306..6bf5a5b76 100644 --- a/pkg/probo/document_service.go +++ b/pkg/probo/document_service.go @@ -184,6 +184,27 @@ func (req *PublishDocumentRequest) Validate() error { }) v.Check(req.Changelog, "changelog", validator.Required(), validator.SafeText(5000)) + // approver_ids must be an explicit choice for a major publish (an empty list + // publishes directly without approval, a non-empty list requests approval) + // and must be omitted for a minor publish, which ignores approvers. + if req.Minor && req.ApproverIDs != nil { + v.Check(req.ApproverIDs, "approver_ids", func(any) *validator.ValidationError { + return &validator.ValidationError{ + Code: validator.ErrorCodeCustom, + Message: "must not be set when publishing a minor version", + } + }) + } + + if !req.Minor && req.ApproverIDs == nil { + v.Check(req.ApproverIDs, "approver_ids", func(any) *validator.ValidationError { + return &validator.ValidationError{ + Code: validator.ErrorCodeCustom, + Message: "must be set when publishing a major version: provide approver profile IDs to request approval, or an empty list to publish directly without approval", + } + }) + } + return v.Error() } diff --git a/pkg/server/api/mcp/v1/specification.yaml b/pkg/server/api/mcp/v1/specification.yaml index d4193e56e..348b70670 100644 --- a/pkg/server/api/mcp/v1/specification.yaml +++ b/pkg/server/api/mcp/v1/specification.yaml @@ -6305,12 +6305,12 @@ components: description: Document ID minor: type: boolean - description: When true, publish the draft as a minor version and ignore approver_ids. When false, publish as a new major version; if approver_ids are provided, an approval is requested instead. + 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 (ignored when minor is true) + 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." changelog: type: string description: Changelog for this version