refactor(pdf): simplify AcroForm import code

apps/remix/app/components/general/envelope-editor/envelope-editor-fields-page.tsx

@@ -98,9 +98,8 @@ export const EnvelopeEditorFieldsPage = () => {
   const currentEnvelopeItemRevision = currentEnvelopeItem
     ? `${currentEnvelopeItem.id}:${currentEnvelopeItem.documentDataId}`
     : null;
-  const currentItemHasAcroForm = currentEnvelopeItemRevision
-    ? acroFormHasFieldsByItemRevision[currentEnvelopeItemRevision] === true
-    : false;
+  const currentItemHasAcroForm =
+    currentEnvelopeItemRevision !== null && acroFormHasFieldsByItemRevision[currentEnvelopeItemRevision] === true;
 
   const onAcroFormDetected = useCallback(
     (hasFields: boolean) => {

apps/remix/app/components/general/pdf-viewer/envelope-pdf-viewer.tsx

@@ -16,12 +16,7 @@ export type EnvelopePdfViewerProps = {
   errorMessage: { title: MessageDescriptor; description: MessageDescriptor } | null;
 } & Omit<PDFViewerProps, 'data'>;
 
-export const EnvelopePdfViewer = ({
-  errorMessage,
-  className,
-  onAcroFormDetected,
-  ...props
-}: EnvelopePdfViewerProps) => {
+export const EnvelopePdfViewer = ({ errorMessage, className, ...props }: EnvelopePdfViewerProps) => {
   const { t } = useLingui();
 
   const $el = useRef<HTMLDivElement>(null);
@@ -55,7 +50,6 @@ export const EnvelopePdfViewer = ({
       {...props}
       className={cn('h-full w-full max-w-[800px]', className)}
       data={currentEnvelopeItem.data}
-      onAcroFormDetected={onAcroFormDetected}
     />
   );
 };

packages/app-tests/e2e/scenarios/acroform-import.spec.ts

@@ -4,6 +4,7 @@ import { NEXT_PUBLIC_WEBAPP_URL } from '@documenso/lib/constants/app';
 import { UNSAFE_importAcroFormFieldsFromEnvelope } from '@documenso/lib/server-only/envelope-item/import-acroform-fields';
 import { UNSAFE_replaceEnvelopeItemPdf } from '@documenso/lib/server-only/envelope-item/replace-envelope-item-pdf';
 import { createApiToken } from '@documenso/lib/server-only/public-api/create-api-token';
+import type { ApiRequestMetadata } from '@documenso/lib/universal/extract-request-metadata';
 import { getFileServerSide } from '@documenso/lib/universal/upload/get-file.server';
 import { prisma } from '@documenso/prisma';
 import { EnvelopeType, RecipientRole } from '@documenso/prisma/client';
@@ -13,29 +14,61 @@ import type {
   TCreateEnvelopeResponse,
 } from '@documenso/trpc/server/envelope-router/create-envelope.types';
 import { PDF } from '@libpdf/core';
-import { expect, test } from '@playwright/test';
+import { type APIRequestContext, expect, test } from '@playwright/test';
 
 const WEBAPP_BASE_URL = NEXT_PUBLIC_WEBAPP_URL();
 const baseUrl = `${WEBAPP_BASE_URL}/api/v2-beta`;
 
 const ACROFORM_FIXTURE = fs.readFileSync(path.join(__dirname, '../../../../assets/acroform-import-test.pdf'));
 
+const ACROFORM_DOCUMENT_PAYLOAD: TCreateEnvelopePayload = {
+  type: EnvelopeType.DOCUMENT,
+  title: 'AcroForm document',
+  recipients: [
+    {
+      email: 'signer@example.com',
+      name: 'Signer',
+      role: RecipientRole.SIGNER,
+    },
+  ],
+};
+
+const API_REQUEST_METADATA: ApiRequestMetadata = {
+  requestMetadata: {},
+  source: 'apiV1',
+  auth: 'api',
+};
+
+type TestUser = Awaited<ReturnType<typeof seedUser>>['user'];
+
+const seedUserWithApiToken = async (): Promise<{ token: string; user: TestUser }> => {
+  const { user, team } = await seedUser();
+  const { token } = await createApiToken({
+    userId: user.id,
+    teamId: team.id,
+    tokenName: 'test',
+    expiresIn: null,
+  });
+
+  return { token, user };
+};
+
 const pdfHasFormFields = async (pdf: Uint8Array): Promise<boolean> => {
   const pdfDoc = await PDF.load(new Uint8Array(pdf));
   const form = pdfDoc.getForm();
 
-  return Boolean(form && form.fieldCount > 0);
+  return (form?.fieldCount ?? 0) > 0;
 };
 
 const uploadAcroFormEnvelope = async ({
   request,
   token,
-  payload,
+  payload = ACROFORM_DOCUMENT_PAYLOAD,
 }: {
-  request: import('@playwright/test').APIRequestContext;
+  request: APIRequestContext;
   token: string;
-  payload: TCreateEnvelopePayload;
-}) => {
+  payload?: TCreateEnvelopePayload;
+}): Promise<TCreateEnvelopeResponse> => {
   const formData = new FormData();
 
   formData.append('payload', JSON.stringify(payload));
@@ -66,29 +99,9 @@ test.describe.configure({
 
 test.describe('AcroForm Import', () => {
   test('upload does not create fields and preserves widgets in the stored PDF', async ({ request }) => {
-    const { user, team } = await seedUser();
-    const { token } = await createApiToken({
-      userId: user.id,
-      teamId: team.id,
-      tokenName: 'test',
-      expiresIn: null,
-    });
+    const { token } = await seedUserWithApiToken();
 
-    const response = await uploadAcroFormEnvelope({
-      request,
-      token,
-      payload: {
-        type: EnvelopeType.DOCUMENT,
-        title: 'AcroForm document',
-        recipients: [
-          {
-            email: 'signer@example.com',
-            name: 'Signer',
-            role: RecipientRole.SIGNER,
-          },
-        ],
-      },
-    });
+    const response = await uploadAcroFormEnvelope({ request, token });
 
     const envelope = await prisma.envelope.findUniqueOrThrow({
       where: { id: response.id },
@@ -106,29 +119,9 @@ test.describe('AcroForm Import', () => {
   });
 
   test('replacement preserves widgets in the stored PDF for later import', async ({ request }) => {
-    const { user, team } = await seedUser();
-    const { token } = await createApiToken({
-      userId: user.id,
-      teamId: team.id,
-      tokenName: 'test',
-      expiresIn: null,
-    });
+    const { token, user } = await seedUserWithApiToken();
 
-    const response = await uploadAcroFormEnvelope({
-      request,
-      token,
-      payload: {
-        type: EnvelopeType.DOCUMENT,
-        title: 'AcroForm document',
-        recipients: [
-          {
-            email: 'signer@example.com',
-            name: 'Signer',
-            role: RecipientRole.SIGNER,
-          },
-        ],
-      },
-    });
+    const response = await uploadAcroFormEnvelope({ request, token });
 
     const envelope = await loadEnvelopeForImport(response.id);
     const oldDocumentDataId = envelope.envelopeItems[0].documentDataId;
@@ -143,11 +136,7 @@ test.describe('AcroForm Import', () => {
         file: new File([ACROFORM_FIXTURE], 'replacement-acroform.pdf', { type: 'application/pdf' }),
       },
       user,
-      apiRequestMetadata: {
-        requestMetadata: {},
-        source: 'apiV1',
-        auth: 'api',
-      },
+      apiRequestMetadata: API_REQUEST_METADATA,
     });
 
     const after = await prisma.envelope.findUniqueOrThrow({
@@ -167,40 +156,16 @@ test.describe('AcroForm Import', () => {
   });
 
   test('import creates fields assigned to the signer, flattens the PDF, and emits audit logs', async ({ request }) => {
-    const { user, team } = await seedUser();
-    const { token } = await createApiToken({
-      userId: user.id,
-      teamId: team.id,
-      tokenName: 'test',
-      expiresIn: null,
-    });
+    const { token } = await seedUserWithApiToken();
 
-    const response = await uploadAcroFormEnvelope({
-      request,
-      token,
-      payload: {
-        type: EnvelopeType.DOCUMENT,
-        title: 'AcroForm document',
-        recipients: [
-          {
-            email: 'signer@example.com',
-            name: 'Signer',
-            role: RecipientRole.SIGNER,
-          },
-        ],
-      },
-    });
+    const response = await uploadAcroFormEnvelope({ request, token });
 
     const envelope = await loadEnvelopeForImport(response.id);
     const oldDocumentDataId = envelope.envelopeItems[0].documentDataId;
 
     const result = await UNSAFE_importAcroFormFieldsFromEnvelope({
       envelope,
-      apiRequestMetadata: {
-        requestMetadata: {},
-        source: 'apiV1',
-        auth: 'api',
-      },
+      apiRequestMetadata: API_REQUEST_METADATA,
     });
 
     expect(result.fieldsCreated).toBeGreaterThan(0);
@@ -241,13 +206,7 @@ test.describe('AcroForm Import', () => {
   });
 
   test('import creates a placeholder Recipient 1 SIGNER when no recipients exist', async ({ request }) => {
-    const { user, team } = await seedUser();
-    const { token } = await createApiToken({
-      userId: user.id,
-      teamId: team.id,
-      tokenName: 'test',
-      expiresIn: null,
-    });
+    const { token } = await seedUserWithApiToken();
 
     const response = await uploadAcroFormEnvelope({
       request,
@@ -264,11 +223,7 @@ test.describe('AcroForm Import', () => {
 
     await UNSAFE_importAcroFormFieldsFromEnvelope({
       envelope,
-      apiRequestMetadata: {
-        requestMetadata: {},
-        source: 'apiV1',
-        auth: 'api',
-      },
+      apiRequestMetadata: API_REQUEST_METADATA,
     });
 
     const after = await prisma.envelope.findUniqueOrThrow({

packages/lib/server-only/envelope-item/import-acroform-fields.ts

@@ -110,11 +110,7 @@ export const UNSAFE_importAcroFormFieldsFromEnvelope = async ({
         extraction,
       };
 
-      if (extraction.fields.length === 0) {
-        return base;
-      }
-
-      if (extraction.hasSignedSignature) {
+      if (extraction.fields.length === 0 || extraction.hasSignedSignature) {
         return base;
       }
 
@@ -174,7 +170,7 @@ export const UNSAFE_importAcroFormFieldsFromEnvelope = async ({
         return null;
       }
 
-      return [...signable].sort((a, b) => {
+      return signable.sort((a, b) => {
         const aOrder = a.signingOrder ?? Number.MAX_SAFE_INTEGER;
         const bOrder = b.signingOrder ?? Number.MAX_SAFE_INTEGER;
 

packages/lib/server-only/pdf/acroform-fields.ts

@@ -68,6 +68,7 @@ const NAME_NAME_PATTERN = /name/i;
 const INITIALS_NAME_PATTERN = /initial/i;
 
 const ROW_TOLERANCE_PERCENT = 2;
+const ACROFORM_FIELD_SOURCE = 'acroform';
 
 export type AcroFormUnsupportedReason =
   | 'unsupported-type'
@@ -153,25 +154,15 @@ const hasXfa = (pdfDoc: PDF, resolver: RefResolver): boolean => {
   }
 };
 
-const isDateFieldByName = (name: string | null | undefined): boolean => {
-  return name ? DATE_NAME_PATTERN.test(name) : false;
-};
+const isDateFieldByName = (name: string | null | undefined): boolean => !!name && DATE_NAME_PATTERN.test(name);
 
-const isNumberFieldByName = (name: string | null | undefined): boolean => {
-  return name ? NUMBER_NAME_PATTERN.test(name) : false;
-};
+const isNumberFieldByName = (name: string | null | undefined): boolean => !!name && NUMBER_NAME_PATTERN.test(name);
 
-const isEmailFieldByName = (name: string | null | undefined): boolean => {
-  return name ? EMAIL_NAME_PATTERN.test(name) : false;
-};
+const isEmailFieldByName = (name: string | null | undefined): boolean => !!name && EMAIL_NAME_PATTERN.test(name);
 
-const isNameFieldByName = (name: string | null | undefined): boolean => {
-  return name ? NAME_NAME_PATTERN.test(name) : false;
-};
+const isNameFieldByName = (name: string | null | undefined): boolean => !!name && NAME_NAME_PATTERN.test(name);
 
-const isInitialsFieldByName = (name: string | null | undefined): boolean => {
-  return name ? INITIALS_NAME_PATTERN.test(name) : false;
-};
+const isInitialsFieldByName = (name: string | null | undefined): boolean => !!name && INITIALS_NAME_PATTERN.test(name);
 
 /**
  * Detect AcroForm format actions on a text field dictionary.
@@ -185,13 +176,7 @@ const isInitialsFieldByName = (name: string | null | undefined): boolean => {
  */
 const getTextFieldFormatHint = (fieldDict: PdfDict, resolver: RefResolver): 'date' | 'number' | null => {
   try {
-    const aaDict = fieldDict.getDict('AA', resolver);
-
-    if (!aaDict) {
-      return null;
-    }
-
-    const formatDict = aaDict.getDict('F', resolver);
+    const formatDict = fieldDict.getDict('AA', resolver)?.getDict('F', resolver);
 
     if (!formatDict) {
       return null;
@@ -416,7 +401,7 @@ const buildSignatureFieldAndMeta = (field: FormFieldWithDict): TFieldAndMeta =>
       ...FIELD_SIGNATURE_META_DEFAULT_VALUES,
       required: field.isRequired() || undefined,
       readOnly: field.isReadOnly() || undefined,
-      source: 'acroform' as const,
+      source: ACROFORM_FIELD_SOURCE,
     },
   });
 };
@@ -430,14 +415,15 @@ const buildTextFieldAndMeta = (
   const required = field.isRequired() || undefined;
   const readOnly = field.isReadOnly() || undefined;
 
+  const defaultValue = defaultText && defaultText.length > 0 ? defaultText : undefined;
   if (documensoType === FieldType.NUMBER) {
     const fieldMeta: TNumberFieldMeta = {
       ...FIELD_NUMBER_META_DEFAULT_VALUES,
       label: label ?? FIELD_NUMBER_META_DEFAULT_VALUES.label,
       required,
       readOnly,
-      source: 'acroform' as const,
-      value: defaultText && defaultText.length > 0 ? defaultText : undefined,
+      source: ACROFORM_FIELD_SOURCE,
+      value: defaultValue,
     };
 
     return ZEnvelopeFieldAndMetaSchema.parse({ type: documensoType, fieldMeta });
@@ -449,8 +435,8 @@ const buildTextFieldAndMeta = (
       label: label ?? FIELD_TEXT_META_DEFAULT_VALUES.label,
       required,
       readOnly,
-      source: 'acroform' as const,
-      text: defaultText && defaultText.length > 0 ? defaultText : FIELD_TEXT_META_DEFAULT_VALUES.text,
+      source: ACROFORM_FIELD_SOURCE,
+      text: defaultValue ?? FIELD_TEXT_META_DEFAULT_VALUES.text,
     };
 
     return ZEnvelopeFieldAndMetaSchema.parse({ type: documensoType, fieldMeta });
@@ -464,7 +450,7 @@ const buildTextFieldAndMeta = (
         label,
         required,
         readOnly,
-        source: 'acroform' as const,
+        source: ACROFORM_FIELD_SOURCE,
       },
     });
   }
@@ -477,7 +463,7 @@ const buildTextFieldAndMeta = (
         label,
         required,
         readOnly,
-        source: 'acroform' as const,
+        source: ACROFORM_FIELD_SOURCE,
       },
     });
   }
@@ -490,7 +476,7 @@ const buildTextFieldAndMeta = (
         label,
         required,
         readOnly,
-        source: 'acroform' as const,
+        source: ACROFORM_FIELD_SOURCE,
       },
     });
   }
@@ -502,7 +488,7 @@ const buildTextFieldAndMeta = (
       label,
       required,
       readOnly,
-      source: 'acroform' as const,
+      source: ACROFORM_FIELD_SOURCE,
     },
   });
 };
@@ -520,7 +506,7 @@ const buildCheckboxFieldAndMeta = (
     label: pickLabel(field) ?? FIELD_CHECKBOX_META_DEFAULT_VALUES.label,
     required: required || undefined,
     readOnly: field.isReadOnly() || undefined,
-    source: 'acroform' as const,
+    source: ACROFORM_FIELD_SOURCE,
     values: [{ id: 1, checked: isChecked, value }],
     validationRule: required ? 'at-least' : '',
     validationLength: required ? 1 : 0,
@@ -555,7 +541,7 @@ const buildRadioFieldAndMeta = (
     label: pickLabel(field) ?? '',
     required: field.isRequired() || undefined,
     readOnly: field.isReadOnly() || undefined,
-    source: 'acroform' as const,
+    source: ACROFORM_FIELD_SOURCE,
     values,
   };
 
@@ -572,7 +558,7 @@ const buildDropdownFieldAndMeta = (
     label: pickLabel(field) ?? '',
     required: field.isRequired() || undefined,
     readOnly: field.isReadOnly() || undefined,
-    source: 'acroform' as const,
+    source: ACROFORM_FIELD_SOURCE,
     values: options.length > 0 ? options.map((value) => ({ value })) : FIELD_DROPDOWN_META_DEFAULT_VALUES.values,
     defaultValue: defaultValue ?? '',
   };
@@ -661,6 +647,10 @@ export const extractAcroFormFieldsFromPDF = async (
     const fields: AcroFormFieldImportInfo[] = [];
     const unsupported: AcroFormUnsupportedFieldInfo[] = [];
     let hasSignedSignature = false;
+    const usePdfDefaults = !options.formValuesProvided;
+    const addUnsupported = (fieldName: string, acroFormType: string, reason: AcroFormUnsupportedReason): void => {
+      unsupported.push({ fieldName, acroFormType, reason });
+    };
 
     for (const field of formFields) {
       const acroFormType = field.type;
@@ -671,11 +661,7 @@ export const extractAcroFormFieldsFromPDF = async (
         acroFormType === 'unknown' ||
         acroFormType === 'non-terminal'
       ) {
-        unsupported.push({
-          fieldName: field.name,
-          acroFormType,
-          reason: 'unsupported-type',
-        });
+        addUnsupported(field.name, acroFormType, 'unsupported-type');
         continue;
       }
 
@@ -687,11 +673,7 @@ export const extractAcroFormFieldsFromPDF = async (
 
         if (typeof sigField.isSigned === 'function' && sigField.isSigned()) {
           hasSignedSignature = true;
-          unsupported.push({
-            fieldName: field.name,
-            acroFormType,
-            reason: 'signed-signature',
-          });
+          addUnsupported(field.name, acroFormType, 'signed-signature');
           continue;
         }
       }
@@ -701,38 +683,25 @@ export const extractAcroFormFieldsFromPDF = async (
       const { matched, unmatched } = resolveWidgetPages(widgets, pageByRef);
 
       for (let i = 0; i < unmatched.length; i += 1) {
-        unsupported.push({
-          fieldName: field.name,
-          acroFormType,
-          reason: 'no-page-match',
-        });
+        addUnsupported(field.name, acroFormType, 'no-page-match');
       }
 
       let widgetCounter = 0;
 
       for (const { widget, pageIndex, page } of matched) {
         if (widget.isHidden()) {
-          unsupported.push({
-            fieldName: field.name,
-            acroFormType,
-            reason: 'hidden',
-          });
+          addUnsupported(field.name, acroFormType, 'hidden');
           continue;
         }
 
         const { geometry, reason } = resolveGeometry(widget, pageIndex, page);
 
         if (!geometry) {
-          unsupported.push({
-            fieldName: field.name,
-            acroFormType,
-            reason: reason ?? 'zero-size',
-          });
+          addUnsupported(field.name, acroFormType, reason ?? 'zero-size');
           continue;
         }
 
         let fieldAndMeta: TFieldAndMeta;
-        const usePdfDefaults = !options.formValuesProvided;
 
         if (acroFormType === 'signature') {
           fieldAndMeta = buildSignatureFieldAndMeta(formField);
@@ -779,16 +748,12 @@ export const extractAcroFormFieldsFromPDF = async (
           const currentSelection = usePdfDefaults ? dropdown.getValue?.() || dropdown.getDefaultValue?.() || '' : '';
           fieldAndMeta = buildDropdownFieldAndMeta(formField, optionValues, currentSelection || undefined);
         } else {
-          unsupported.push({
-            fieldName: field.name,
-            acroFormType,
-            reason: 'unsupported-type',
-          });
+          addUnsupported(field.name, acroFormType, 'unsupported-type');
           continue;
         }
 
         fields.push({
-          source: 'acroform',
+          source: ACROFORM_FIELD_SOURCE,
           fieldName: field.name,
           widgetIndex: widgetCounter,
           fieldAndMeta,
@@ -845,9 +810,7 @@ export const convertAcroFormFieldsToFieldInputs = (
   recipientResolver: (fieldName: string) => Pick<Recipient, 'id'>,
   envelopeItemId?: string,
 ): FieldToCreate[] => {
-  const sorted = sortFieldsForCreate(fields);
-
-  return sorted.map((f) => {
+  return sortFieldsForCreate(fields).map((f) => {
     const xPercent = (f.x / f.pageWidth) * 100;
     const yPercent = (f.y / f.pageHeight) * 100;
     const widthPercent = (f.width / f.pageWidth) * 100;