diff --git a/apps/remix/app/components/general/document-signing/envelope-signing-provider.tsx b/apps/remix/app/components/general/document-signing/envelope-signing-provider.tsx index d77d41c1f..631123b6a 100644 --- a/apps/remix/app/components/general/document-signing/envelope-signing-provider.tsx +++ b/apps/remix/app/components/general/document-signing/envelope-signing-provider.tsx @@ -6,7 +6,8 @@ import type { EnvelopeForSigningResponse } from '@documenso/lib/server-only/enve import type { TRecipientActionAuth } from '@documenso/lib/types/document-auth'; import { isFieldUnsignedAndRequired, isRequiredField } from '@documenso/lib/utils/advanced-fields-helpers'; import { extractFieldInsertionValues } from '@documenso/lib/utils/envelope-signing'; -import { effectiveSigningOrder, getNextDictatableRecipient } from '@documenso/lib/utils/recipient-groups'; +import { getNextDictatableRecipient } from '@documenso/lib/utils/recipient-groups'; +import { isRecipientBefore } from '@documenso/lib/utils/recipients'; import { trpc } from '@documenso/trpc/react'; import type { TSignEnvelopeFieldValue } from '@documenso/trpc/server/envelope-router/sign-envelope-field.types'; import { EnvelopeType, type Field, FieldType, type Recipient, RecipientRole, SigningStatus } from '@prisma/client'; @@ -237,14 +238,15 @@ export const EnvelopeSigningProvider = ({ }, [envelopeData.recipient.fields]); /** - * Assistant recipients are those that have a signing order after the assistant. + * Assistant recipients are those positioned strictly after the assistant — + * never their own group peers. */ const assistantRecipients = useMemo(() => { if (recipient.role !== RecipientRole.ASSISTANT) { return []; } - return envelope.recipients.filter((r) => effectiveSigningOrder(r) > effectiveSigningOrder(recipient)); + return envelope.recipients.filter((r) => isRecipientBefore(recipient, r)); }, [envelope.recipients, recipient]); /** diff --git a/apps/remix/app/components/general/envelope-editor/envelope-editor-recipient-form.tsx b/apps/remix/app/components/general/envelope-editor/envelope-editor-recipient-form.tsx index ac56a9c3a..fbe513120 100644 --- a/apps/remix/app/components/general/envelope-editor/envelope-editor-recipient-form.tsx +++ b/apps/remix/app/components/general/envelope-editor/envelope-editor-recipient-form.tsx @@ -9,7 +9,7 @@ import { useOptionalSession } from '@documenso/lib/client-only/providers/session import type { TDetectedRecipientSchema } from '@documenso/lib/server-only/ai/envelope/detect-recipients/schema'; import { ZRecipientAuthOptionsSchema } from '@documenso/lib/types/document-auth'; import { nanoid } from '@documenso/lib/universal/id'; -import { groupRecipientsBySigningOrder, normalizeGroupedSigningOrders } from '@documenso/lib/utils/recipient-groups'; +import { normalizeGroupedSigningOrders } from '@documenso/lib/utils/recipient-groups'; import { canEditorRecipientBeModified } from '@documenso/lib/utils/recipients'; import { cn } from '@documenso/ui/lib/utils'; import { Alert, AlertDescription } from '@documenso/ui/primitives/alert'; @@ -146,8 +146,6 @@ export const EnvelopeEditorRecipientForm = () => { name: 'signers', }); - const stepCount = useMemo(() => groupRecipientsBySigningOrder(watchedSigners).steps.length, [watchedSigners]); - const emptySignerIndex = watchedSigners.findIndex( (signer) => !signer.name && !signer.email && envelope.fields.filter((field) => field.recipientId === signer.id).length === 0, @@ -192,16 +190,13 @@ export const EnvelopeEditorRecipientForm = () => { email: '', role: RecipientRole.SIGNER, actionAuth: [], - signingOrder: stepCount + 1, + signingOrder: undefined, }); }; const onAiDetectionComplete = (detectedRecipients: TDetectedRecipientSchema[]) => { const currentSigners = form.getValues('signers'); - let nextSigningOrder = - currentSigners.length > 0 ? Math.max(...currentSigners.map((s) => s.signingOrder ?? 0)) + 1 : 1; - // If the only signer is the default empty signer lets just replace it with the detected recipients if (currentSigners.length === 1 && !currentSigners[0].name && !currentSigners[0].email) { updateEditorSigners( @@ -234,10 +229,8 @@ export const EnvelopeEditorRecipientForm = () => { email: recipient.email, role: recipient.role, actionAuth: [], - signingOrder: nextSigningOrder, + signingOrder: undefined, }); - - nextSigningOrder += 1; } updateEditorSigners(form, normalizeSigningOrders(currentSigners)); @@ -274,7 +267,7 @@ export const EnvelopeEditorRecipientForm = () => { email: currentEditorEmail ?? '', role: RecipientRole.SIGNER, actionAuth: [], - signingOrder: stepCount + 1, + signingOrder: undefined, }, true, ); diff --git a/apps/remix/app/components/general/envelope-editor/recipient-step-card.tsx b/apps/remix/app/components/general/envelope-editor/recipient-step-card.tsx index cb625b561..579684dea 100644 --- a/apps/remix/app/components/general/envelope-editor/recipient-step-card.tsx +++ b/apps/remix/app/components/general/envelope-editor/recipient-step-card.tsx @@ -126,6 +126,8 @@ export const RecipientStepCard = ({ const isGroup = step.members.length > 1; const isCombineTarget = draggingType === 'STEP' && Boolean(draggableSnapshot.combineTargetFor); + const stepLabel = step.order ?? stepIndex + 1; + // All droppable ids are anchored to the first member's formId (never a // positional index) so they stay stable while cards are reordered — // @hello-pangea/dnd does not support changing ids on mounted elements. @@ -182,7 +184,7 @@ export const RecipientStepCard = ({ - Group {step.order} + Group {stepLabel} {isGroup && ( diff --git a/apps/remix/app/components/general/envelope-editor/recipient-step-list.tsx b/apps/remix/app/components/general/envelope-editor/recipient-step-list.tsx index b436cab83..9c4f69ff3 100644 --- a/apps/remix/app/components/general/envelope-editor/recipient-step-list.tsx +++ b/apps/remix/app/components/general/envelope-editor/recipient-step-list.tsx @@ -8,6 +8,7 @@ import { extractRecipientToNewStep, getLastLockedStepIndex, groupRecipientsBySigningOrder, + isSigningOrderFrozen, mergeSteps, moveRecipientToStep, normalizeGroupedSigningOrders, @@ -82,6 +83,11 @@ export const RecipientStepList = ({ showAdvancedSettings }: RecipientStepListPro [steps, envelope], ); + const isOrderingFrozen = useMemo( + () => isSigningOrderFrozen(steps, (signer) => canEditorRecipientBeModified(envelope, signer.id)), + [steps, envelope], + ); + const isRemoveDisabled = watchedSigners.length === 1; const flatIndexByFormId = useMemo( @@ -326,7 +332,7 @@ export const RecipientStepList = ({ showAdvancedSettings }: RecipientStepListPro {(provided) => (
{steps.map((step, stepIndex) => { - const isStepLocked = stepIndex <= lastLockedStepIndex; + const isStepLocked = isOrderingFrozen || stepIndex <= lastLockedStepIndex; return ( { +test('[TSP_GROUPING]: numbers recipients created without a signing order on a QES envelope', async ({ request }) => { const { envelopeId, token } = await seedEnvelopeAtSignatureLevel(request, 'QES'); - // Both land in the same tail step, so they would sign in parallel. - const response = await createRecipients(request, token, envelopeId, [{}, {}]); + const response = await createRecipients(request, token, envelopeId, [{}, { signingOrder: 3 }, {}]); - expect(response.status()).toBe(400); + expect(response.ok(), await response.text()).toBeTruthy(); - const recipients = await prisma.recipient.findMany({ where: { envelopeId } }); + const recipients = await prisma.recipient.findMany({ where: { envelopeId }, orderBy: { id: 'asc' } }); - expect(recipients).toHaveLength(0); + expect(recipients.map((recipient) => recipient.signingOrder)).toEqual([4, 3, 5]); + + const second = await createRecipients(request, token, envelopeId, [{}]); + + expect(second.ok(), await second.text()).toBeTruthy(); + + const recipientsAfter = await prisma.recipient.findMany({ where: { envelopeId }, orderBy: { id: 'asc' } }); + + expect(recipientsAfter.map((recipient) => recipient.signingOrder)).toEqual([4, 3, 5, 6]); }); test('[TSP_GROUPING]: accepts distinct signing orders on an AES envelope', async ({ request }) => { diff --git a/packages/app-tests/e2e/document-auth/assistant-null-order.spec.ts b/packages/app-tests/e2e/document-auth/assistant-null-order.spec.ts index 2fd523a96..b7eb5597c 100644 --- a/packages/app-tests/e2e/document-auth/assistant-null-order.spec.ts +++ b/packages/app-tests/e2e/document-auth/assistant-null-order.spec.ts @@ -12,18 +12,10 @@ import { DocumentSigningOrder, FieldType, RecipientRole } from '@prisma/client'; const WEBAPP_BASE_URL = NEXT_PUBLIC_WEBAPP_URL(); /** - * A recipient without a signing order sits in the LAST step — the convention - * `effectiveSigningOrder` encodes and the server sorts by (NULLS LAST). The - * assistant scoping filters must agree with it: - * - * - an ordered assistant may assist a null-order recipient (they are in the - * strictly later tail step), and - * - a null-order assistant may assist NOBODY (nobody comes after the last - * step) — historically `signingOrder ?? 0` treated them as FIRST, letting - * their token prefill every ordered recipient's fields. - * - * Null orders are only produced via the API, which is why no editor-driven - * test covers this. + * Assistant scoping must follow the same position model as signing (numbered + * first, then unordered by id). Historically `signingOrder ?? 0` treated an + * unordered assistant as FIRST, letting their token prefill every ordered + * recipient's fields. */ const seedAssistantDocument = async (options: { @@ -117,18 +109,20 @@ test('[ASSISTANT_NULL_ORDER]: an ordered assistant can assist a null-order (tail expect(fieldAfter.inserted).toBe(true); }); -test('[ASSISTANT_NULL_ORDER]: a null-order (tail-step) assistant cannot assist anyone', async () => { - const { assistant, signer, signerTextField } = await seedAssistantDocument({ +test('[ASSISTANT_NULL_ORDER]: a null-order assistant cannot assist an ordered recipient', async () => { + const { assistant, signerTextField } = await seedAssistantDocument({ assistantOrder: null, signerOrder: 1, }); - // The null-order assistant sits in the last step: nobody comes after them. const assistableRecipients = await getRecipientsForAssistant({ token: assistant.token }); expect(assistableRecipients.map((recipient) => recipient.id)).toEqual([assistant.id]); - // Every ordered recipient is in an EARLIER step — prefilling must fail. + const fields = await getFieldsForToken({ token: assistant.token }); + + expect(fields.map((field) => field.id)).not.toContain(signerTextField.id); + await expect( signFieldWithToken({ token: assistant.token, @@ -140,7 +134,77 @@ test('[ASSISTANT_NULL_ORDER]: a null-order (tail-step) assistant cannot assist a const fieldAfter = await prisma.field.findUniqueOrThrow({ where: { id: signerTextField.id } }); expect(fieldAfter.inserted).toBe(false); - expect(fieldAfter.id).not.toBe(signer.id); // sanity: distinct entities +}); + +test('[ASSISTANT_NULL_ORDER]: a null-order assistant can assist a null-order recipient created after them', async () => { + const { assistant, signer, signerTextField } = await seedAssistantDocument({ + assistantOrder: null, + signerOrder: null, + }); + + const assistableRecipients = await getRecipientsForAssistant({ token: assistant.token }); + + expect(assistableRecipients.map((recipient) => recipient.id)).toEqual([assistant.id, signer.id]); + + const fields = await getFieldsForToken({ token: assistant.token }); + + expect(fields.map((field) => field.id)).toContain(signerTextField.id); + + await signFieldWithToken({ + token: assistant.token, + fieldId: signerTextField.id, + value: 'TEXT', + }); + + const fieldAfter = await prisma.field.findUniqueOrThrow({ where: { id: signerTextField.id } }); + + expect(fieldAfter.inserted).toBe(true); +}); + +test('[ASSISTANT_NULL_ORDER]: a null-order recipient cannot be assisted by a null-order assistant created after them', async () => { + const { user, team } = await seedUser(); + const { user: signerUser } = await seedUser(); + const { user: assistantUser } = await seedUser(); + + const { recipients } = await seedPendingDocumentWithFullFields({ + owner: user, + teamId: team.id, + recipients: [signerUser, assistantUser], + recipientsCreateOptions: [ + { signingOrder: null, role: RecipientRole.SIGNER }, + { signingOrder: null, role: RecipientRole.ASSISTANT }, + ], + fields: [FieldType.TEXT], + updateDocumentOptions: { + documentMeta: { + upsert: { + create: { signingOrder: DocumentSigningOrder.SEQUENTIAL }, + update: { signingOrder: DocumentSigningOrder.SEQUENTIAL }, + }, + }, + }, + }); + + const assistant = recipients.find((recipient) => recipient.role === RecipientRole.ASSISTANT); + const signerTextField = recipients + .find((recipient) => recipient.role === RecipientRole.SIGNER) + ?.fields.find((field) => field.type === FieldType.TEXT); + + if (!assistant || !signerTextField) { + throw new Error('Seeded recipients not found'); + } + + const assistableRecipients = await getRecipientsForAssistant({ token: assistant.token }); + + expect(assistableRecipients.map((recipient) => recipient.id)).toEqual([assistant.id]); + + await expect( + signFieldWithToken({ + token: assistant.token, + fieldId: signerTextField.id, + value: 'TEXT', + }), + ).rejects.toThrow(); }); test('[ASSISTANT_NULL_ORDER]: V2 route allows an ordered assistant to prefill a null-order recipient', async ({ diff --git a/packages/app-tests/e2e/envelope-editor-v2/envelope-recipient-legacy-unordered-tail.spec.ts b/packages/app-tests/e2e/envelope-editor-v2/envelope-recipient-legacy-unordered-tail.spec.ts new file mode 100644 index 000000000..c8dea6559 --- /dev/null +++ b/packages/app-tests/e2e/envelope-editor-v2/envelope-recipient-legacy-unordered-tail.spec.ts @@ -0,0 +1,81 @@ +import { prisma } from '@documenso/prisma'; +import { seedPendingDocumentWithFullFields } from '@documenso/prisma/seed/documents'; +import { seedUser } from '@documenso/prisma/seed/users'; +import { expect, test } from '@playwright/test'; +import { DocumentSigningOrder, SendStatus, SigningStatus } from '@prisma/client'; + +import { apiSignin } from '../fixtures/authentication'; +import { + clickAddSignerButton, + getRecipientEmailInputs, + getRecipientStepCards, + setRecipientEmail, +} from '../fixtures/envelope-editor'; + +/** + * Numbers sort ahead of unordered rows, so numbering anything behind a signed + * unordered recipient would move it in front of them. + */ + +test('[LEGACY_UNORDERED_TAIL]: additions queue behind a locked unordered recipient', async ({ page }) => { + const { user, team } = await seedUser(); + const { user: alice } = await seedUser(); + const { user: bob } = await seedUser(); + + const { document } = await seedPendingDocumentWithFullFields({ + owner: user, + teamId: team.id, + recipients: [alice, bob], + recipientsCreateOptions: [ + { signingOrder: null, signingStatus: SigningStatus.SIGNED, sendStatus: SendStatus.SENT }, + { signingOrder: null, signingStatus: SigningStatus.NOT_SIGNED, sendStatus: SendStatus.NOT_SENT }, + ], + fields: [], + updateDocumentOptions: { + internalVersion: 2, + documentMeta: { + upsert: { + create: { signingOrder: DocumentSigningOrder.SEQUENTIAL }, + update: { signingOrder: DocumentSigningOrder.SEQUENTIAL }, + }, + }, + }, + }); + + await apiSignin({ + page, + email: user.email, + redirectPath: `/t/${team.url}/documents/${document.id}/edit?step=uploadAndRecipients`, + }); + + await expect(getRecipientEmailInputs(page)).toHaveCount(2); + await expect(getRecipientStepCards(page)).toHaveCount(2); + + await expect(getRecipientEmailInputs(page).nth(0)).toHaveValue(alice.email); + await expect(getRecipientEmailInputs(page).nth(1)).toHaveValue(bob.email); + + await clickAddSignerButton(page); + await setRecipientEmail(page, 2, 'carol@example.com'); + + await expect + .poll(async () => { + const recipients = await prisma.recipient.findMany({ where: { envelopeId: document.id } }); + + return recipients.find((recipient) => recipient.email === 'carol@example.com')?.id ?? null; + }) + .not.toBeNull(); + + const recipients = await prisma.recipient.findMany({ + where: { envelopeId: document.id }, + orderBy: { id: 'asc' }, + }); + + expect(recipients.map((recipient) => [recipient.email, recipient.signingOrder])).toEqual([ + [alice.email, null], + [bob.email, null], + ['carol@example.com', null], + ]); + + await expect(getRecipientStepCards(page)).toHaveCount(3); + await expect(getRecipientEmailInputs(page).nth(2)).toHaveValue('carol@example.com'); +}); diff --git a/packages/app-tests/e2e/envelope-editor-v2/envelope-recipient-null-order-hydration.spec.ts b/packages/app-tests/e2e/envelope-editor-v2/envelope-recipient-null-order-hydration.spec.ts index 257d36e1f..50f0b7e77 100644 --- a/packages/app-tests/e2e/envelope-editor-v2/envelope-recipient-null-order-hydration.spec.ts +++ b/packages/app-tests/e2e/envelope-editor-v2/envelope-recipient-null-order-hydration.spec.ts @@ -8,11 +8,9 @@ import { apiSignin } from '../fixtures/authentication'; import { getRecipientEmailInputs, getRecipientStepCards, setRecipientName } from '../fixtures/envelope-editor'; /** - * A recipient with no persisted signing order means "last" everywhere on the - * server (queries sort NULLS LAST, and `effectiveSigningOrder` maps null to the end). * The editor must not invent an order from array position: the guess can land - * on a real order — which now means "same signing step" — or move the - * recipient ahead of one that was meant to sign first. + * on a real order (a signing step) or move the recipient ahead of one meant to + * sign first. */ const seedMixedOrderEnvelope = async (options: { firstOrder: number }) => { diff --git a/packages/app-tests/e2e/recipient/legacy-unordered-recipients.spec.ts b/packages/app-tests/e2e/recipient/legacy-unordered-recipients.spec.ts new file mode 100644 index 000000000..a7eceb08e --- /dev/null +++ b/packages/app-tests/e2e/recipient/legacy-unordered-recipients.spec.ts @@ -0,0 +1,129 @@ +import { completeDocumentWithToken } from '@documenso/lib/server-only/document/complete-document-with-token'; +import { getIsRecipientsTurnToSign } from '@documenso/lib/server-only/recipient/get-is-recipient-turn'; +import { prisma } from '@documenso/prisma'; +import { seedPendingDocumentWithFullFields } from '@documenso/prisma/seed/documents'; +import { seedUser } from '@documenso/prisma/seed/users'; +import { expect, test } from '@playwright/test'; +import { DocumentSigningOrder, SendStatus } from '@prisma/client'; + +/** + * Sequential documents created before automatic numbering hold recipients + * with a NULL `signingOrder`, processed one at a time in id order. + */ + +const expectSigningRequestJobCount = async (recipientId: number, expected: number) => { + const jobs = await prisma.backgroundJob.findMany({ + where: { + jobId: 'send.signing.requested.email', + payload: { + path: ['recipientId'], + equals: recipientId, + }, + }, + }); + + expect(jobs.length).toBe(expected); +}; + +const seedLegacySequentialDocument = async (options: { + signingOrders: Array; + signatureLevel?: string; +}) => { + const { user, team } = await seedUser(); + const signers = await Promise.all(options.signingOrders.map(async () => (await seedUser()).user)); + + const { recipients } = await seedPendingDocumentWithFullFields({ + owner: user, + teamId: team.id, + recipients: signers, + recipientsCreateOptions: options.signingOrders.map((signingOrder, index) => ({ + signingOrder, + sendStatus: index === 0 ? SendStatus.SENT : SendStatus.NOT_SENT, + })), + fields: [], + updateDocumentOptions: { + signatureLevel: options.signatureLevel, + documentMeta: { + upsert: { + create: { signingOrder: DocumentSigningOrder.SEQUENTIAL }, + update: { signingOrder: DocumentSigningOrder.SEQUENTIAL }, + }, + }, + }, + }); + + const byId = [...recipients].sort((a, b) => a.id - b.id); + + return { first: byId[0], second: byId[1], third: byId[2] }; +}; + +test('[LEGACY_UNORDERED]: unordered recipients take turns by id and are invited one at a time', async () => { + const { first, second, third } = await seedLegacySequentialDocument({ signingOrders: [null, null, null] }); + + expect(await getIsRecipientsTurnToSign({ token: first.token })).toBe(true); + expect(await getIsRecipientsTurnToSign({ token: second.token })).toBe(false); + expect(await getIsRecipientsTurnToSign({ token: third.token })).toBe(false); + + await completeDocumentWithToken({ + token: first.token, + id: { type: 'envelopeId', id: first.envelopeId }, + }); + + const secondAfter = await prisma.recipient.findUniqueOrThrow({ where: { id: second.id } }); + const thirdAfter = await prisma.recipient.findUniqueOrThrow({ where: { id: third.id } }); + + expect(secondAfter.sendStatus).toBe(SendStatus.SENT); + await expectSigningRequestJobCount(second.id, 1); + + expect(thirdAfter.sendStatus).toBe(SendStatus.NOT_SENT); + await expectSigningRequestJobCount(third.id, 0); + + expect(await getIsRecipientsTurnToSign({ token: second.token })).toBe(true); + expect(await getIsRecipientsTurnToSign({ token: third.token })).toBe(false); +}); + +test('[LEGACY_UNORDERED]: numbered recipients sign before unordered ones', async () => { + const { first, second, third } = await seedLegacySequentialDocument({ signingOrders: [null, null, 1] }); + + expect(await getIsRecipientsTurnToSign({ token: third.token })).toBe(true); + expect(await getIsRecipientsTurnToSign({ token: first.token })).toBe(false); + + await completeDocumentWithToken({ + token: third.token, + id: { type: 'envelopeId', id: third.envelopeId }, + }); + + await expectSigningRequestJobCount(first.id, 1); + await expectSigningRequestJobCount(second.id, 0); + + expect(await getIsRecipientsTurnToSign({ token: first.token })).toBe(true); + expect(await getIsRecipientsTurnToSign({ token: second.token })).toBe(false); +}); + +test('[LEGACY_UNORDERED]: an AES envelope sequences a legacy duplicate order one recipient at a time', async () => { + const { first, second } = await seedLegacySequentialDocument({ + signingOrders: [1, 1], + signatureLevel: 'AES', + }); + + expect(await getIsRecipientsTurnToSign({ token: first.token })).toBe(true); + expect(await getIsRecipientsTurnToSign({ token: second.token })).toBe(false); + + await expect( + completeDocumentWithToken({ + token: second.token, + id: { type: 'envelopeId', id: second.envelopeId }, + }), + ).rejects.toThrow(); + + await completeDocumentWithToken({ + token: first.token, + id: { type: 'envelopeId', id: first.envelopeId }, + }); + + const secondAfter = await prisma.recipient.findUniqueOrThrow({ where: { id: second.id } }); + + expect(secondAfter.sendStatus).toBe(SendStatus.SENT); + await expectSigningRequestJobCount(second.id, 1); + expect(await getIsRecipientsTurnToSign({ token: second.token })).toBe(true); +}); diff --git a/packages/lib/client-only/hooks/use-editor-recipients.ts b/packages/lib/client-only/hooks/use-editor-recipients.ts index fc1fd2532..04eb37d6a 100644 --- a/packages/lib/client-only/hooks/use-editor-recipients.ts +++ b/packages/lib/client-only/hooks/use-editor-recipients.ts @@ -172,39 +172,15 @@ export const useEditorRecipients = ({ envelope }: EditorRecipientsProps): UseEdi return !canRecipientBeModified(persistedRecipient, envelope.fields); }; - // A recipient without a persisted order means "last" everywhere else — the - // server sorts NULLS LAST. Continue numbering after the highest existing - // order rather than guessing from array position: a guess can land on a - // real order, and equal orders now mean "same signing step". - let fallbackOrder = sourceRecipients.reduce( - (highest, recipient) => Math.max(highest, recipient.signingOrder ?? 0), - 0, - ); - - const signingOrderByRecipientId = new Map(); - - for (const recipient of sourceRecipients) { - if (isCcRecipient(recipient)) { - signingOrderByRecipientId.set(recipient.id, undefined); - } else if (typeof recipient.signingOrder === 'number') { - signingOrderByRecipientId.set(recipient.id, recipient.signingOrder); - } else if (isRecipientLocked(recipient.id)) { - // A locked null order must round-trip as-is: a synthetic number would - // read as a change to a recipient the server refuses to modify. - signingOrderByRecipientId.set(recipient.id, undefined); - } else { - fallbackOrder += 1; - signingOrderByRecipientId.set(recipient.id, fallbackOrder); - } - } - + // Persisted orders round-trip as-is; `normalizeGroupedSigningOrders` + // decides whether an unordered recipient can be numbered. const formRecipients = sourceRecipients.map((recipient) => ({ id: recipient.id, formId: String(recipient.id), name: recipient.name, email: recipient.email, role: recipient.role, - signingOrder: signingOrderByRecipientId.get(recipient.id), + signingOrder: isCcRecipient(recipient) ? undefined : (recipient.signingOrder ?? undefined), actionAuth: ZRecipientAuthOptionsSchema.parse(recipient.authOptions)?.actionAuth ?? undefined, })); diff --git a/packages/lib/server-only/document/complete-document-with-token.ts b/packages/lib/server-only/document/complete-document-with-token.ts index 5d0280044..0b75210fb 100644 --- a/packages/lib/server-only/document/complete-document-with-token.ts +++ b/packages/lib/server-only/document/complete-document-with-token.ts @@ -21,12 +21,13 @@ import { AppError, AppErrorCode } from '../../errors/app-error'; import { jobs } from '../../jobs/client'; import type { TRecipientAccessAuth } from '../../types/document-auth'; import { DocumentAuth } from '../../types/document-auth'; +import { isTspEnvelope } from '../../types/signature-level'; import { mapEnvelopeToWebhookDocumentPayload, ZWebhookDocumentSchema } from '../../types/webhook-payload'; import { extractDocumentAuthMethods } from '../../utils/document-auth'; import type { EnvelopeIdOptions } from '../../utils/envelope'; import { mapSecondaryIdToDocumentId, unsafeBuildEnvelopeIdQuery } from '../../utils/envelope'; import { getRecipientsInActiveSigningStep, isRecipientTurnBySigningOrder } from '../../utils/recipient-groups'; -import { assertRecipientNotExpired } from '../../utils/recipients'; +import { assertRecipientNotExpired, isRecipientBefore } from '../../utils/recipients'; import { getIsRecipientsTurnToSign } from '../recipient/get-is-recipient-turn'; import { triggerWebhook } from '../webhooks/trigger/trigger-webhook'; import { isRecipientAuthorized } from './is-recipient-authorized'; @@ -453,19 +454,21 @@ export const completeDocumentWithToken = async ({ }); if (envelope.documentMeta?.signingOrder === DocumentSigningOrder.SEQUENTIAL) { - const nextRecipients = getRecipientsInActiveSigningStep(pendingRecipients); + const sequencing = { strictlySequential: isTspEnvelope(envelope) }; - const currentRecipientOrder = recipient.signingOrder ?? Number.MAX_SAFE_INTEGER; + const nextRecipients = getRecipientsInActiveSigningStep(pendingRecipients, sequencing); - const hasCompletedCurrentStep = nextRecipients.every( - (pendingRecipient) => (pendingRecipient.signingOrder ?? Number.MAX_SAFE_INTEGER) > currentRecipientOrder, + // Peers still pending in the current step are not the "next" step: + // nobody advances until the whole group has completed. + const hasCompletedCurrentStep = nextRecipients.every((pendingRecipient) => + isRecipientBefore(recipient, pendingRecipient, sequencing), ); if ( nextRecipients.length > 0 && hasCompletedCurrentStep && // Ensure that the next recipient can actually act on the document. - isRecipientTurnBySigningOrder(pendingRecipients, nextRecipients[0]) + isRecipientTurnBySigningOrder(pendingRecipients, nextRecipients[0], sequencing) ) { // Dictation is only allowed when advancing to a single-recipient step. const canDictateNextSigner = diff --git a/packages/lib/server-only/document/send-document.ts b/packages/lib/server-only/document/send-document.ts index 7e555ad3e..c688427f2 100644 --- a/packages/lib/server-only/document/send-document.ts +++ b/packages/lib/server-only/document/send-document.ts @@ -42,7 +42,6 @@ import { getRecipientsInActiveSigningStep } from '../../utils/recipient-groups'; import { getRecipientsWithMissingFields, isRecipientEmailValidForSending } from '../../utils/recipients'; import { getEnvelopeWhereInput } from '../envelope/get-envelope-by-id'; import { insertFormValuesInPdf } from '../pdf/insert-form-values-in-pdf'; -import { assertCompatibleRecipientGrouping } from '../signature-level/assert-compatible-recipient-grouping'; import { assertUserNotDisabledById } from '../user/assert-user-not-disabled'; import { triggerWebhook } from '../webhooks/trigger/trigger-webhook'; @@ -149,17 +148,12 @@ export const sendDocument = async ({ id, userId, teamId, sendEmail, requestMetad envelope.documentMeta.signingOrder = DocumentSigningOrder.SEQUENTIAL; } - assertCompatibleRecipientGrouping({ - signatureLevel: envelope.signatureLevel, - recipients: envelope.recipients, - }); - let recipientsToNotify = envelope.recipients; if (signingOrder === DocumentSigningOrder.SEQUENTIAL) { - // Get the currently active signing group. Recipients sharing the lowest - // pending signing order act in parallel within their group. - recipientsToNotify = getRecipientsInActiveSigningStep(envelope.recipients); + recipientsToNotify = getRecipientsInActiveSigningStep(envelope.recipients, { + strictlySequential: isTspEnvelope(envelope), + }); } if (envelope.envelopeItems.length === 0) { diff --git a/packages/lib/server-only/envelope/create-envelope.ts b/packages/lib/server-only/envelope/create-envelope.ts index 98e90efab..c565ab634 100644 --- a/packages/lib/server-only/envelope/create-envelope.ts +++ b/packages/lib/server-only/envelope/create-envelope.ts @@ -38,9 +38,9 @@ import { createDocumentAuthOptions, createRecipientAuthOptions } from '../../uti import { buildTeamWhereQuery } from '../../utils/teams'; import { incrementDocumentId, incrementTemplateId } from '../envelope/increment-id'; import { assertOrganisationRatesAndLimits } from '../rate-limit/assert-organisation-rates-and-limits'; +import { assignOmittedRecipientSigningOrders } from '../recipient/assign-omitted-recipient-signing-orders'; import { assertCompatibleRecipientGrouping } from '../signature-level/assert-compatible-recipient-grouping'; import { assertCompatibleRecipientRole } from '../signature-level/assert-compatible-recipient-role'; -import { assignDefaultRecipientSigningOrders } from '../signature-level/assign-default-recipient-signing-orders'; import { resolveSignatureLevel } from '../signature-level/resolve-signature-level'; import { getTeamSettings } from '../team/get-team-settings'; import { assertUserNotDisabledById } from '../user/assert-user-not-disabled'; @@ -293,19 +293,11 @@ export const createEnvelope = async ({ role: recipient.role, })); - // Assign default recipients signing orders if TSP is enabled since - // TSP envelopes require sequential signing. - const orderedDefaultRecipients = assignDefaultRecipientSigningOrders({ - signatureLevel, - payloadRecipients: data.recipients ?? [], - defaultRecipients, - }); + const requestedRecipients = [...(data.recipients ?? []), ...defaultRecipients]; - const recipientsToCreate = [...(data.recipients || []), ...orderedDefaultRecipients]; + assertCompatibleRecipientGrouping({ signatureLevel, recipients: requestedRecipients }); - // The grouping assertion runs against the COMBINED set the envelope will - // actually hold, not just the payload. - assertCompatibleRecipientGrouping({ signatureLevel, recipients: recipientsToCreate }); + const recipientsToCreate = assignOmittedRecipientSigningOrders({ recipients: requestedRecipients }); const visibility = visibilityOverride || settings.documentVisibility; diff --git a/packages/lib/server-only/envelope/get-envelope-for-recipient-signing.ts b/packages/lib/server-only/envelope/get-envelope-for-recipient-signing.ts index e4c93d474..f5caab1a1 100644 --- a/packages/lib/server-only/envelope/get-envelope-for-recipient-signing.ts +++ b/packages/lib/server-only/envelope/get-envelope-for-recipient-signing.ts @@ -12,6 +12,7 @@ import { AppError, AppErrorCode } from '../../errors/app-error'; import type { TDocumentAuthMethods } from '../../types/document-auth'; import { ZEnvelopeFieldSchema, ZFieldSchema } from '../../types/field'; import { ZRecipientLiteSchema } from '../../types/recipient'; +import { isTspEnvelope } from '../../types/signature-level'; import { isRecipientTurnBySigningOrder } from '../../utils/recipient-groups'; import { isRecipientExpired } from '../../utils/recipients'; import { isRecipientAuthorized } from '../document/is-recipient-authorized'; @@ -263,7 +264,7 @@ export const getEnvelopeForRecipientSigning = async ({ const isRecipientsTurn = envelope.documentMeta.signingOrder !== DocumentSigningOrder.SEQUENTIAL || - isRecipientTurnBySigningOrder(envelope.recipients, recipient); + isRecipientTurnBySigningOrder(envelope.recipients, recipient, { strictlySequential: isTspEnvelope(envelope) }); const sender = settings.includeSenderDetails ? { diff --git a/packages/lib/server-only/field/get-fields-for-token.ts b/packages/lib/server-only/field/get-fields-for-token.ts index 22a913aec..441f4ad79 100644 --- a/packages/lib/server-only/field/get-fields-for-token.ts +++ b/packages/lib/server-only/field/get-fields-for-token.ts @@ -21,9 +21,7 @@ export const getFieldsForToken = async ({ token }: GetFieldsForTokenOptions) => return []; } - // Assistants can only assist those in strictly later steps — never their - // own group peers. They must have a signing order. - if (recipient.role === RecipientRole.ASSISTANT && typeof recipient.signingOrder === 'number') { + if (recipient.role === RecipientRole.ASSISTANT) { return await prisma.field.findMany({ where: { OR: [ @@ -36,12 +34,7 @@ export const getFieldsForToken = async ({ token }: GetFieldsForTokenOptions) => not: SigningStatus.SIGNED, }, envelopeId: recipient.envelopeId, - AND: [ - getLaterSigningStepRecipientsWhereInput({ - envelopeId: recipient.envelopeId, - signingOrder: recipient.signingOrder, - }), - ], + AND: [getLaterSigningStepRecipientsWhereInput(recipient)], }, envelope: { id: recipient.envelopeId, diff --git a/packages/lib/server-only/recipient/assign-omitted-recipient-signing-orders.test.ts b/packages/lib/server-only/recipient/assign-omitted-recipient-signing-orders.test.ts new file mode 100644 index 000000000..b6391fbb2 --- /dev/null +++ b/packages/lib/server-only/recipient/assign-omitted-recipient-signing-orders.test.ts @@ -0,0 +1,83 @@ +import { RecipientRole } from '@prisma/client'; +import { describe, expect, it } from 'vitest'; + +import { + assignOmittedRecipientSigningOrders, + resolveReplacedRecipientSigningOrders, +} from './assign-omitted-recipient-signing-orders'; + +const signer = (signingOrder?: number | null, id?: number) => ({ + id, + role: RecipientRole.SIGNER, + signingOrder, +}); + +const cc = () => ({ role: RecipientRole.CC, signingOrder: undefined }); + +const ordersOf = (recipients: Array<{ signingOrder?: number | null }>) => + recipients.map((recipient) => recipient.signingOrder); + +describe('assignOmittedRecipientSigningOrders', () => { + it('numbers omitted orders after the highest explicit order, in request order', () => { + const result = assignOmittedRecipientSigningOrders({ + recipients: [signer(), signer(4), signer(), signer(1)], + }); + + expect(ordersOf(result)).toEqual([5, 4, 6, 1]); + }); + + it('continues after the highest order already on the envelope and never numbers CC recipients', () => { + const result = assignOmittedRecipientSigningOrders({ + recipients: [cc(), signer(), signer()], + existingRecipients: [signer(2), signer(7), cc()], + }); + + expect(ordersOf(result)).toEqual([undefined, 8, 9]); + }); + + it('leaves additions unordered when a signing recipient on the envelope has no order', () => { + const result = assignOmittedRecipientSigningOrders({ + recipients: [signer(), signer(9)], + existingRecipients: [signer(1), signer(null)], + }); + + expect(ordersOf(result)).toEqual([undefined, 9]); + }); +}); + +describe('resolveReplacedRecipientSigningOrders', () => { + const existingRecipients = [signer(1, 10), signer(2, 11)] as Array<{ + id: number; + role: RecipientRole; + signingOrder: number | null; + }>; + + it('keeps the persisted order of a recipient whose order was omitted', () => { + const { recipients, requestedOrderRecipients } = resolveReplacedRecipientSigningOrders({ + recipients: [signer(undefined, 11), signer(undefined, 10)], + existingRecipients, + }); + + expect(ordersOf(recipients)).toEqual([2, 1]); + expect(requestedOrderRecipients).toEqual([]); + }); + + it('numbers new recipients after the kept ones and reports explicitly requested orders', () => { + const { recipients, requestedOrderRecipients } = resolveReplacedRecipientSigningOrders({ + recipients: [signer(undefined, 10), signer(), signer(5, 11), signer()], + existingRecipients, + }); + + expect(ordersOf(recipients)).toEqual([1, 6, 5, 7]); + expect(requestedOrderRecipients).toEqual([recipients[2]]); + }); + + it('does not report an unchanged persisted order as requested', () => { + const { requestedOrderRecipients } = resolveReplacedRecipientSigningOrders({ + recipients: [signer(1, 10), signer(3)], + existingRecipients, + }); + + expect(ordersOf(requestedOrderRecipients)).toEqual([3]); + }); +}); diff --git a/packages/lib/server-only/recipient/assign-omitted-recipient-signing-orders.ts b/packages/lib/server-only/recipient/assign-omitted-recipient-signing-orders.ts new file mode 100644 index 000000000..3e17385f9 --- /dev/null +++ b/packages/lib/server-only/recipient/assign-omitted-recipient-signing-orders.ts @@ -0,0 +1,118 @@ +import type { Recipient } from '@prisma/client'; + +import { hasSigningOrder, isCcRecipient } from '../../utils/recipients'; + +type OrderableRecipient = Pick & { signingOrder?: number | null }; + +type AssignOmittedRecipientSigningOrdersOptions = { + recipients: T[]; + existingRecipients?: OrderableRecipient[]; +}; + +/** + * Numbers signing recipients created without an order after the highest + * explicit order on the envelope, in request order. + */ +export const assignOmittedRecipientSigningOrders = ({ + recipients, + existingRecipients = [], +}: AssignOmittedRecipientSigningOrdersOptions): T[] => { + // Numbers sort ahead of unordered rows, so numbering an addition would move + // it in front of a legacy unordered signer. Left unordered it queues behind + // that tail by id. + const hasUnorderedExistingSigner = existingRecipients.some( + (recipient) => !isCcRecipient(recipient) && !hasSigningOrder(recipient), + ); + + if (hasUnorderedExistingSigner) { + return recipients; + } + + let highestOrder = 0; + + for (const recipient of [...existingRecipients, ...recipients]) { + if (hasSigningOrder(recipient)) { + highestOrder = Math.max(highestOrder, recipient.signingOrder); + } + } + + let nextOrder = highestOrder + 1; + + return recipients.map((recipient) => { + if (isCcRecipient(recipient) || hasSigningOrder(recipient)) { + return recipient; + } + + const signingOrder = nextOrder; + + nextOrder += 1; + + return { ...recipient, signingOrder }; + }); +}; + +type ReplacementRecipient = OrderableRecipient & { id?: number | null }; + +type ResolveReplacedRecipientSigningOrdersOptions = { + recipients: T[]; + existingRecipients: Array>; +}; + +/** + * Resolves signing orders for a full recipient replacement (`set*Recipients`): + * a persisted recipient whose order was omitted keeps it, new recipients are + * numbered after the kept ones. + * + * `requestedOrderRecipients` is the subset whose order differs from what was + * persisted — the only entries that can form a new signing group. + */ +export const resolveReplacedRecipientSigningOrders = ({ + recipients, + existingRecipients, +}: ResolveReplacedRecipientSigningOrdersOptions): { recipients: T[]; requestedOrderRecipients: T[] } => { + const persistedById = new Map(existingRecipients.map((recipient) => [recipient.id, recipient])); + + const findPersisted = (recipient: T) => + typeof recipient.id === 'number' ? persistedById.get(recipient.id) : undefined; + + const preserved = recipients.map((recipient) => { + const persisted = findPersisted(recipient); + + if (!persisted || hasSigningOrder(recipient)) { + return recipient; + } + + return { ...recipient, signingOrder: persisted.signingOrder }; + }); + + const keptRecipients = preserved.filter((recipient) => findPersisted(recipient) !== undefined); + const newRecipients = preserved.filter((recipient) => findPersisted(recipient) === undefined); + + const numberedNewRecipients = assignOmittedRecipientSigningOrders({ + recipients: newRecipients, + existingRecipients: keptRecipients, + }); + + let numberedIndex = 0; + + const resolved = preserved.map((recipient) => { + if (findPersisted(recipient) !== undefined) { + return recipient; + } + + const numbered = numberedNewRecipients[numberedIndex]; + + numberedIndex += 1; + + return numbered; + }); + + const requestedOrderRecipients = resolved.filter((_recipient, index) => { + const requested = recipients[index]; + const persisted = findPersisted(requested); + + return hasSigningOrder(requested) && (!persisted || persisted.signingOrder !== requested.signingOrder); + }); + + return { recipients: resolved, requestedOrderRecipients }; +}; diff --git a/packages/lib/server-only/recipient/create-envelope-recipients.ts b/packages/lib/server-only/recipient/create-envelope-recipients.ts index 01a2a6767..e54acba77 100644 --- a/packages/lib/server-only/recipient/create-envelope-recipients.ts +++ b/packages/lib/server-only/recipient/create-envelope-recipients.ts @@ -14,6 +14,7 @@ import { assertEnvelopeMutable } from '../envelope/assert-envelope-mutable'; import { getEnvelopeWhereInput } from '../envelope/get-envelope-by-id'; import { assertCompatibleRecipientGrouping } from '../signature-level/assert-compatible-recipient-grouping'; import { assertCompatibleRecipientRole } from '../signature-level/assert-compatible-recipient-role'; +import { assignOmittedRecipientSigningOrders } from './assign-omitted-recipient-signing-orders'; export interface CreateEnvelopeRecipientsOptions { userId: number; @@ -47,7 +48,6 @@ export const createEnvelopeRecipients = async ({ const envelope = await prisma.envelope.findFirst({ where: envelopeWhereInput, include: { - recipients: true, team: { select: { organisation: { @@ -92,21 +92,32 @@ export const createEnvelopeRecipients = async ({ }); } - // Grouping is a property of the whole recipient set, so check the state the - // envelope will be left in rather than the incoming batch alone. - assertCompatibleRecipientGrouping({ - signatureLevel: envelope.signatureLevel, - recipients: [...envelope.recipients, ...recipientsToCreate], - }); - - const normalizedRecipients = recipientsToCreate.map((recipient) => ({ - ...recipient, - email: recipient.email.toLowerCase(), - })); - const createdRecipients = await prisma.$transaction(async (tx) => { + // Lock the envelope so concurrent additions allocate distinct signing orders. + await tx.$queryRaw`SELECT "id" FROM "Envelope" WHERE "id" = ${envelope.id} FOR UPDATE`; + await assertEnvelopeMutable(envelope, tx); + const existingRecipients = await tx.recipient.findMany({ + where: { + envelopeId: envelope.id, + }, + }); + + assertCompatibleRecipientGrouping({ + signatureLevel: envelope.signatureLevel, + recipients: recipientsToCreate, + existingRecipients, + }); + + const normalizedRecipients = assignOmittedRecipientSigningOrders({ + recipients: recipientsToCreate.map((recipient) => ({ + ...recipient, + email: recipient.email.toLowerCase(), + })), + existingRecipients, + }); + return await Promise.all( normalizedRecipients.map(async (recipient) => { const authOptions = createRecipientAuthOptions({ diff --git a/packages/lib/server-only/recipient/get-is-recipient-turn.ts b/packages/lib/server-only/recipient/get-is-recipient-turn.ts index cbbd8020c..57168800e 100644 --- a/packages/lib/server-only/recipient/get-is-recipient-turn.ts +++ b/packages/lib/server-only/recipient/get-is-recipient-turn.ts @@ -1,6 +1,7 @@ import { prisma } from '@documenso/prisma'; import { DocumentSigningOrder, EnvelopeType } from '@prisma/client'; +import { isTspEnvelope } from '../../types/signature-level'; import { isRecipientTurnBySigningOrder } from '../../utils/recipient-groups'; export type GetIsRecipientTurnOptions = { @@ -33,5 +34,7 @@ export async function getIsRecipientsTurnToSign({ token }: GetIsRecipientTurnOpt return false; } - return isRecipientTurnBySigningOrder(envelope.recipients, currentRecipient); + return isRecipientTurnBySigningOrder(envelope.recipients, currentRecipient, { + strictlySequential: isTspEnvelope(envelope), + }); } diff --git a/packages/lib/server-only/recipient/set-document-recipients.ts b/packages/lib/server-only/recipient/set-document-recipients.ts index f4ea37ef8..0d7f753c8 100644 --- a/packages/lib/server-only/recipient/set-document-recipients.ts +++ b/packages/lib/server-only/recipient/set-document-recipients.ts @@ -19,6 +19,7 @@ import { assertEnvelopeMutable } from '../envelope/assert-envelope-mutable'; import { getEnvelopeWhereInput } from '../envelope/get-envelope-by-id'; import { assertCompatibleRecipientGrouping } from '../signature-level/assert-compatible-recipient-grouping'; import { assertCompatibleRecipientRole } from '../signature-level/assert-compatible-recipient-role'; +import { resolveReplacedRecipientSigningOrders } from './assign-omitted-recipient-signing-orders'; export interface SetDocumentRecipientsOptions { userId: number; @@ -99,17 +100,21 @@ export const setDocumentRecipients = async ({ }); } - assertCompatibleRecipientGrouping({ - signatureLevel: envelope.signatureLevel, - recipients, + const existingRecipients = envelope.recipients; + + const { recipients: normalizedRecipients, requestedOrderRecipients } = resolveReplacedRecipientSigningOrders({ + recipients: recipients.map((recipient) => ({ + ...recipient, + email: recipient.email.toLowerCase(), + })), + existingRecipients, }); - const normalizedRecipients = recipients.map((recipient) => ({ - ...recipient, - email: recipient.email.toLowerCase(), - })); - - const existingRecipients = envelope.recipients; + assertCompatibleRecipientGrouping({ + signatureLevel: envelope.signatureLevel, + recipients: requestedOrderRecipients, + existingRecipients: normalizedRecipients.filter((recipient) => !requestedOrderRecipients.includes(recipient)), + }); const removedRecipients = existingRecipients.filter( (existingRecipient) => !normalizedRecipients.find((recipient) => recipient.id === existingRecipient.id), diff --git a/packages/lib/server-only/recipient/set-template-recipients.ts b/packages/lib/server-only/recipient/set-template-recipients.ts index 2ea330ec4..a26d920be 100644 --- a/packages/lib/server-only/recipient/set-template-recipients.ts +++ b/packages/lib/server-only/recipient/set-template-recipients.ts @@ -14,6 +14,7 @@ import { type EnvelopeIdOptions, mapSecondaryIdToTemplateId } from '../../utils/ import { getEnvelopeWhereInput } from '../envelope/get-envelope-by-id'; import { assertCompatibleRecipientGrouping } from '../signature-level/assert-compatible-recipient-grouping'; import { assertCompatibleRecipientRole } from '../signature-level/assert-compatible-recipient-role'; +import { resolveReplacedRecipientSigningOrders } from './assign-omitted-recipient-signing-orders'; export type SetTemplateRecipientsOptions = { userId: number; @@ -69,28 +70,32 @@ export const setTemplateRecipients = async ({ userId, teamId, id, recipients }: }); } - assertCompatibleRecipientGrouping({ - signatureLevel: envelope.signatureLevel, - recipients, - }); + const existingRecipients = envelope.recipients; + + const { recipients: normalizedRecipients, requestedOrderRecipients } = resolveReplacedRecipientSigningOrders({ + recipients: recipients.map((recipient) => { + // Force replace any changes to the name or email of the direct recipient. + if (envelope.directLink && recipient.id === envelope.directLink.directTemplateRecipientId) { + return { + ...recipient, + email: DIRECT_TEMPLATE_RECIPIENT_EMAIL, + name: DIRECT_TEMPLATE_RECIPIENT_NAME, + }; + } - const normalizedRecipients = recipients.map((recipient) => { - // Force replace any changes to the name or email of the direct recipient. - if (envelope.directLink && recipient.id === envelope.directLink.directTemplateRecipientId) { return { ...recipient, - email: DIRECT_TEMPLATE_RECIPIENT_EMAIL, - name: DIRECT_TEMPLATE_RECIPIENT_NAME, + email: recipient.email.toLowerCase(), }; - } - - return { - ...recipient, - email: recipient.email.toLowerCase(), - }; + }), + existingRecipients, }); - const existingRecipients = envelope.recipients; + assertCompatibleRecipientGrouping({ + signatureLevel: envelope.signatureLevel, + recipients: requestedOrderRecipients, + existingRecipients: normalizedRecipients.filter((recipient) => !requestedOrderRecipients.includes(recipient)), + }); const removedRecipients = existingRecipients.filter( (existingRecipient) => !normalizedRecipients.find((recipient) => recipient.id === existingRecipient.id), diff --git a/packages/lib/server-only/recipient/update-envelope-recipients.ts b/packages/lib/server-only/recipient/update-envelope-recipients.ts index 4ffb372e2..371609ce6 100644 --- a/packages/lib/server-only/recipient/update-envelope-recipients.ts +++ b/packages/lib/server-only/recipient/update-envelope-recipients.ts @@ -100,14 +100,22 @@ export const updateEnvelopeRecipients = async ({ }); } + const orderUpdates = recipients.flatMap((update) => { + const existingRecipient = envelope.recipients.find((recipient) => recipient.id === update.id); + + if (!existingRecipient || update.signingOrder === undefined) { + return []; + } + + return [{ ...existingRecipient, ...update }]; + }); + + const orderUpdateIds = new Set(orderUpdates.map((recipient) => recipient.id)); + assertCompatibleRecipientGrouping({ signatureLevel: envelope.signatureLevel, - // Combine the existing recipients with the new ones to see if the grouping is compatible. - recipients: envelope.recipients.map((existingRecipient) => { - const update = recipients.find((recipient) => recipient.id === existingRecipient.id); - - return update ? { ...existingRecipient, ...update } : existingRecipient; - }), + recipients: orderUpdates, + existingRecipients: envelope.recipients.filter((recipient) => !orderUpdateIds.has(recipient.id)), }); const recipientsToUpdate = recipients.map((recipient) => { diff --git a/packages/lib/server-only/signature-level/assert-compatible-recipient-grouping.test.ts b/packages/lib/server-only/signature-level/assert-compatible-recipient-grouping.test.ts index a832564be..9744b01e5 100644 --- a/packages/lib/server-only/signature-level/assert-compatible-recipient-grouping.test.ts +++ b/packages/lib/server-only/signature-level/assert-compatible-recipient-grouping.test.ts @@ -4,29 +4,33 @@ import { describe, expect, it } from 'vitest'; import { SignatureLevel } from '../../types/signature-level'; import { assertCompatibleRecipientGrouping } from './assert-compatible-recipient-grouping'; -const signer = (signingOrder: number | null) => ({ role: RecipientRole.SIGNER, signingOrder }); -const cc = (signingOrder: number | null) => ({ role: RecipientRole.CC, signingOrder }); +type TestRecipient = { role: RecipientRole; signingOrder?: number | null }; + +const signer = (signingOrder?: number | null): TestRecipient => ({ role: RecipientRole.SIGNER, signingOrder }); +const cc = (signingOrder?: number | null): TestRecipient => ({ role: RecipientRole.CC, signingOrder }); const expectRejected = ( signatureLevel: string, - recipients: Array<{ role: RecipientRole; signingOrder: number | null }>, + recipients: TestRecipient[], + existingRecipients: TestRecipient[] = [], ) => { - expect(() => assertCompatibleRecipientGrouping({ signatureLevel, recipients })).toThrow( + expect(() => assertCompatibleRecipientGrouping({ signatureLevel, recipients, existingRecipients })).toThrow( /signing group|same signing step/i, ); }; const expectAccepted = ( signatureLevel: string, - recipients: Array<{ role: RecipientRole; signingOrder: number | null }>, + recipients: TestRecipient[], + existingRecipients: TestRecipient[] = [], ) => { - expect(() => assertCompatibleRecipientGrouping({ signatureLevel, recipients })).not.toThrow(); + expect(() => assertCompatibleRecipientGrouping({ signatureLevel, recipients, existingRecipients })).not.toThrow(); }; describe('assertCompatibleRecipientGrouping', () => { describe('AES/QES envelopes', () => { for (const signatureLevel of [SignatureLevel.AES, SignatureLevel.QES]) { - it(`rejects two signers sharing a signing order (${signatureLevel})`, () => { + it(`rejects two signers sharing an explicit signing order (${signatureLevel})`, () => { expectRejected(signatureLevel, [signer(1), signer(2), signer(2)]); }); @@ -35,46 +39,27 @@ describe('assertCompatibleRecipientGrouping', () => { }); } - // Every null order collapses into the same tail step, so two of them sign - // in parallel exactly as a duplicate order would. - it('rejects two signers without a signing order', () => { - expectRejected(SignatureLevel.AES, [signer(null), signer(null)]); + it('rejects a new signer joining a step that already exists on the envelope', () => { + expectRejected(SignatureLevel.AES, [signer(1)], [signer(1)]); }); - it('rejects a signer without an order alongside an ordered signer', () => { - // The unordered recipient shares the tail step with the other null. - expectRejected(SignatureLevel.AES, [signer(1), signer(null), signer(null)]); + it('accepts any number of signers without a signing order', () => { + expectAccepted(SignatureLevel.AES, [signer(), signer(null), signer(undefined)]); + expectAccepted(SignatureLevel.AES, [signer(1), signer(), signer()], [signer(null), signer(null)]); }); - it('accepts a single signer without a signing order', () => { - expectAccepted(SignatureLevel.AES, [signer(null)]); + it('does not validate untouched existing recipients against each other', () => { + expectAccepted(SignatureLevel.AES, [signer(3)], [signer(1), signer(1)]); }); - it('accepts one ordered signer and one unordered signer', () => { - expectAccepted(SignatureLevel.AES, [signer(1), signer(null)]); - }); - - // CC recipients never sign, so their order carries no meaning. - it('ignores CC recipients sharing an order with a signer', () => { - expectAccepted(SignatureLevel.AES, [signer(1), cc(1)]); - }); - - it('ignores several CC recipients sharing an order with each other', () => { - expectAccepted(SignatureLevel.AES, [signer(1), cc(2), cc(2), cc(null), cc(null)]); - }); - - it('accepts an empty recipient list', () => { - expectAccepted(SignatureLevel.AES, []); + it('ignores CC recipients', () => { + expectAccepted(SignatureLevel.AES, [signer(1), cc(1), cc(1)], [cc(1)]); }); }); describe('SES envelopes', () => { it('permits signing groups', () => { - expectAccepted(SignatureLevel.SES, [signer(1), signer(2), signer(2)]); - }); - - it('permits multiple unordered signers', () => { - expectAccepted(SignatureLevel.SES, [signer(null), signer(null)]); + expectAccepted(SignatureLevel.SES, [signer(1), signer(2), signer(2)], [signer(2)]); }); }); }); diff --git a/packages/lib/server-only/signature-level/assert-compatible-recipient-grouping.ts b/packages/lib/server-only/signature-level/assert-compatible-recipient-grouping.ts index 652f24c5f..13c648689 100644 --- a/packages/lib/server-only/signature-level/assert-compatible-recipient-grouping.ts +++ b/packages/lib/server-only/signature-level/assert-compatible-recipient-grouping.ts @@ -2,53 +2,58 @@ import type { Recipient } from '@prisma/client'; import { AppError, AppErrorCode } from '../../errors/app-error'; import { isTspEnvelope } from '../../types/signature-level'; -import { effectiveSigningOrder } from '../../utils/recipient-groups'; -import { isCcRecipient } from '../../utils/recipients'; +import { hasSigningOrder, isCcRecipient } from '../../utils/recipients'; + +type GroupableRecipient = Pick & { signingOrder?: number | null }; type AssertCompatibleRecipientGroupingOptions = { signatureLevel: string; - recipients: Array & { signingOrder?: number | null }>; + recipients: GroupableRecipient[]; + + /** + * Recipients this request leaves untouched. A request may not join their + * steps, but they are never validated against each other: legacy duplicates + * are sequenced strictly at runtime instead. + */ + existingRecipients?: GroupableRecipient[]; }; /** - * Reject recipient signing groups on AES/QES envelopes. + * Reject newly requested signing groups on AES/QES envelopes: group members + * may sign at the same time, and a TSP signature computed over a document + * snapshot would be invalidated by an overlapping signer. * - * A "group" is two or more signing recipients sharing a signing step, which - * they may then complete in any order — including at the same time. That is - * parallel signing scoped to one step - * - * Recipients sharing a step are detected by {@link effectiveSigningOrder}, so an - * absent signing order counts too — every unordered recipient lands in the - * same tail step and would sign in parallel. - * - * CC recipients are ignored: they never sign, and their signing order carries - * no meaning anywhere else. - * - * SES envelopes pass through unchanged — signing groups are an SES feature. + * An omitted order never forms a group (it is numbered on creation, or + * sequenced by id for legacy rows). CC recipients never sign and are ignored. */ export const assertCompatibleRecipientGrouping = ({ signatureLevel, recipients, + existingRecipients = [], }: AssertCompatibleRecipientGroupingOptions): void => { if (!isTspEnvelope({ signatureLevel })) { return; } - const seenOrders = new Set(); + const takenOrders = new Set(); + + for (const recipient of existingRecipients) { + if (!isCcRecipient(recipient) && hasSigningOrder(recipient)) { + takenOrders.add(recipient.signingOrder); + } + } for (const recipient of recipients) { - if (isCcRecipient(recipient)) { + if (isCcRecipient(recipient) || !hasSigningOrder(recipient)) { continue; } - const order = effectiveSigningOrder(recipient); - - if (seenOrders.has(order)) { + if (takenOrders.has(recipient.signingOrder)) { throw new AppError(AppErrorCode.INVALID_BODY, { - message: `Envelopes signed at '${signatureLevel}' cannot place two recipients in the same signing step — a signing group is parallel signing within one step, which breaks the per-recipient /ByteRange invariant TSP signatures rely on. Give every signing recipient a distinct signingOrder.`, + message: `Envelopes signed at '${signatureLevel}' cannot place two recipients in the same signing step — a signing group is parallel signing within one step, which breaks the per-recipient /ByteRange invariant TSP signatures rely on. Give every signing recipient a distinct signingOrder or omit it.`, }); } - seenOrders.add(order); + takenOrders.add(recipient.signingOrder); } }; diff --git a/packages/lib/server-only/signature-level/assign-default-recipient-signing-orders.test.ts b/packages/lib/server-only/signature-level/assign-default-recipient-signing-orders.test.ts deleted file mode 100644 index 157204b1c..000000000 --- a/packages/lib/server-only/signature-level/assign-default-recipient-signing-orders.test.ts +++ /dev/null @@ -1,100 +0,0 @@ -import { RecipientRole } from '@prisma/client'; -import { describe, expect, it } from 'vitest'; - -import { assertCompatibleRecipientGrouping } from './assert-compatible-recipient-grouping'; -import { assignDefaultRecipientSigningOrders } from './assign-default-recipient-signing-orders'; - -const signer = (signingOrder?: number | null) => ({ - role: RecipientRole.SIGNER, - signingOrder, -}); - -const defaultRecipient = (email: string, role: RecipientRole = RecipientRole.SIGNER) => ({ - email, - name: email, - role, -}); - -describe('assignDefaultRecipientSigningOrders', () => { - it('leaves defaults unordered on SES envelopes', () => { - const result = assignDefaultRecipientSigningOrders({ - signatureLevel: 'SES', - payloadRecipients: [signer(1), signer(2)], - defaultRecipients: [defaultRecipient('a@example.com'), defaultRecipient('b@example.com')], - }); - - expect(result.map((recipient) => recipient.signingOrder)).toEqual([undefined, undefined]); - }); - - it.each(['AES', 'QES'])('assigns distinct orders after the payload max on %s envelopes', (signatureLevel) => { - const result = assignDefaultRecipientSigningOrders({ - signatureLevel, - payloadRecipients: [signer(1), signer(4)], - defaultRecipients: [defaultRecipient('a@example.com'), defaultRecipient('b@example.com')], - }); - - expect(result.map((recipient) => recipient.signingOrder)).toEqual([5, 6]); - }); - - it('numbers defaults from 1 when the payload has no numeric orders', () => { - const result = assignDefaultRecipientSigningOrders({ - signatureLevel: 'QES', - payloadRecipients: [], - defaultRecipients: [defaultRecipient('a@example.com'), defaultRecipient('b@example.com')], - }); - - expect(result.map((recipient) => recipient.signingOrder)).toEqual([1, 2]); - }); - - it('skips CC defaults while numbering the rest', () => { - const result = assignDefaultRecipientSigningOrders({ - signatureLevel: 'AES', - payloadRecipients: [signer(2)], - defaultRecipients: [ - defaultRecipient('a@example.com'), - defaultRecipient('cc@example.com', RecipientRole.CC), - defaultRecipient('b@example.com'), - ], - }); - - expect(result.map((recipient) => recipient.signingOrder)).toEqual([3, undefined, 4]); - }); - - it('produces a combined set that satisfies the TSP grouping assertion', () => { - const payloadRecipients = [signer(1), signer(2)]; - - const defaults = assignDefaultRecipientSigningOrders({ - signatureLevel: 'QES', - payloadRecipients, - defaultRecipients: [defaultRecipient('a@example.com'), defaultRecipient('b@example.com')], - }); - - expect(() => - assertCompatibleRecipientGrouping({ - signatureLevel: 'QES', - recipients: [...payloadRecipients, ...defaults], - }), - ).not.toThrow(); - }); - - it('remains assertion-compatible when the payload holds a single unordered recipient', () => { - // The write-path assert permits one unordered recipient (no shared step); - // numbered defaults must not collide with it. - const payloadRecipients = [signer(null)]; - - const defaults = assignDefaultRecipientSigningOrders({ - signatureLevel: 'AES', - payloadRecipients, - defaultRecipients: [defaultRecipient('a@example.com')], - }); - - expect(defaults.map((recipient) => recipient.signingOrder)).toEqual([1]); - - expect(() => - assertCompatibleRecipientGrouping({ - signatureLevel: 'AES', - recipients: [...payloadRecipients, ...defaults], - }), - ).not.toThrow(); - }); -}); diff --git a/packages/lib/server-only/signature-level/assign-default-recipient-signing-orders.ts b/packages/lib/server-only/signature-level/assign-default-recipient-signing-orders.ts deleted file mode 100644 index ea11739a2..000000000 --- a/packages/lib/server-only/signature-level/assign-default-recipient-signing-orders.ts +++ /dev/null @@ -1,57 +0,0 @@ -import type { Recipient } from '@prisma/client'; - -import { isTspEnvelope } from '../../types/signature-level'; -import { isCcRecipient } from '../../utils/recipients'; - -type AssignDefaultRecipientSigningOrdersOptions = { - signatureLevel: string; - - /** - * The recipients supplied by the caller, which should already be validated by - * {@link assertCompatibleRecipientGrouping}. - */ - payloadRecipients: Array & { signingOrder?: number | null }>; - - /** - * The team default recipients to append. - */ - defaultRecipients: T[]; -}; - -/** - * Assigns distinct signing orders to default recipients appended to a - * TSP (AES/QES) envelope. - * - * Default recipients carry no signing order, so on a TSP envelope two or more of - * them would share the unordered tail step — a signing group, which TSP - * signatures cannot hold - * - * CC defaults are left unordered: they never sign and are ignored by the - * grouping assertion. - * - * SES envelopes pass through unchanged — shared steps are an SES feature. - */ -export const assignDefaultRecipientSigningOrders = >({ - signatureLevel, - payloadRecipients, - defaultRecipients, -}: AssignDefaultRecipientSigningOrdersOptions): Array => { - if (!isTspEnvelope({ signatureLevel })) { - return defaultRecipients; - } - - let nextOrder = - payloadRecipients.reduce((highest, recipient) => Math.max(highest, recipient.signingOrder ?? 0), 0) + 1; - - return defaultRecipients.map((recipient) => { - if (isCcRecipient(recipient)) { - return recipient; - } - - const signingOrder = nextOrder; - - nextOrder += 1; - - return { ...recipient, signingOrder }; - }); -}; diff --git a/packages/lib/server-only/template/create-document-from-direct-template.ts b/packages/lib/server-only/template/create-document-from-direct-template.ts index bf7e25d59..b61683fab 100644 --- a/packages/lib/server-only/template/create-document-from-direct-template.ts +++ b/packages/lib/server-only/template/create-document-from-direct-template.ts @@ -41,11 +41,16 @@ import { } from '../../utils/document-auth'; import { mapSecondaryIdToTemplateId } from '../../utils/envelope'; import { getRecipientsInActiveSigningStep } from '../../utils/recipient-groups'; -import { getRecipientsWithMissingFields } from '../../utils/recipients'; +import { + getRecipientsWithMissingFields, + isRecipientBefore, + sortRecipientsBySigningPosition, +} from '../../utils/recipients'; import { sendDocument } from '../document/send-document'; import { validateFieldAuth } from '../document/validate-field-auth'; import { incrementDocumentId } from '../envelope/increment-id'; import { assertOrganisationRatesAndLimits } from '../rate-limit/assert-organisation-rates-and-limits'; +import { assignOmittedRecipientSigningOrders } from '../recipient/assign-omitted-recipient-signing-orders'; import { resolveSignatureLevel } from '../signature-level/resolve-signature-level'; import { getTeamSettings } from '../team/get-team-settings'; import { triggerWebhook } from '../webhooks/trigger/trigger-webhook'; @@ -212,6 +217,16 @@ export const createDocumentFromDirectTemplate = async ({ (recipient) => recipient.id !== directTemplateRecipient.id, ); + // Number unordered recipients by template position since the copies get new ids. + const signingOrderByTemplateRecipientId = new Map( + assignOmittedRecipientSigningOrders({ + recipients: sortRecipientsBySigningPosition(recipients), + }).map((recipient) => [recipient.id, recipient.signingOrder]), + ); + + const resolveSigningOrder = (templateRecipientId: number) => + signingOrderByTemplateRecipientId.get(templateRecipientId) ?? null; + // Carry the template's level forward, coercing if the instance mode has // changed since the template was created. ZSignatureLevelSchema parses the // free-form TEXT column defensively. Resolved before meta extraction so @@ -411,7 +426,7 @@ export const createDocumentFromDirectTemplate = async ({ }), sendStatus: recipient.role === RecipientRole.CC ? SendStatus.SENT : SendStatus.NOT_SENT, signingStatus: recipient.role === RecipientRole.CC ? SigningStatus.SIGNED : SigningStatus.NOT_SIGNED, - signingOrder: recipient.signingOrder, + signingOrder: resolveSigningOrder(recipient.id), token: nanoid(), }; }), @@ -483,7 +498,7 @@ export const createDocumentFromDirectTemplate = async ({ signingStatus: SigningStatus.SIGNED, sendStatus: SendStatus.SENT, signedAt: initialRequestTime, - signingOrder: directTemplateRecipient.signingOrder, + signingOrder: resolveSigningOrder(directTemplateRecipient.id), fields: { createMany: { data: directTemplateNonSignatureFields.map(({ templateField, customText }) => { @@ -698,14 +713,12 @@ export const createDocumentFromDirectTemplate = async ({ const nextRecipients = getRecipientsInActiveSigningStep(pendingRecipients); - const directRecipientOrder = createdDirectRecipient.signingOrder ?? Number.MAX_SAFE_INTEGER; - // The direct recipient can share a step with other recipients (a signing // group). Those peers are still pending, so without this check they would // look like the "next" step and be dictated over — dictation may only // affect a strictly later step. - const hasCompletedCurrentStep = nextRecipients.every( - (pendingRecipient) => (pendingRecipient.signingOrder ?? Number.MAX_SAFE_INTEGER) > directRecipientOrder, + const hasCompletedCurrentStep = nextRecipients.every((pendingRecipient) => + isRecipientBefore(createdDirectRecipient, pendingRecipient), ); // Dictation can only apply when the next step is a single recipient. diff --git a/packages/lib/server-only/template/create-document-from-template.ts b/packages/lib/server-only/template/create-document-from-template.ts index bd54d650a..ac705e242 100644 --- a/packages/lib/server-only/template/create-document-from-template.ts +++ b/packages/lib/server-only/template/create-document-from-template.ts @@ -1,6 +1,6 @@ import { nanoid, prefixedId } from '@documenso/lib/universal/id'; import { prisma } from '@documenso/prisma'; -import type { DocumentDistributionMethod, DocumentSigningOrder } from '@prisma/client'; +import type { DocumentDistributionMethod, DocumentSigningOrder, Prisma } from '@prisma/client'; import { DocumentSource, EnvelopeType, @@ -52,9 +52,9 @@ import { getEnvelopeWhereInput } from '../envelope/get-envelope-by-id'; import { incrementDocumentId } from '../envelope/increment-id'; import { insertFormValuesInPdf } from '../pdf/insert-form-values-in-pdf'; import { assertOrganisationRatesAndLimits } from '../rate-limit/assert-organisation-rates-and-limits'; +import { assignOmittedRecipientSigningOrders } from '../recipient/assign-omitted-recipient-signing-orders'; import { assertCompatibleRecipientGrouping } from '../signature-level/assert-compatible-recipient-grouping'; import { assertCompatibleRecipientRole } from '../signature-level/assert-compatible-recipient-role'; -import { assignDefaultRecipientSigningOrders } from '../signature-level/assign-default-recipient-signing-orders'; import { resolveSignatureLevel } from '../signature-level/resolve-signature-level'; import { getTeamSettings } from '../team/get-team-settings'; import { triggerWebhook } from '../webhooks/trigger/trigger-webhook'; @@ -312,6 +312,11 @@ export const createDocumentFromTemplate = async ({ include: { fields: true, }, + // Unordered template recipients are numbered in this sequence. + orderBy: [ + { signingOrder: { sort: 'asc', nulls: 'last' } }, + { id: 'asc' }, + ] satisfies Prisma.RecipientOrderByWithRelationInput[], }, envelopeItems: { include: { @@ -525,22 +530,26 @@ export const createDocumentFromTemplate = async ({ strict: false, }); - // Assign default recipients signing orders if TSP is enabled since - // TSP envelopes require sequential signing. - const orderedDefaultRecipients = assignDefaultRecipientSigningOrders({ - signatureLevel, - payloadRecipients: finalRecipients, - defaultRecipients: defaultRecipientsFinal, + const requestedOrderRecipients = finalRecipients.filter((finalRecipient) => { + const override = recipients.find((recipient) => recipient.id === finalRecipient.templateRecipientId); + + return typeof override?.signingOrder === 'number'; }); - const allFinalRecipients = [...finalRecipients, ...orderedDefaultRecipients]; + assertCompatibleRecipientGrouping({ + signatureLevel, + recipients: requestedOrderRecipients, + existingRecipients: finalRecipients.filter((recipient) => !requestedOrderRecipients.includes(recipient)), + }); + + const allFinalRecipients = assignOmittedRecipientSigningOrders({ + recipients: [...finalRecipients, ...defaultRecipientsFinal], + }); for (const recipient of allFinalRecipients) { assertCompatibleRecipientRole({ signatureLevel, role: recipient.role }); } - assertCompatibleRecipientGrouping({ signatureLevel, recipients: allFinalRecipients }); - const documentMeta = await prisma.documentMeta.create({ data: extractDerivedDocumentMeta( settings, diff --git a/packages/lib/utils/recipient-groups.test.ts b/packages/lib/utils/recipient-groups.test.ts index 9019c6ba9..8f04aa652 100644 --- a/packages/lib/utils/recipient-groups.test.ts +++ b/packages/lib/utils/recipient-groups.test.ts @@ -54,17 +54,17 @@ describe('groupRecipientsBySigningOrder', () => { expect(steps.map((step) => step.members.map((m) => m.formId))).toEqual([['a'], ['c', 'b']]); }); - it('collects recipients without a signing order into a single tail step', () => { + it('places each recipient without a signing order in its own step, after numbered steps, by id', () => { const recipients = [ - { formId: 'a', role: RecipientRole.SIGNER, signingOrder: 1 }, - { formId: 'b', role: RecipientRole.SIGNER, signingOrder: null }, - { formId: 'c', role: RecipientRole.SIGNER, signingOrder: undefined }, + { id: 30, formId: 'c', role: RecipientRole.SIGNER, signingOrder: null }, + { id: 20, formId: 'b', role: RecipientRole.SIGNER, signingOrder: null }, + { id: 40, formId: 'a', role: RecipientRole.SIGNER, signingOrder: 1 }, ]; const { steps } = groupRecipientsBySigningOrder(recipients); - expect(steps).toHaveLength(2); - expect(steps[1].members.map((m) => m.formId)).toEqual(['b', 'c']); + expect(steps.map((step) => step.order)).toEqual([1, null, null]); + expect(steps.map((step) => step.members.map((m) => m.formId))).toEqual([['a'], ['b'], ['c']]); }); }); @@ -177,22 +177,67 @@ describe('normalizeGroupedSigningOrders', () => { ]); }); - it('leaves a locked recipient without a persisted order alone', () => { - const recipients: Array<{ formId: string; role: RecipientRole; signingOrder: number | null }> = [ - { formId: 'locked', role: RecipientRole.SIGNER, signingOrder: null }, - { formId: 'a', role: RecipientRole.SIGNER, signingOrder: null }, + it('numbers an editable unordered recipient after a numbered locked step', () => { + const recipients = [ + { id: 2, formId: 'b', role: RecipientRole.SIGNER, signingOrder: null }, + { id: 1, formId: 'locked', role: RecipientRole.SIGNER, signingOrder: 1 }, + ]; + + const normalized = normalizeGroupedSigningOrders(recipients, (r) => r.formId !== 'locked'); + + expect(normalized.map((r) => [r.formId, r.signingOrder])).toEqual([ + ['locked', 1], + ['b', 2], + ]); + }); + + // Numbers sort ahead of unordered rows, so giving 'b' or the new signer a + // number would move them in front of the locked recipient who already acted. + it('keeps everything unordered behind a locked recipient without an order', () => { + const recipients = [ + { id: 1, formId: 'locked', role: RecipientRole.SIGNER, signingOrder: null }, + { id: 2, formId: 'b', role: RecipientRole.SIGNER, signingOrder: null }, + { formId: 'new', role: RecipientRole.SIGNER, signingOrder: undefined }, + { formId: 'cc', role: RecipientRole.CC, signingOrder: undefined }, ]; - // Both share the null tail step, so the whole step is locked. const normalized = normalizeGroupedSigningOrders(recipients, (r) => r.formId !== 'locked'); expect(normalized.map((r) => [r.formId, r.signingOrder])).toEqual([ ['locked', undefined], - ['a', undefined], + ['b', undefined], + ['new', undefined], + ['cc', undefined], ]); }); }); +describe('editor operations behind a locked unordered recipient', () => { + const frozen = () => [ + { id: 1, formId: 'locked', role: RecipientRole.SIGNER, signingOrder: null }, + { id: 2, formId: 'b', role: RecipientRole.SIGNER, signingOrder: null }, + { id: 3, formId: 'c', role: RecipientRole.SIGNER, signingOrder: null }, + ]; + + const canUpdate = (r: { formId: string }) => r.formId !== 'locked'; + + const positions = (signers: Array<{ formId: string; signingOrder?: number }>) => + signers.map((signer) => [signer.formId, signer.signingOrder]); + + const expected = [ + ['locked', undefined], + ['b', undefined], + ['c', undefined], + ]; + + it('refuses to number or move anything', () => { + expect(positions(reorderStep(frozen(), 2, 1, canUpdate))).toEqual(expected); + expect(positions(extractRecipientToNewStep(frozen(), 'b', 3, canUpdate))).toEqual(expected); + expect(positions(mergeSteps(frozen(), 2, 1, canUpdate))).toEqual(expected); + expect(positions(moveRecipientToStep(frozen(), 'c', 1, canUpdate))).toEqual(expected); + }); +}); + const makeSigners = () => [ { formId: 'a', role: RecipientRole.SIGNER, signingOrder: 1 }, { formId: 'b', role: RecipientRole.SIGNER, signingOrder: 2 }, @@ -496,15 +541,23 @@ describe('isRecipientTurnBySigningOrder', () => { expect(isRecipientTurnBySigningOrder(recipients, recipients[1])).toBe(true); }); - it('treats recipients without a signing order as a parallel tail group', () => { + it('sequences recipients without a signing order one at a time by id', () => { const recipients = [ recipient(1, 1, SigningStatus.SIGNED), - recipient(2, null, SigningStatus.NOT_SIGNED), recipient(3, null, SigningStatus.NOT_SIGNED), + recipient(2, null, SigningStatus.NOT_SIGNED), ]; - expect(isRecipientTurnBySigningOrder(recipients, recipients[1])).toBe(true); expect(isRecipientTurnBySigningOrder(recipients, recipients[2])).toBe(true); + expect(isRecipientTurnBySigningOrder(recipients, recipients[1])).toBe(false); + }); + + it('orders group members by id when strictly sequential', () => { + const recipients = [recipient(2, 1, SigningStatus.NOT_SIGNED), recipient(1, 1, SigningStatus.NOT_SIGNED)]; + + expect(isRecipientTurnBySigningOrder(recipients, recipients[0], { strictlySequential: true })).toBe(false); + expect(isRecipientTurnBySigningOrder(recipients, recipients[1], { strictlySequential: true })).toBe(true); + expect(isRecipientTurnBySigningOrder(recipients, recipients[0])).toBe(true); }); }); @@ -554,6 +607,18 @@ describe('getRecipientsInActiveSigningStep', () => { expect(getRecipientsInActiveSigningStep(recipients)).toEqual([]); }); + + it('activates a single unordered recipient at a time, lowest id first', () => { + const recipients = [candidate(7, null), candidate(5, null), candidate(9, null)]; + + expect(getRecipientsInActiveSigningStep(recipients).map((r) => r.id)).toEqual([5]); + }); + + it('activates only the lowest id of a group when strictly sequential', () => { + const recipients = [candidate(4, 1), candidate(3, 1), candidate(5, 2)]; + + expect(getRecipientsInActiveSigningStep(recipients, { strictlySequential: true }).map((r) => r.id)).toEqual([3]); + }); }); describe('getNextDictatableRecipient', () => { @@ -643,4 +708,14 @@ describe('getNextDictatableRecipient', () => { expect(getNextDictatableRecipient({ recipients, currentRecipientId: 1 })).toBeNull(); }); + + it('treats the next unordered recipient by id as a single-recipient step', () => { + const recipients = [ + recipient(1, null, SigningStatus.NOT_SIGNED), + recipient(2, null, SigningStatus.NOT_SIGNED), + recipient(3, null, SigningStatus.NOT_SIGNED), + ]; + + expect(getNextDictatableRecipient({ recipients, currentRecipientId: 1 })?.id).toBe(2); + }); }); diff --git a/packages/lib/utils/recipient-groups.ts b/packages/lib/utils/recipient-groups.ts index f9c4856d3..4a76ba2ad 100644 --- a/packages/lib/utils/recipient-groups.ts +++ b/packages/lib/utils/recipient-groups.ts @@ -1,60 +1,53 @@ import type { Recipient } from '@prisma/client'; import { SigningStatus } from '@prisma/client'; -import { isCcRecipient } from './recipients'; +import type { PositionedRecipient } from './recipients'; +import { + hasSigningOrder, + isCcRecipient, + isRecipientBefore, + isSameSigningStep, + sortRecipientsBySigningPosition, +} from './recipients'; /** - * A recipient "step" is the set of non-CC recipients sharing a signing order. - * A step with 2 or more members is a "signing group": members may act in any - * order among themselves, and the next step only unlocks once every member of - * the group has completed their action. + * A recipient "step" is the set of non-CC recipients sharing an explicit + * signing order. A step with 2 or more members is a "signing group": members + * may act in any order among themselves, and the next step only unlocks once + * every member of the group has completed their action. + * + * A recipient without a signing order is always a single-member step (see + * `PositionedRecipient`). */ -type GroupableRecipient = Pick & { - signingOrder?: number | null; -}; +type GroupableRecipient = Pick & PositionedRecipient; export type RecipientStep = { /** - * The signing order shared by all members of the step. + * Null for a legacy unordered recipient. */ - order: number; + order: number | null; members: T[]; }; -const UNORDERED = Number.MAX_SAFE_INTEGER; - -/** - * The signing order to sort and group by — a missing order means LAST. - */ -export const effectiveSigningOrder = (recipient: { signingOrder?: number | null }) => - recipient.signingOrder ?? UNORDERED; - -/** - * Derives the ordered list of steps from a list of recipients. - * - * - Non-CC recipients who share a signing order form a signing group. - * - Recipients without a signing order share a single tail group. - * - CC recipients are returned separately and never belong to a group. - */ export const groupRecipientsBySigningOrder = (recipients: T[]) => { const ccRecipients = recipients.filter((recipient) => isCcRecipient(recipient)); - const nonCcRecipients = recipients.filter((recipient) => !isCcRecipient(recipient)); + const nonCcRecipients = sortRecipientsBySigningPosition(recipients.filter((recipient) => !isCcRecipient(recipient))); - const membersByOrder = new Map(); + const steps: RecipientStep[] = []; for (const recipient of nonCcRecipients) { - const order = effectiveSigningOrder(recipient); - const members = membersByOrder.get(order) ?? []; + const lastStep = steps[steps.length - 1]; - members.push(recipient); - membersByOrder.set(order, members); + if (lastStep && lastStep.order !== null && isSameSigningStep(lastStep.members[0], recipient)) { + lastStep.members.push(recipient); + + continue; + } + + steps.push({ order: hasSigningOrder(recipient) ? recipient.signingOrder : null, members: [recipient] }); } - const steps: RecipientStep[] = [...membersByOrder.entries()] - .sort(([orderA], [orderB]) => orderA - orderB) - .map(([order, members]) => ({ order, members })); - return { steps, ccRecipients }; }; @@ -70,11 +63,26 @@ export const getLastLockedStepIndex = ( -1, ); +/** + * Numbers sort ahead of unordered recipients, so once a locked recipient holds + * no order, numbering anything behind it would move that recipient ahead of + * someone who has already acted. Everything must stay unordered, sequenced by id. + */ +export const isSigningOrderFrozen = ( + steps: RecipientStep[], + canUpdateRecipient: (recipient: T) => boolean = () => true, +): boolean => { + const lastLockedStepIndex = getLastLockedStepIndex(steps, canUpdateRecipient); + + return lastLockedStepIndex !== -1 && steps[lastLockedStepIndex].order === null; +}; + /** * Dense-renumbers steps to 1..K while preserving groups (duplicate orders). * * - Locked steps keep their persisted order * - Editable steps never collide into a locked step's number + * - A frozen ordering (see `isSigningOrderFrozen`) is returned untouched * - CC recipients move to the tail with an undefined order * - The returned array is re-ordered by step sequence */ @@ -85,14 +93,15 @@ export const normalizeGroupedSigningOrders = ( const { steps, ccRecipients } = groupRecipientsBySigningOrder(recipients); const lastLockedStepIndex = getLastLockedStepIndex(steps, canUpdateRecipient); + const isFrozen = isSigningOrderFrozen(steps, canUpdateRecipient); let nextOrder = 1; const normalizedSteps = steps.map((step, index) => { // Locked steps hold persisted orders. Keep them exactly as they are, even // when sparse. - if (index <= lastLockedStepIndex) { - const order = step.order === UNORDERED ? undefined : step.order; + if (isFrozen || index <= lastLockedStepIndex) { + const order = step.order ?? undefined; if (order !== undefined) { nextOrder = Math.max(nextOrder, order + 1); @@ -116,6 +125,26 @@ export const normalizeGroupedSigningOrders = ( type EditorRecipient = GroupableRecipient & { formId: string }; +/** + * Editor operations work on the normalized state so every editable step + * carries a number. + */ +const prepareEditorRecipients = ( + recipients: T[], + canUpdateRecipient: (recipient: T) => boolean = () => true, +) => { + const normalized = normalizeGroupedSigningOrders(recipients, canUpdateRecipient); + const { steps, ccRecipients } = groupRecipientsBySigningOrder(normalized); + + return { + recipients: normalized, + steps, + ccRecipients, + lastLockedStepIndex: getLastLockedStepIndex(steps, canUpdateRecipient), + isFrozen: isSigningOrderFrozen(steps, canUpdateRecipient), + }; +}; + /** * Merges all members of the source step into the target step. */ @@ -125,26 +154,27 @@ export const mergeSteps = ( targetStepIndex: number, canUpdateRecipient?: (recipient: T) => boolean, ): Array => { - const { steps } = groupRecipientsBySigningOrder(recipients); + const prepared = prepareEditorRecipients(recipients, canUpdateRecipient); - const sourceStep = steps[sourceStepIndex]; - const targetStep = steps[targetStepIndex]; - const lastLockedStepIndex = getLastLockedStepIndex(steps, canUpdateRecipient); + const sourceStep = prepared.steps[sourceStepIndex]; + const targetStep = prepared.steps[targetStepIndex]; if ( + prepared.isFrozen || !sourceStep || !targetStep || + targetStep.order === null || sourceStepIndex === targetStepIndex || - sourceStepIndex <= lastLockedStepIndex || - targetStepIndex <= lastLockedStepIndex + sourceStepIndex <= prepared.lastLockedStepIndex || + targetStepIndex <= prepared.lastLockedStepIndex ) { - return normalizeGroupedSigningOrders(recipients, canUpdateRecipient); + return prepared.recipients; } const sourceFormIds = new Set(sourceStep.members.map((member) => member.formId)); // Source members join after the target step's existing members. - const remaining = recipients.filter((recipient) => !sourceFormIds.has(recipient.formId)); + const remaining = prepared.recipients.filter((recipient) => !sourceFormIds.has(recipient.formId)); const lastMemberFormId = targetStep.members[targetStep.members.length - 1].formId; const insertAfterIndex = remaining.findIndex((recipient) => recipient.formId === lastMemberFormId); @@ -168,27 +198,26 @@ export const moveRecipientToStep = ( targetStepIndex: number, canUpdateRecipient?: (recipient: T) => boolean, ): Array => { - const { steps } = groupRecipientsBySigningOrder(recipients); + const prepared = prepareEditorRecipients(recipients, canUpdateRecipient); - const targetStep = steps[targetStepIndex]; - const mover = recipients.find((recipient) => recipient.formId === formId); - const lastLockedStepIndex = getLastLockedStepIndex(steps, canUpdateRecipient); - const moverStepIndex = steps.findIndex((step) => step.members.some((member) => member.formId === formId)); + const targetStep = prepared.steps[targetStepIndex]; + const mover = prepared.recipients.find((recipient) => recipient.formId === formId); + const moverStepIndex = prepared.steps.findIndex((step) => step.members.some((member) => member.formId === formId)); - if (!targetStep || !mover || isCcRecipient(mover)) { - return normalizeGroupedSigningOrders(recipients, canUpdateRecipient); + if (prepared.isFrozen || !targetStep || targetStep.order === null || !mover || isCcRecipient(mover)) { + return prepared.recipients; } // Neither the recipient nor the destination may sit in the locked region. - if (targetStepIndex <= lastLockedStepIndex || moverStepIndex <= lastLockedStepIndex) { - return normalizeGroupedSigningOrders(recipients, canUpdateRecipient); + if (targetStepIndex <= prepared.lastLockedStepIndex || moverStepIndex <= prepared.lastLockedStepIndex) { + return prepared.recipients; } if (targetStep.members.some((member) => member.formId === formId)) { - return normalizeGroupedSigningOrders(recipients, canUpdateRecipient); + return prepared.recipients; } - const remaining = recipients.filter((recipient) => recipient.formId !== formId); + const remaining = prepared.recipients.filter((recipient) => recipient.formId !== formId); const lastMemberFormId = targetStep.members[targetStep.members.length - 1].formId; const insertAfterIndex = remaining.findIndex((recipient) => recipient.formId === lastMemberFormId); @@ -213,33 +242,38 @@ export const extractRecipientToNewStep = ( insertStepIndex: number, canUpdateRecipient?: (recipient: T) => boolean, ): Array => { - const { steps } = groupRecipientsBySigningOrder(recipients); + const prepared = prepareEditorRecipients(recipients, canUpdateRecipient); - const mover = recipients.find((recipient) => recipient.formId === formId); + const mover = prepared.recipients.find((recipient) => recipient.formId === formId); - if (!mover || isCcRecipient(mover)) { - return normalizeGroupedSigningOrders(recipients, canUpdateRecipient); + if (prepared.isFrozen || !mover || isCcRecipient(mover)) { + return prepared.recipients; } - const currentStepIndex = steps.findIndex((step) => step.members.some((member) => member.formId === formId)); - const isSoloStep = currentStepIndex !== -1 && steps[currentStepIndex].members.length === 1; - const lastLockedStepIndex = getLastLockedStepIndex(steps, canUpdateRecipient); + const currentStepIndex = prepared.steps.findIndex((step) => step.members.some((member) => member.formId === formId)); + const isSoloStep = currentStepIndex !== -1 && prepared.steps[currentStepIndex].members.length === 1; // Dropping a solo step into the gap directly above or below itself is a no-op. if (isSoloStep && (insertStepIndex === currentStepIndex || insertStepIndex === currentStepIndex + 1)) { - return normalizeGroupedSigningOrders(recipients, canUpdateRecipient); + return prepared.recipients; } // Gap N sits before step N, so inserting at or before the last locked step // would land the recipient inside the locked region. - if (insertStepIndex <= lastLockedStepIndex || currentStepIndex <= lastLockedStepIndex) { - return normalizeGroupedSigningOrders(recipients, canUpdateRecipient); + if (insertStepIndex <= prepared.lastLockedStepIndex || currentStepIndex <= prepared.lastLockedStepIndex) { + return prepared.recipients; } - const insertOrder = - insertStepIndex >= steps.length ? (steps[steps.length - 1]?.order ?? 0) + 1 : steps[insertStepIndex].order - 0.5; + // Every step past the locked region is numbered after normalization. + const lastStepOrder = prepared.steps[prepared.steps.length - 1]?.order ?? 0; + const insertStepOrder = prepared.steps[insertStepIndex]?.order; - const updated = recipients.map((recipient) => + const insertOrder = + insertStepIndex >= prepared.steps.length || insertStepOrder === null || insertStepOrder === undefined + ? lastStepOrder + 1 + : insertStepOrder - 0.5; + + const updated = prepared.recipients.map((recipient) => recipient.formId === formId ? { ...recipient, signingOrder: insertOrder } : recipient, ); @@ -258,20 +292,19 @@ export const reorderStep = ( toStepIndex: number, canUpdateRecipient: (recipient: T) => boolean = () => true, ): Array => { - const { steps, ccRecipients } = groupRecipientsBySigningOrder(recipients); - - const lastLockedStepIndex = getLastLockedStepIndex(steps, canUpdateRecipient); + const prepared = prepareEditorRecipients(recipients, canUpdateRecipient); if ( - !steps[fromStepIndex] || + prepared.isFrozen || + !prepared.steps[fromStepIndex] || fromStepIndex === toStepIndex || - fromStepIndex <= lastLockedStepIndex || - toStepIndex <= lastLockedStepIndex + fromStepIndex <= prepared.lastLockedStepIndex || + toStepIndex <= prepared.lastLockedStepIndex ) { - return normalizeGroupedSigningOrders(recipients, canUpdateRecipient); + return prepared.recipients; } - const reorderedSteps = [...steps]; + const reorderedSteps = [...prepared.steps]; const [movedStep] = reorderedSteps.splice(fromStepIndex, 1); reorderedSteps.splice(Math.min(toStepIndex, reorderedSteps.length), 0, movedStep); @@ -280,71 +313,131 @@ export const reorderStep = ( // position and their persisted order. The moved tail is numbered above the // highest locked order so it still sorts after them. const highestLockedOrder = reorderedSteps - .slice(0, lastLockedStepIndex + 1) - .reduce((highest, step) => (step.order === UNORDERED ? highest : Math.max(highest, step.order)), 0); + .slice(0, prepared.lastLockedStepIndex + 1) + .reduce((highest, step) => (step.order === null ? highest : Math.max(highest, step.order)), 0); const updated = [ ...reorderedSteps.flatMap((step, index) => { - if (index <= lastLockedStepIndex) { + if (index <= prepared.lastLockedStepIndex) { return step.members; } - const order = highestLockedOrder + (index - lastLockedStepIndex); + const order = highestLockedOrder + (index - prepared.lastLockedStepIndex); return step.members.map((member) => ({ ...member, signingOrder: order })); }), - ...ccRecipients, + ...prepared.ccRecipients, ]; return normalizeGroupedSigningOrders(updated, canUpdateRecipient); }; -type SignableRecipient = Pick & { - signingOrder?: number | null; +/** + * Dissolves a group into consecutive standalone steps preserving relative order. + */ +export const ungroupStep = ( + recipients: T[], + stepIndex: number, + canUpdateRecipient?: (recipient: T) => boolean, +): Array => { + const prepared = prepareEditorRecipients(recipients, canUpdateRecipient); + + const step = prepared.steps[stepIndex]; + + // Splitting a locked step would rewrite persisted orders. + if ( + prepared.isFrozen || + !step || + step.order === null || + step.members.length < 2 || + stepIndex <= prepared.lastLockedStepIndex + ) { + return prepared.recipients; + } + + const stepOrder = step.order; + const offsetByFormId = new Map(step.members.map((member, index) => [member.formId, index])); + + const updated = prepared.recipients.map((recipient) => { + const offset = offsetByFormId.get(recipient.formId); + + if (offset === undefined) { + return recipient; + } + + return { ...recipient, signingOrder: stepOrder + offset / (step.members.length + 1) }; + }); + + return normalizeGroupedSigningOrders(updated, canUpdateRecipient); +}; + +type SignableRecipient = Pick & PositionedRecipient; + +type SequencingOptions = { + /** + * See `isRecipientBefore`. Required for AES/QES envelopes. + */ + strictlySequential?: boolean; }; /** * Whether it is the recipient's turn to act under SEQUENTIAL signing. * - * - A recipient may act if no non-CC recipient with a lower order is still unsigned - * - Recipients sharing a signing order never block each other. + * - A recipient may act once every non-CC recipient positioned before them has signed. + * - Recipients sharing an explicit signing order never block each other, unless + * `strictlySequential` is set. * - Callers must check the document is in SEQUENTIAL mode. */ export const isRecipientTurnBySigningOrder = ( recipients: T[], - currentRecipient: { signingOrder?: number | null }, -): boolean => { - const currentOrder = effectiveSigningOrder(currentRecipient); - - return !recipients.some( + currentRecipient: PositionedRecipient, + options: SequencingOptions = {}, +): boolean => + !recipients.some( (recipient) => !isCcRecipient(recipient) && recipient.signingStatus !== SigningStatus.SIGNED && - effectiveSigningOrder(recipient) < currentOrder, + isRecipientBefore(recipient, currentRecipient, options), ); -}; /** - * Every pending recipient sharing the lowest pending signing order — the "active step". + * Every pending recipient in the earliest pending step — the "active step". * - * - Two or more members form a signing group and act in parallel. + * - Two or more members form a signing group and act in parallel, unless + * `strictlySequential` is set, in which case only the first member by id is + * active. * - Pending means non-CC and NOT_SIGNED; rejected recipients are excluded so * the flow never re-activates somebody who declined. * - Pass the full recipient list: filtering happens here so every caller * agrees on what "pending" means. */ -export const getRecipientsInActiveSigningStep = (recipients: T[]): T[] => { - const pendingRecipients = recipients.filter( - (recipient) => !isCcRecipient(recipient) && recipient.signingStatus === SigningStatus.NOT_SIGNED, +export const getRecipientsInActiveSigningStep = ( + recipients: T[], + options: SequencingOptions = {}, +): T[] => { + const pendingRecipients = sortRecipientsBySigningPosition( + recipients.filter((recipient) => !isCcRecipient(recipient) && recipient.signingStatus === SigningStatus.NOT_SIGNED), ); - if (pendingRecipients.length === 0) { + const [first] = pendingRecipients; + + if (!first) { return []; } - const minOrder = Math.min(...pendingRecipients.map((recipient) => effectiveSigningOrder(recipient))); + const activeStep = pendingRecipients.filter( + (recipient) => recipient === first || isSameSigningStep(recipient, first), + ); - return pendingRecipients.filter((recipient) => effectiveSigningOrder(recipient) === minOrder); + if (!options.strictlySequential) { + return activeStep; + } + + return [ + activeStep.reduce((earliest, recipient) => + isRecipientBefore(recipient, earliest, options) ? recipient : earliest, + ), + ]; }; /** @@ -352,7 +445,7 @@ export const getRecipientsInActiveSigningStep = (re * completion, or null when dictation does not apply: * * - the current recipient must be the last unsigned member of their step, and - * - the next step must contain exactly one recipient. + * - the next step must contain exactly one pending recipient. */ export const getNextDictatableRecipient = >({ recipients, @@ -367,13 +460,11 @@ export const getNextDictatableRecipient = recipient.id !== currentRecipientId && !isCcRecipient(recipient) && - effectiveSigningOrder(recipient) === currentOrder && + isSameSigningStep(recipient, currentRecipient) && recipient.signingStatus !== SigningStatus.SIGNED, ); @@ -383,7 +474,7 @@ export const getNextDictatableRecipient = effectiveSigningOrder(recipient) > currentOrder); + const laterRecipients = recipients.filter((recipient) => isRecipientBefore(currentRecipient, recipient)); const nextStep = getRecipientsInActiveSigningStep(laterRecipients); @@ -393,35 +484,3 @@ export const getNextDictatableRecipient = ( - recipients: T[], - stepIndex: number, - canUpdateRecipient?: (recipient: T) => boolean, -): Array => { - const { steps } = groupRecipientsBySigningOrder(recipients); - - const step = steps[stepIndex]; - - // Splitting a locked step would rewrite persisted orders. - if (!step || step.members.length < 2 || stepIndex <= getLastLockedStepIndex(steps, canUpdateRecipient)) { - return normalizeGroupedSigningOrders(recipients, canUpdateRecipient); - } - - const offsetByFormId = new Map(step.members.map((member, index) => [member.formId, index])); - - const updated = recipients.map((recipient) => { - const offset = offsetByFormId.get(recipient.formId); - - if (offset === undefined) { - return recipient; - } - - return { ...recipient, signingOrder: step.order + offset / (step.members.length + 1) }; - }); - - return normalizeGroupedSigningOrders(updated, canUpdateRecipient); -}; diff --git a/packages/lib/utils/recipient-queries.test.ts b/packages/lib/utils/recipient-queries.test.ts index 8286e23f2..8d206f42c 100644 --- a/packages/lib/utils/recipient-queries.test.ts +++ b/packages/lib/utils/recipient-queries.test.ts @@ -1,82 +1,42 @@ import { RecipientRole } from '@prisma/client'; import { describe, expect, it } from 'vitest'; -import { - getAssistableRecipientsWhereInput, - getLaterSigningStepRecipientsWhereInput, - getRecipientFieldsWhereInput, -} from './recipient-queries'; +import { getLaterSigningStepRecipientsWhereInput, getRecipientFieldsWhereInput } from './recipient-queries'; describe('getLaterSigningStepRecipientsWhereInput', () => { - it('scopes ordered assistants to their own envelope', () => { - const where = getLaterSigningStepRecipientsWhereInput({ signingOrder: 2, envelopeId: 'envelope_1' }); + it('follows a numbered assistant with higher numbers and every unordered recipient', () => { + const where = getLaterSigningStepRecipientsWhereInput({ id: 10, signingOrder: 2, envelopeId: 'envelope_1' }); - // Envelope scoping must be baked into the predicate itself — the - // signingOrder arms alone would match recipients across every envelope. expect(where).toEqual({ envelopeId: 'envelope_1', OR: [{ signingOrder: { gt: 2 } }, { signingOrder: null }], }); }); - // A non-finite order bypassing the types would emit `{ gt: undefined }` / - // `{ gt: NaN }`, which Prisma silently drops — inverting the predicate into - // match-everything. The runtime backstop must throw instead of failing open. - it('throws when a non-numeric signing order bypasses the types', () => { - expect(() => - getLaterSigningStepRecipientsWhereInput({ - signingOrder: undefined as unknown as number, - envelopeId: 'envelope_1', - }), - ).toThrow(); - - expect(() => - getLaterSigningStepRecipientsWhereInput({ - signingOrder: NaN, - envelopeId: 'envelope_1', - }), - ).toThrow(); - }); -}); - -describe('getAssistableRecipientsWhereInput', () => { - it('scopes to the envelope and matches self plus strictly later steps', () => { - const where = getAssistableRecipientsWhereInput({ id: 10, signingOrder: 2, envelopeId: 'envelope_1' }); + it('follows an unordered assistant with later unordered recipients only', () => { + const where = getLaterSigningStepRecipientsWhereInput({ id: 10, signingOrder: null, envelopeId: 'envelope_1' }); expect(where).toEqual({ envelopeId: 'envelope_1', - OR: [ - { id: 10 }, - { - envelopeId: 'envelope_1', - OR: [{ signingOrder: { gt: 2 } }, { signingOrder: null }], - }, - ], + signingOrder: null, + id: { gt: 10 }, }); }); - it('matches only self for a null-order assistant', () => { - // A null-order assistant sits in the last step: nobody comes after them, - // so no later-step predicate exists at all — just the self match. - expect(getAssistableRecipientsWhereInput({ id: 10, signingOrder: null, envelopeId: 'envelope_1' })).toEqual({ - envelopeId: 'envelope_1', - id: 10, - }); - }); - - // `{ gt: undefined }` is silently dropped by Prisma, turning a comparison - // arm into match-everything — non-numeric orders must fail closed to self. - it('matches only self when the signing order is undefined (fail closed)', () => { - expect( - getAssistableRecipientsWhereInput({ - id: 10, - signingOrder: undefined as unknown as number | null, + // `{ gt: undefined }` / `{ gt: NaN }` is silently dropped by Prisma, which + // would invert the predicate into match-everything. Fail closed instead. + it('throws when a non-finite id or order bypasses the types', () => { + expect(() => + getLaterSigningStepRecipientsWhereInput({ + id: undefined as unknown as number, + signingOrder: null, envelopeId: 'envelope_1', }), - ).toEqual({ - envelopeId: 'envelope_1', - id: 10, - }); + ).toThrow(); + + expect(() => + getLaterSigningStepRecipientsWhereInput({ id: 10, signingOrder: NaN, envelopeId: 'envelope_1' }), + ).toThrow(); }); }); @@ -88,25 +48,23 @@ describe('getRecipientFieldsWhereInput', () => { envelopeId: 'envelope_1', }; - it('restricts non-assistants to their own recipient row', () => { - const where = getRecipientFieldsWhereInput({ - recipient: { ...assistant, role: RecipientRole.SIGNER }, - allowAssistantAccessToOtherRecipients: true, - }); + it('restricts non-assistants and disallowed assistants to their own recipient row', () => { + expect( + getRecipientFieldsWhereInput({ + recipient: { ...assistant, role: RecipientRole.SIGNER }, + allowAssistantAccessToOtherRecipients: true, + }), + ).toEqual({ id: 10 }); - expect(where).toEqual({ id: 10 }); + expect( + getRecipientFieldsWhereInput({ + recipient: assistant, + allowAssistantAccessToOtherRecipients: false, + }), + ).toEqual({ id: 10 }); }); - it('restricts assistants to their own recipient row when access is not allowed', () => { - const where = getRecipientFieldsWhereInput({ - recipient: assistant, - allowAssistantAccessToOtherRecipients: false, - }); - - expect(where).toEqual({ id: 10 }); - }); - - it('scopes assistant access to unsigned recipients in the same envelope', () => { + it('scopes assistant access to unsigned recipients positioned after them in the same envelope', () => { const where = getRecipientFieldsWhereInput({ recipient: assistant, allowAssistantAccessToOtherRecipients: true, @@ -129,17 +87,4 @@ describe('getRecipientFieldsWhereInput', () => { ], }); }); - - it('collapses a null-order assistant to their own unsigned fields', () => { - const where = getRecipientFieldsWhereInput({ - recipient: { ...assistant, signingOrder: null }, - allowAssistantAccessToOtherRecipients: true, - }); - - expect(where).toEqual({ - signingStatus: { not: 'SIGNED' }, - envelopeId: 'envelope_1', - AND: [{ envelopeId: 'envelope_1', id: 10 }], - }); - }); }); diff --git a/packages/lib/utils/recipient-queries.ts b/packages/lib/utils/recipient-queries.ts index 99fad3f12..8daa5a26f 100644 --- a/packages/lib/utils/recipient-queries.ts +++ b/packages/lib/utils/recipient-queries.ts @@ -4,13 +4,31 @@ import { RecipientRole, SigningStatus } from '@prisma/client'; import { AppError, AppErrorCode } from '../errors/app-error'; /** - * Prisma `where` input matching recipients in the assistant's envelope in - * strictly LATER signing steps. + * Prisma `where` input matching recipients in the assistant's envelope + * positioned strictly after the assistant, mirroring + * `compareRecipientSigningPosition`. Same-step peers are never included. */ export const getLaterSigningStepRecipientsWhereInput = ( - assistant: Pick & { signingOrder: number }, + assistant: Pick, ): Prisma.RecipientWhereInput => { - // Backup guard. + // `{ gt: undefined }` is silently dropped by Prisma, turning the predicate + // into match-everything. + if (!Number.isFinite(assistant.id)) { + throw new AppError(AppErrorCode.INVALID_REQUEST, { + message: 'Assistant id must be a finite number', + }); + } + + if (assistant.signingOrder === null || assistant.signingOrder === undefined) { + return { + envelopeId: assistant.envelopeId, + signingOrder: null, + id: { + gt: assistant.id, + }, + }; + } + if (!Number.isFinite(assistant.signingOrder)) { throw new AppError(AppErrorCode.INVALID_REQUEST, { message: 'Assistant signing order must be a finite number', @@ -34,38 +52,26 @@ export const getLaterSigningStepRecipientsWhereInput = ( /** * Prisma `where` input matching every recipient an assistant may act for: - * themself, plus recipients in strictly later steps — never their own group - * peers. Scoped to the assistant's envelope. + * themself, plus recipients positioned strictly after them — never their own + * group peers. Scoped to the assistant's envelope. */ export const getAssistableRecipientsWhereInput = ( assistant: Pick, -): Prisma.RecipientWhereInput => { - if (typeof assistant.signingOrder !== 'number') { - return { - envelopeId: assistant.envelopeId, +): Prisma.RecipientWhereInput => ({ + envelopeId: assistant.envelopeId, + OR: [ + { id: assistant.id, - }; - } - - return { - envelopeId: assistant.envelopeId, - OR: [ - { - id: assistant.id, - }, - getLaterSigningStepRecipientsWhereInput({ - envelopeId: assistant.envelopeId, - signingOrder: assistant.signingOrder, - }), - ], - }; -}; + }, + getLaterSigningStepRecipientsWhereInput(assistant), + ], +}); /** * Prisma `where` input matching the recipients whose fields the token holder * may act on: non-assistants may only act on their own fields, while - * assistants may also act on fields of unsigned recipients in strictly later - * steps. + * assistants may also act on fields of unsigned recipients positioned after + * them. * * Shared by every field-level endpoint (sign / uninsert, V1 and V2) so the * RECIPIENT scoping rule cannot drift between them. @@ -81,7 +87,6 @@ export const getRecipientFieldsWhereInput = ({ return { id: recipient.id }; } - // Custom query to allow assistants to be able to interact other recipients in the same envelope. return { signingStatus: { not: SigningStatus.SIGNED, diff --git a/packages/lib/utils/recipients.test.ts b/packages/lib/utils/recipients.test.ts index d76f262ee..646419c0a 100644 --- a/packages/lib/utils/recipients.test.ts +++ b/packages/lib/utils/recipients.test.ts @@ -23,6 +23,34 @@ describe('recipient signing order helpers', () => { expect(sortRecipientsForSigningOrder(recipients).map((recipient) => recipient.id)).toEqual([2, 1]); }); + it('sorts recipients without a signing order after numbered ones, by id', () => { + const recipients = [ + { id: 3, role: RecipientRole.SIGNER, signingOrder: null }, + { id: 4, role: RecipientRole.CC, signingOrder: null }, + { id: 2, role: RecipientRole.SIGNER, signingOrder: null }, + { id: 5, role: RecipientRole.SIGNER, signingOrder: 9 }, + ]; + + expect(sortRecipientsForSigningOrder(recipients).map((recipient) => recipient.id)).toEqual([5, 2, 3, 4]); + }); + + it('treats an unordered assistant as last only when no unordered recipient has a higher id', () => { + expect( + isAssistantLastSigner([ + { id: 2, role: RecipientRole.ASSISTANT, signingOrder: null }, + { id: 1, role: RecipientRole.SIGNER, signingOrder: null }, + { id: 3, role: RecipientRole.SIGNER, signingOrder: 1 }, + ]), + ).toBe(true); + + expect( + isAssistantLastSigner([ + { id: 1, role: RecipientRole.ASSISTANT, signingOrder: null }, + { id: 2, role: RecipientRole.SIGNER, signingOrder: null }, + ]), + ).toBe(false); + }); + it('sorts and normalizes active recipient signing order and removes it from CC recipients', () => { const recipients = [ { id: 1, role: RecipientRole.CC, signingOrder: 1 }, diff --git a/packages/lib/utils/recipients.ts b/packages/lib/utils/recipients.ts index 44edf806c..252de4c2e 100644 --- a/packages/lib/utils/recipients.ts +++ b/packages/lib/utils/recipients.ts @@ -17,37 +17,107 @@ import { zEmail } from './zod'; export const RECIPIENT_ROLES_THAT_REQUIRE_FIELDS = [RecipientRole.SIGNER] as const; // signingOrder isn't required when submitting the recipient form (Zod: z.number().optional()) -type RecipientWithSigningOrder = Pick & Partial>; +type RecipientWithSigningOrder = Pick & PositionedRecipient; export const isCcRecipient = (recipient: Pick) => { return recipient.role === RecipientRole.CC; }; /** - * Whether an assistant sits in the last signing step (nobody after them to assist). - * - * Falls back to a positional check when no recipient carries a signing order. + * Recipients sharing an explicit signing order form a step and may act in + * parallel. A recipient without one (legacy rows predating automatic + * numbering) never shares a step: unordered recipients sort after every + * numbered recipient and among themselves by id, matching how the server has + * always processed them (`ORDER BY signingOrder NULLS LAST, id`). */ -export const isAssistantLastSigner = ( - recipients: Array & { signingOrder?: number | null }>, -) => { - const nonCcRecipients = recipients.filter((recipient) => !isCcRecipient(recipient)); +export type PositionedRecipient = { + id?: number | null; + signingOrder?: number | null; +}; - if (nonCcRecipients.length === 0) { +export const hasSigningOrder = (recipient: PositionedRecipient): recipient is { signingOrder: number } => + typeof recipient.signingOrder === 'number'; + +const hasPersistedId = (recipient: PositionedRecipient): recipient is { id: number } => + typeof recipient.id === 'number'; + +/** + * Unsaved (id-less) unordered recipients sort last, in input order. + */ +export const compareRecipientSigningPosition = (a: PositionedRecipient, b: PositionedRecipient): number => { + const aIsNumbered = hasSigningOrder(a); + const bIsNumbered = hasSigningOrder(b); + + if (aIsNumbered && bIsNumbered) { + return a.signingOrder - b.signingOrder; + } + + if (aIsNumbered !== bIsNumbered) { + return aIsNumbered ? -1 : 1; + } + + const aHasId = hasPersistedId(a); + const bHasId = hasPersistedId(b); + + if (aHasId && bHasId) { + return a.id - b.id; + } + + if (aHasId !== bHasId) { + return aHasId ? -1 : 1; + } + + return 0; +}; + +export const sortRecipientsBySigningPosition = (recipients: T[]): T[] => + [...recipients].sort(compareRecipientSigningPosition); + +export const isSameSigningStep = (a: PositionedRecipient, b: PositionedRecipient): boolean => + hasSigningOrder(a) && hasSigningOrder(b) && a.signingOrder === b.signingOrder; + +/** + * Whether `recipient` must act before `other`. + * + * `strictlySequential` also orders group members by id so no two recipients + * are ever eligible at once — required on AES/QES, where a TSP signature is + * computed over a document snapshot and overlapping signers would invalidate + * each other's /ByteRange. + */ +export const isRecipientBefore = ( + recipient: PositionedRecipient, + other: PositionedRecipient, + options: { strictlySequential?: boolean } = {}, +): boolean => { + const comparison = compareRecipientSigningPosition(recipient, other); + + if (comparison !== 0) { + return comparison < 0; + } + + if (!options.strictlySequential || !isSameSigningStep(recipient, other)) { return false; } - const hasAnySigningOrder = nonCcRecipients.some((recipient) => typeof recipient.signingOrder === 'number'); + return hasPersistedId(recipient) && hasPersistedId(other) && recipient.id < other.id; +}; - if (!hasAnySigningOrder) { - return nonCcRecipients[nonCcRecipients.length - 1]?.role === RecipientRole.ASSISTANT; +/** + * Whether an assistant sits in the last signing step (nobody after them to assist). + */ +export const isAssistantLastSigner = (recipients: RecipientWithSigningOrder[]) => { + const nonCcRecipients = sortRecipientsBySigningPosition(recipients.filter((recipient) => !isCcRecipient(recipient))); + + const lastRecipient = nonCcRecipients[nonCcRecipients.length - 1]; + + if (!lastRecipient) { + return false; } - const maxOrder = Math.max(...nonCcRecipients.map((recipient) => recipient.signingOrder ?? Number.MAX_SAFE_INTEGER)); - return nonCcRecipients.some( (recipient) => - (recipient.signingOrder ?? Number.MAX_SAFE_INTEGER) === maxOrder && recipient.role === RecipientRole.ASSISTANT, + recipient.role === RecipientRole.ASSISTANT && + (recipient === lastRecipient || isSameSigningStep(recipient, lastRecipient)), ); }; @@ -61,11 +131,7 @@ export const sortRecipientsForSigningOrder =