extract shared proposal validation and apply to updateProposal

Extract validateProposalAgainstTemplate to DRY up the collab-doc-aware
validation logic shared by submitProposal and updateProposal. The update
endpoint now uses resolveProposalTemplate (checking both instanceData and
processSchema) instead of only reading processSchema directly.

Author: valentin0h

packages/common/src/services/decision/submitProposal.ts

@@ -1,17 +1,13 @@
-import { createTipTapClient } from '@op/collab';
 import { db, eq } from '@op/db/client';
 import { type ProcessInstance, ProposalStatus, proposals } from '@op/db/schema';
 import { assertAccess, permission } from 'access-zones';
 
 import { CommonError, NotFoundError, ValidationError } from '../../utils';
 import { getProfileAccessUser } from '../access';
-import { assembleProposalData } from './assembleProposalData';
-import { getProposalFragmentNames } from './getProposalFragmentNames';
-import { parseProposalData } from './proposalDataSchema';
 import { resolveProposalTemplate } from './resolveProposalTemplate';
-import { schemaValidator } from './schemaValidator';
 import type { DecisionInstanceData } from './schemas/instanceData';
 import { checkProposalsAllowed } from './utils/proposal';
+import { validateProposalAgainstTemplate } from './validateProposalAgainstTemplate';
 
 export interface SubmitProposalInput {
   proposalId: string;
@@ -86,39 +82,10 @@ export const submitProposal = async ({
   );
 
   if (proposalTemplate) {
-    const parsed = parseProposalData(existingProposal.proposalData);
-
-    if (parsed.collaborationDocId) {
-      // Fetch field values from the Yjs collaboration document
-      const appId = process.env.NEXT_PUBLIC_TIPTAP_APP_ID;
-      const secret = process.env.TIPTAP_SECRET;
-
-      if (!appId || !secret) {
-        throw new CommonError(
-          'TipTap credentials not configured, cannot validate proposal',
-        );
-      }
-
-      const client = createTipTapClient({ appId, secret });
-      const fragmentNames = getProposalFragmentNames(proposalTemplate);
-      const fragmentTexts = await client.getDocumentFragments(
-        parsed.collaborationDocId,
-        fragmentNames,
-        'text',
-      );
-      const validationData = assembleProposalData(
-        proposalTemplate,
-        fragmentTexts,
-      );
-
-      schemaValidator.validateProposalData(proposalTemplate, validationData);
-    } else {
-      // Legacy proposal without collaboration doc — validate DB data directly
-      schemaValidator.validateProposalData(
-        proposalTemplate,
-        existingProposal.proposalData,
-      );
-    }
+    await validateProposalAgainstTemplate(
+      proposalTemplate,
+      existingProposal.proposalData,
+    );
   }
 
   // Update proposal status to submitted

packages/common/src/services/decision/updateProposal.ts

@@ -1,15 +1,14 @@
 import { db, eq } from '@op/db/client';
 import {
-  DecisionProcess,
-  ProcessInstance,
+  type ProcessInstance,
   ProposalStatus,
-  Visibility,
+  type Visibility,
   proposalCategories,
   proposals,
   taxonomies,
   taxonomyTerms,
 } from '@op/db/schema';
-import { User } from '@op/supabase/lib';
+import type { User } from '@op/supabase/lib';
 import { permission } from 'access-zones';
 
 import {
@@ -21,11 +20,9 @@ import {
 import { assertInstanceProfileAccess } from '../access';
 import { assertUserByAuthId } from '../assert';
 import type { ProposalDataInput } from './proposalDataSchema';
-import { schemaValidator } from './schemaValidator';
-
-type ProcessInstanceWithProcess = ProcessInstance & {
-  process: DecisionProcess;
-};
+import { resolveProposalTemplate } from './resolveProposalTemplate';
+import type { DecisionInstanceData } from './schemas/instanceData';
+import { validateProposalAgainstTemplate } from './validateProposalAgainstTemplate';
 
 /**
  * Updates the category link for a proposal
@@ -101,20 +98,15 @@ export const updateProposal = async ({
     const existingProposal = await db._query.proposals.findFirst({
       where: eq(proposals.id, proposalId),
       with: {
-        processInstance: {
-          with: {
-            process: true,
-          },
-        },
+        processInstance: true,
       },
     });
 
     if (!existingProposal) {
       throw new NotFoundError('Proposal not found');
     }
 
-    const processInstance =
-      existingProposal.processInstance as ProcessInstanceWithProcess;
+    const processInstance = existingProposal.processInstance as ProcessInstance;
 
     await assertInstanceProfileAccess({
       user: { id: user.id },
@@ -134,14 +126,19 @@ export const updateProposal = async ({
     }
 
     // Validate proposal data against schema if updating proposalData
-    if (data.proposalData && processInstance.process) {
-      const process = processInstance.process as any;
-      const processSchema = process.processSchema;
-
-      if (processSchema?.proposalTemplate) {
-        schemaValidator.validateProposalData(
-          processSchema.proposalTemplate,
-          data.proposalData,
+    if (data.proposalData) {
+      const instanceData =
+        processInstance.instanceData as DecisionInstanceData | null;
+
+      const proposalTemplate = await resolveProposalTemplate(
+        instanceData,
+        processInstance.processId,
+      );
+
+      if (proposalTemplate) {
+        await validateProposalAgainstTemplate(
+          proposalTemplate,
+          existingProposal.proposalData,
         );
       }
     }

packages/common/src/services/decision/validateProposalAgainstTemplate.ts

@@ -0,0 +1,52 @@
+import { createTipTapClient } from '@op/collab';
+
+import { CommonError } from '../../utils';
+import { assembleProposalData } from './assembleProposalData';
+import { getProposalFragmentNames } from './getProposalFragmentNames';
+import { parseProposalData } from './proposalDataSchema';
+import { schemaValidator } from './schemaValidator';
+import type { ProposalTemplateSchema } from './types';
+
+/**
+ * Validates proposal data against a proposal template schema.
+ *
+ * For proposals with a TipTap collaboration document, fetches the latest
+ * field values from the Yjs doc and assembles them before validation.
+ * For legacy proposals without a collab doc, validates the raw proposalData directly.
+ *
+ * @throws {ValidationError} when the proposal data does not satisfy the template schema
+ * @throws {CommonError} when TipTap credentials are missing for a collab-doc proposal
+ */
+export async function validateProposalAgainstTemplate(
+  proposalTemplate: ProposalTemplateSchema,
+  proposalData: unknown,
+): Promise<void> {
+  const parsed = parseProposalData(proposalData);
+
+  if (parsed.collaborationDocId) {
+    const appId = process.env.NEXT_PUBLIC_TIPTAP_APP_ID;
+    const secret = process.env.TIPTAP_SECRET;
+
+    if (!appId || !secret) {
+      throw new CommonError(
+        'TipTap credentials not configured, cannot validate proposal',
+      );
+    }
+
+    const client = createTipTapClient({ appId, secret });
+    const fragmentNames = getProposalFragmentNames(proposalTemplate);
+    const fragmentTexts = await client.getDocumentFragments(
+      parsed.collaborationDocId,
+      fragmentNames,
+      'text',
+    );
+    const validationData = assembleProposalData(
+      proposalTemplate,
+      fragmentTexts,
+    );
+
+    schemaValidator.validateProposalData(proposalTemplate, validationData);
+  } else {
+    schemaValidator.validateProposalData(proposalTemplate, proposalData);
+  }
+}

services/api/src/routers/decision/proposals/update.test.ts

@@ -1,4 +1,6 @@
-import { ProposalStatus, Visibility } from '@op/db/schema';
+import { mockCollab } from '@op/collab/testing';
+import { ProposalStatus, Visibility, proposals } from '@op/db/schema';
+import { db, eq } from '@op/db/test';
 import { describe, expect, it } from 'vitest';
 
 import { appRouter } from '../..';
@@ -467,3 +469,72 @@ describe.concurrent('updateProposal status', () => {
     expect(result.visibility).toBe(Visibility.HIDDEN);
   });
 });
+
+describe.concurrent('updateProposal validation', () => {
+  it('should reject update when required fields are missing from collaboration document', async ({
+    task,
+    onTestFinished,
+  }) => {
+    const testData = new TestDecisionsDataManager(task.id, onTestFinished);
+
+    const setup = await testData.createDecisionSetup({
+      instanceCount: 1,
+      grantAccess: true,
+      proposalTemplate: {
+        type: 'object',
+        required: ['title', 'summary'],
+        'x-field-order': ['title', 'summary'],
+        properties: {
+          title: {
+            type: 'string',
+            title: 'Title',
+            minLength: 1,
+            'x-format': 'short-text',
+          },
+          summary: {
+            type: 'string',
+            title: 'Project Summary',
+            minLength: 1,
+            'x-format': 'long-text',
+          },
+        },
+      },
+    });
+
+    const instance = setup.instances[0];
+    if (!instance) {
+      throw new Error('No instance created');
+    }
+
+    const proposal = await testData.createProposal({
+      callerEmail: setup.userEmail,
+      processInstanceId: instance.instance.id,
+      proposalData: { title: 'Draft' },
+    });
+
+    const collaborationDocId = `proposal-${proposal.id}`;
+
+    await db
+      .update(proposals)
+      .set({
+        proposalData: { title: 'Draft', collaborationDocId },
+      })
+      .where(eq(proposals.id, proposal.id));
+
+    // Seed title but omit the required summary field
+    mockCollab.setDocFragments(collaborationDocId, {
+      title: 'Draft',
+    });
+
+    const caller = await createAuthenticatedCaller(setup.userEmail);
+
+    await expect(
+      caller.decision.updateProposal({
+        proposalId: proposal.id,
+        data: { proposalData: { title: 'Updated Draft' } },
+      }),
+    ).rejects.toMatchObject({
+      code: 'BAD_REQUEST',
+    });
+  });
+});