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 96b810af8..1acfa915d 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 @@ -8,6 +8,7 @@ import type { TDetectedRecipientSchema } from '@documenso/lib/server-only/ai/env import { ZRecipientAuthOptionsSchema } from '@documenso/lib/types/document-auth'; import { nanoid } from '@documenso/lib/universal/id'; import { + isAssistantLastSigner, isCcRecipient, normalizeRecipientSigningOrders, canRecipientBeModified as utilCanRecipientBeModified, @@ -164,13 +165,6 @@ export const EnvelopeEditorRecipientForm = () => { const activeRecipientCount = watchedSigners.filter((signer) => !isCcRecipient(signer)).length; - const isAssistantLastActiveRecipient = (signers: typeof watchedSigners) => { - const activeRecipients = signers.filter((signer) => !isCcRecipient(signer)); - const lastActiveRecipient = activeRecipients[activeRecipients.length - 1]; - - return lastActiveRecipient?.role === RecipientRole.ASSISTANT; - }; - const { fields: signers, remove: removeSigner } = useFieldArray({ control, name: 'signers', @@ -397,7 +391,7 @@ export const EnvelopeEditorRecipientForm = () => { shouldDirty: true, }); - if (isAssistantLastActiveRecipient(updatedSigners)) { + if (isAssistantLastSigner(updatedSigners)) { toast({ title: t`Warning: Assistant as last signer`, description: t`Having an assistant as the last signer means they will be unable to take any action as there are no subsequent signers to assist.`, @@ -440,7 +434,7 @@ export const EnvelopeEditorRecipientForm = () => { shouldDirty: true, }); - if (role === RecipientRole.ASSISTANT && isAssistantLastActiveRecipient(updatedSigners)) { + if (role === RecipientRole.ASSISTANT && isAssistantLastSigner(updatedSigners)) { toast({ title: t`Warning: Assistant as last signer`, description: t`Having an assistant as the last signer means they will be unable to take any action as there are no subsequent signers to assist.`, @@ -469,26 +463,26 @@ export const EnvelopeEditorRecipientForm = () => { return; } - const signersWithSigningOrder = currentSigners.filter((s) => !isCcRecipient(s)); - const signersWithoutSigningOrder = currentSigners.filter((s) => isCcRecipient(s)); - const currentSigningOrderIndex = signersWithSigningOrder.findIndex((s) => s.formId === signer.formId); + const nonCcSigners = currentSigners.filter((s) => !isCcRecipient(s)); + const ccSigners = currentSigners.filter((s) => isCcRecipient(s)); + const currentSigningOrderIndex = nonCcSigners.findIndex((s) => s.formId === signer.formId); if (currentSigningOrderIndex === -1) { return; } - const [reorderedSigner] = signersWithSigningOrder.splice(currentSigningOrderIndex, 1); - const newPosition = Math.min(Math.max(0, newOrder - 1), signersWithSigningOrder.length); - signersWithSigningOrder.splice(newPosition, 0, reorderedSigner); + const [reorderedSigner] = nonCcSigners.splice(currentSigningOrderIndex, 1); + const newPosition = Math.min(Math.max(0, newOrder - 1), nonCcSigners.length); + nonCcSigners.splice(newPosition, 0, reorderedSigner); - const updatedSigners = normalizeSigningOrders([...signersWithSigningOrder, ...signersWithoutSigningOrder]); + const updatedSigners = normalizeSigningOrders([...nonCcSigners, ...ccSigners]); form.setValue('signers', updatedSigners, { shouldValidate: true, shouldDirty: true, }); - if (signer.role === RecipientRole.ASSISTANT && isAssistantLastActiveRecipient(updatedSigners)) { + if (signer.role === RecipientRole.ASSISTANT && isAssistantLastSigner(updatedSigners)) { toast({ title: t`Warning: Assistant as last signer`, description: t`Having an assistant as the last signer means they will be unable to take any action as there are no subsequent signers to assist.`, diff --git a/packages/lib/client-only/hooks/use-editor-recipients.ts b/packages/lib/client-only/hooks/use-editor-recipients.ts index 4eae6418d..680af4ba3 100644 --- a/packages/lib/client-only/hooks/use-editor-recipients.ts +++ b/packages/lib/client-only/hooks/use-editor-recipients.ts @@ -6,9 +6,10 @@ import { DocumentSigningOrder, RecipientRole } from '@prisma/client'; import { useId } from 'react'; import type { UseFormReturn } from 'react-hook-form'; import { useForm } from 'react-hook-form'; -import { prop, sortBy } from 'remeda'; import { z } from 'zod'; +import { isCcRecipient, normalizeRecipientSigningOrders, sortRecipientsForSigningOrder } from '../../utils/recipients'; + const LocalRecipientSchema = z.object({ formId: z.string().min(1), id: z.number().optional(), @@ -55,13 +56,13 @@ export const useEditorRecipients = ({ envelope }: EditorRecipientsProps): UseEdi name: recipient.name, email: recipient.email, role: recipient.role, - signingOrder: recipient.signingOrder ?? index + 1, + signingOrder: isCcRecipient(recipient) ? undefined : (recipient.signingOrder ?? index + 1), actionAuth: ZRecipientAuthOptionsSchema.parse(recipient.authOptions)?.actionAuth ?? undefined, })); const signers: TLocalRecipient[] = formRecipients.length > 0 - ? sortBy(formRecipients, [prop('signingOrder'), 'asc'], [prop('id'), 'asc']) + ? normalizeRecipientSigningOrders(sortRecipientsForSigningOrder(formRecipients)) : [ { formId: initialId, diff --git a/packages/lib/utils/recipients.test.ts b/packages/lib/utils/recipients.test.ts new file mode 100644 index 000000000..832073607 --- /dev/null +++ b/packages/lib/utils/recipients.test.ts @@ -0,0 +1,81 @@ +import { RecipientRole } from '@prisma/client'; +import { describe, expect, it } from 'vitest'; + +import { + getRecipientSigningOrder, + isAssistantLastSigner, + normalizeRecipientSigningOrders, + sortRecipientsForSigningOrder, +} from './recipients'; + +describe('recipient signing order helpers', () => { + it('sorts CC recipients after ordered active recipients', () => { + const recipients = [ + { id: 1, role: RecipientRole.CC, signingOrder: 1 }, + { id: 2, role: RecipientRole.SIGNER, signingOrder: 2 }, + { id: 3, role: RecipientRole.APPROVER, signingOrder: 1 }, + ]; + + expect(sortRecipientsForSigningOrder(recipients).map((recipient) => recipient.id)).toEqual([3, 2, 1]); + }); + + it('keeps original order when recipients have the same signing order', () => { + const recipients = [ + { id: 2, role: RecipientRole.SIGNER, signingOrder: 1 }, + { id: 1, role: RecipientRole.APPROVER, signingOrder: 1 }, + ]; + + expect(sortRecipientsForSigningOrder(recipients).map((recipient) => recipient.id)).toEqual([2, 1]); + }); + + it('sorts and normalizes active recipient signing order and removes it from CC recipients', () => { + const recipients = [ + { id: 1, role: RecipientRole.CC, signingOrder: 1 }, + { id: 2, role: RecipientRole.SIGNER, signingOrder: 4 }, + { id: 3, role: RecipientRole.APPROVER, signingOrder: 2 }, + ]; + + expect(normalizeRecipientSigningOrders(sortRecipientsForSigningOrder(recipients))).toEqual([ + { id: 3, role: RecipientRole.APPROVER, signingOrder: 1 }, + { id: 2, role: RecipientRole.SIGNER, signingOrder: 2 }, + { id: 1, role: RecipientRole.CC, signingOrder: undefined }, + ]); + }); + + it('preserves caller order while normalizing signing order', () => { + const recipients = [ + { id: 2, role: RecipientRole.ASSISTANT, signingOrder: 2 }, + { id: 1, role: RecipientRole.SIGNER, signingOrder: 1 }, + { id: 3, role: RecipientRole.CC, signingOrder: 1 }, + ]; + + expect(normalizeRecipientSigningOrders(recipients)).toEqual([ + { id: 2, role: RecipientRole.ASSISTANT, signingOrder: 1 }, + { id: 1, role: RecipientRole.SIGNER, signingOrder: 2 }, + { id: 3, role: RecipientRole.CC, signingOrder: undefined }, + ]); + }); + + it('coerces CC signing order to null for persistence', () => { + expect(getRecipientSigningOrder({ role: RecipientRole.CC, signingOrder: 1 })).toBeNull(); + expect(getRecipientSigningOrder({ role: RecipientRole.SIGNER, signingOrder: 1 })).toBe(1); + }); + + it('checks whether the last non-CC recipient is an assistant', () => { + expect( + isAssistantLastSigner([ + { role: RecipientRole.SIGNER }, + { role: RecipientRole.ASSISTANT }, + { role: RecipientRole.CC }, + ]), + ).toBe(true); + + expect( + isAssistantLastSigner([ + { role: RecipientRole.ASSISTANT }, + { role: RecipientRole.SIGNER }, + { role: RecipientRole.CC }, + ]), + ).toBe(false); + }); +}); diff --git a/packages/lib/utils/recipients.ts b/packages/lib/utils/recipients.ts index e2d402fa3..f3a2a6e92 100644 --- a/packages/lib/utils/recipients.ts +++ b/packages/lib/utils/recipients.ts @@ -54,14 +54,21 @@ export const sortRecipientsForSigningOrder = []) => { + const nonCcRecipients = recipients.filter((recipient) => !isCcRecipient(recipient)); + const lastNonCcRecipient = nonCcRecipients[nonCcRecipients.length - 1]; + + return lastNonCcRecipient?.role === RecipientRole.ASSISTANT; +}; + export const normalizeRecipientSigningOrders = ( recipients: T[], canUpdateRecipient: CanUpdateRecipient = canUpdateAnyRecipient, ): Array => { - const recipientsWithSigningOrder = recipients.filter((recipient) => !isCcRecipient(recipient)); + const nonCcRecipients = recipients.filter((recipient) => !isCcRecipient(recipient)); const ccRecipients = recipients.filter((recipient) => isCcRecipient(recipient)); - const normalizedRecipients = recipientsWithSigningOrder.map((recipient, index) => ({ + const normalizedNonCcRecipients = nonCcRecipients.map((recipient, index) => ({ ...recipient, signingOrder: canUpdateRecipient(recipient) ? index + 1 : (recipient.signingOrder ?? index + 1), })); @@ -71,7 +78,7 @@ export const normalizeRecipientSigningOrders = 0 - ? sortBy( - recipients.map((recipient, index) => ({ - nativeId: recipient.id, - formId: String(recipient.id), - name: recipient.name, - email: recipient.email, - role: recipient.role, - signingOrder: recipient.signingOrder ?? index + 1, - actionAuth: ZRecipientAuthOptionsSchema.parse(recipient.authOptions)?.actionAuth ?? undefined, - })), - [prop('signingOrder'), 'asc'], - [prop('nativeId'), 'asc'], + ? normalizeRecipientSigningOrders( + sortRecipientsForSigningOrder( + recipients.map((recipient, index) => ({ + nativeId: recipient.id, + formId: String(recipient.id), + name: recipient.name, + email: recipient.email, + role: recipient.role, + signingOrder: isCcRecipient(recipient) ? undefined : (recipient.signingOrder ?? index + 1), + actionAuth: ZRecipientAuthOptionsSchema.parse(recipient.authOptions)?.actionAuth ?? undefined, + })), + ), ) : defaultRecipients, signingOrder: signingOrder || DocumentSigningOrder.PARALLEL, @@ -167,18 +172,14 @@ export const AddSignersFormPartial = ({ }, [watchedSigners]); const normalizeSigningOrders = (signers: typeof watchedSigners) => { - return signers - .sort((a, b) => (a.signingOrder ?? 0) - (b.signingOrder ?? 0)) - .map((signer, index) => ({ ...signer, signingOrder: index + 1 })); + return normalizeRecipientSigningOrders(signers, (signer) => canRecipientBeModified(signer.nativeId)); }; + const activeRecipientCount = watchedSigners.filter((signer) => !isCcRecipient(signer)).length; + const onFormSubmit = form.handleSubmit(onSubmit); - const { - append: appendSigner, - fields: signers, - remove: removeSigner, - } = useFieldArray({ + const { fields: signers, remove: removeSigner } = useFieldArray({ control, name: 'signers', }); @@ -257,14 +258,31 @@ export const AddSignersFormPartial = ({ return utilCanRecipientBeModified(recipient, fields); }; + const appendNormalizedSigner = (signer: (typeof watchedSigners)[number], shouldFocus = false) => { + const updatedSigners = normalizeSigningOrders([...form.getValues('signers'), signer]); + + form.setValue('signers', updatedSigners, { + shouldValidate: true, + shouldDirty: true, + }); + + if (shouldFocus) { + const signerIndex = updatedSigners.findIndex((updatedSigner) => updatedSigner.formId === signer.formId); + + if (signerIndex !== -1) { + requestAnimationFrame(() => form.setFocus(`signers.${signerIndex}.email`)); + } + } + }; + const onAddSigner = () => { - appendSigner({ + appendNormalizedSigner({ formId: nanoid(12), name: '', email: '', role: RecipientRole.SIGNER, actionAuth: [], - signingOrder: signers.length > 0 ? (signers[signers.length - 1]?.signingOrder ?? 0) + 1 : 1, + signingOrder: activeRecipientCount + 1, }); }; @@ -309,18 +327,16 @@ export const AddSignersFormPartial = ({ form.setFocus(`signers.${emptySignerIndex}.email`); } else { - appendSigner( + appendNormalizedSigner( { formId: nanoid(12), name: user?.name ?? '', email: user?.email ?? '', role: RecipientRole.SIGNER, actionAuth: [], - signingOrder: signers.length > 0 ? (signers[signers.length - 1]?.signingOrder ?? 0) + 1 : 1, - }, - { - shouldFocus: true, + signingOrder: activeRecipientCount + 1, }, + true, ); void form.trigger('signers'); @@ -355,18 +371,14 @@ export const AddSignersFormPartial = ({ items.splice(insertIndex, 0, reorderedSigner); - const updatedSigners = items.map((signer, index) => ({ - ...signer, - signingOrder: !canRecipientBeModified(signer.nativeId) ? signer.signingOrder : index + 1, - })); + const updatedSigners = normalizeSigningOrders(items); form.setValue('signers', updatedSigners, { shouldValidate: true, shouldDirty: true, }); - const lastSigner = updatedSigners[updatedSigners.length - 1]; - if (lastSigner.role === RecipientRole.ASSISTANT) { + if (isAssistantLastSigner(updatedSigners)) { toast({ title: _(msg`Warning: Assistant as last signer`), description: _( @@ -401,18 +413,19 @@ export const AddSignersFormPartial = ({ return; } - const updatedSigners = currentSigners.map((signer, idx) => ({ - ...signer, - role: idx === index ? role : signer.role, - signingOrder: !canRecipientBeModified(signer.nativeId) ? signer.signingOrder : idx + 1, - })); + const updatedSigners = normalizeSigningOrders( + currentSigners.map((signer, idx) => ({ + ...signer, + role: idx === index ? role : signer.role, + })), + ); form.setValue('signers', updatedSigners, { shouldValidate: true, shouldDirty: true, }); - if (role === RecipientRole.ASSISTANT && index === updatedSigners.length - 1) { + if (role === RecipientRole.ASSISTANT && isAssistantLastSigner(updatedSigners)) { toast({ title: _(msg`Warning: Assistant as last signer`), description: _( @@ -439,22 +452,30 @@ export const AddSignersFormPartial = ({ const currentSigners = form.getValues('signers'); const signer = currentSigners[index]; - // Remove signer from current position and insert at new position - const remainingSigners = currentSigners.filter((_, idx) => idx !== index); - const newPosition = Math.min(Math.max(0, newOrder - 1), currentSigners.length - 1); - remainingSigners.splice(newPosition, 0, signer); + if (isCcRecipient(signer)) { + return; + } - const updatedSigners = remainingSigners.map((s, idx) => ({ - ...s, - signingOrder: !canRecipientBeModified(s.nativeId) ? s.signingOrder : idx + 1, - })); + const nonCcSigners = currentSigners.filter((s) => !isCcRecipient(s)); + const ccSigners = currentSigners.filter((s) => isCcRecipient(s)); + const currentSigningOrderIndex = nonCcSigners.findIndex((s) => s.formId === signer.formId); + + if (currentSigningOrderIndex === -1) { + return; + } + + const [reorderedSigner] = nonCcSigners.splice(currentSigningOrderIndex, 1); + const newPosition = Math.min(Math.max(0, newOrder - 1), nonCcSigners.length); + nonCcSigners.splice(newPosition, 0, reorderedSigner); + + const updatedSigners = normalizeSigningOrders([...nonCcSigners, ...ccSigners]); form.setValue('signers', updatedSigners, { shouldValidate: true, shouldDirty: true, }); - if (signer.role === RecipientRole.ASSISTANT && newPosition === remainingSigners.length - 1) { + if (signer.role === RecipientRole.ASSISTANT && isAssistantLastSigner(updatedSigners)) { toast({ title: _(msg`Warning: Assistant as last signer`), description: _( @@ -470,10 +491,12 @@ export const AddSignersFormPartial = ({ setShowSigningOrderConfirmation(false); const currentSigners = form.getValues('signers'); - const updatedSigners = currentSigners.map((signer) => ({ - ...signer, - role: signer.role === RecipientRole.ASSISTANT ? RecipientRole.SIGNER : signer.role, - })); + const updatedSigners = normalizeSigningOrders( + currentSigners.map((signer) => ({ + ...signer, + role: signer.role === RecipientRole.ASSISTANT ? RecipientRole.SIGNER : signer.role, + })), + ); form.setValue('signers', updatedSigners, { shouldValidate: true, @@ -627,6 +650,7 @@ export const AddSignersFormPartial = ({ isDragDisabled={ !isSigningOrderSequential || isSubmitting || + isCcRecipient(signer) || !canRecipientBeModified(signer.nativeId) || !signer.signingOrder } @@ -648,7 +672,9 @@ export const AddSignersFormPartial = ({ 'grid-cols-12 pr-3': isSigningOrderSequential, })} > - {isSigningOrderSequential && ( + {isSigningOrderSequential && isCcRecipient(signer) &&
} + + {isSigningOrderSequential && !isCcRecipient(signer) && (