CCM-10422: merge

alexnuttall · alexnuttall · commit 342b54e5e6f5 · 2025-08-14T11:33:30.000+01:00
diff --git a/lambdas/backend-api/src/__tests__/templates/api/process-proof.test.ts b/lambdas/backend-api/src/__tests__/templates/api/process-proof.test.ts
@@ -77,46 +77,3 @@ test.each([
     );
   }
 );
-
-test('sets virus scan status for a user-owned template', async () => {
-  const templateRepository = mockDeep<TemplateRepository>({
-    getOwner: jest.fn().mockReturnValue('user-template-owner'),
-  });
-  const letterFileRepository = mockDeep<LetterFileRepository>();
-
-  const handler = createHandler({
-    templateRepository,
-    letterFileRepository,
-    logger: mockDeep<Logger>(),
-  });
-
-  await handler({
-    detail: {
-      s3ObjectDetails: {
-        objectKey: 'proofs/supplier/template-id/proof.pdf',
-        versionId: 'version-id',
-      },
-      scanResultDetails: { scanResultStatus: 'NO_THREATS_FOUND' },
-    },
-  });
-
-  expect(templateRepository.getOwner).toHaveBeenCalledWith('template-id');
-
-  expect(
-    letterFileRepository.copyFromQuarantineToInternal
-  ).toHaveBeenCalledWith(
-    'proofs/supplier/template-id/proof.pdf',
-    'version-id',
-    'proofs/user-template-owner/template-id/proof.pdf'
-  );
-
-  expect(
-    templateRepository.setLetterFileVirusScanStatusForProof
-  ).toHaveBeenCalledWith(
-    'user-template-owner',
-    'template-id',
-    'proof.pdf',
-    'PASSED',
-    'supplier'
-  );
-});
diff --git a/lambdas/backend-api/src/__tests__/templates/domain/template-pdf.test.ts b/lambdas/backend-api/src/__tests__/templates/domain/template-pdf.test.ts
@@ -11,7 +11,7 @@ test('has the given key attributes', () => {
   );
 
   expect(pdf.templateId).toBe('template-id');
-  expect(pdf.userOrClientId).toBe('template-owner');
+  expect(pdf.owner).toBe('template-owner');
 });
 
 test('parse with no custom personalisation', async () => {
diff --git a/lambdas/backend-api/src/__tests__/templates/infra/letter-upload-repository.test.ts b/lambdas/backend-api/src/__tests__/templates/infra/letter-upload-repository.test.ts
@@ -242,7 +242,7 @@ describe('LetterUploadRepository', () => {
         )
       ).toEqual({
         'file-type': 'pdf-template',
-        'user-or-client-id': 'owner-id',
+        owner: 'owner-id',
         'template-id': 'template-id',
         'version-id': 'version-id',
       });
@@ -255,7 +255,7 @@ describe('LetterUploadRepository', () => {
         )
       ).toEqual({
         'file-type': 'test-data',
-        'user-or-client-id': 'owner-id',
+        owner: 'owner-id',
         'template-id': 'template-id',
         'version-id': 'version-id',
       });
diff --git a/lambdas/backend-api/src/__tests__/templates/infra/template-repository.test.ts b/lambdas/backend-api/src/__tests__/templates/infra/template-repository.test.ts
@@ -2089,69 +2089,12 @@ describe('templateRepository', () => {
     it('updates status to WAITING_FOR_PROOF', async () => {
       const { templateRepository, mocks } = setup();
 
-      mocks.ddbDocClient
-        .on(QueryCommand)
-        .resolvesOnce({
-          Items: [{ id: 'template-id', owner: clientOwner }],
-        })
-        .on(UpdateCommand)
-        .resolvesOnce({
-          Attributes: {
-            // complete template
-            id: 'template-id',
-          },
-        });
-
-      const result = await templateRepository.proofRequestUpdate(
-        'template-id',
-        user
-      );
-
-      expect(result).toEqual({ data: { id: 'template-id' } });
-
-      expect(mocks.ddbDocClient).toHaveReceivedCommandWith(UpdateCommand, {
-        ConditionExpression:
-          '#templateStatus = :condition_1_templateStatus AND #templateType = :condition_2_templateType AND #clientId = :condition_3_clientId AND attribute_exists (#id) AND #proofingEnabled = :condition_5_proofingEnabled',
-        ExpressionAttributeNames: {
-          '#id': 'id',
-          '#clientId': 'clientId',
-          '#templateStatus': 'templateStatus',
-          '#templateType': 'templateType',
-          '#updatedAt': 'updatedAt',
-          '#proofingEnabled': 'proofingEnabled',
-        },
-        ExpressionAttributeValues: {
-          ':condition_1_templateStatus': 'PENDING_PROOF_REQUEST',
-          ':condition_2_templateType': 'LETTER',
-          ':condition_3_clientId': clientId,
-          ':condition_5_proofingEnabled': true,
-          ':templateStatus': 'WAITING_FOR_PROOF',
-          ':updatedAt': '2024-12-27T00:00:00.000Z',
+      mocks.ddbDocClient.on(UpdateCommand).resolvesOnce({
+        Attributes: {
+          // complete template
+          id: 'template-id',
         },
-        Key: { id: 'template-id', owner: ownerWithClientPrefix },
-        ReturnValues: 'ALL_NEW',
-        ReturnValuesOnConditionCheckFailure: 'ALL_OLD',
-        TableName: 'templates',
-        UpdateExpression:
-          'SET #templateStatus = :templateStatus, #updatedAt = :updatedAt',
       });
-    });
-
-    it('updates status to WAITING_FOR_PROOF when template is user-owned', async () => {
-      const { templateRepository, mocks } = setup();
-
-      mocks.ddbDocClient
-        .on(QueryCommand)
-        .resolvesOnce({
-          Items: [{ id: 'template-id', owner: userId }],
-        })
-        .on(UpdateCommand)
-        .resolvesOnce({
-          Attributes: {
-            // complete template
-            id: 'template-id',
-          },
-        });
 
       const result = await templateRepository.proofRequestUpdate(
         'template-id',
@@ -2179,7 +2122,7 @@ describe('templateRepository', () => {
           ':templateStatus': 'WAITING_FOR_PROOF',
           ':updatedAt': '2024-12-27T00:00:00.000Z',
         },
-        Key: { id: 'template-id', owner: userId },
+        Key: { id: 'template-id', owner: ownerWithClientPrefix },
         ReturnValues: 'ALL_NEW',
         ReturnValuesOnConditionCheckFailure: 'ALL_OLD',
         TableName: 'templates',
@@ -2188,28 +2131,6 @@ describe('templateRepository', () => {
       });
     });
 
-    test('returns 404 response when GSI query cannot find owned template', async () => {
-      const { templateRepository, mocks } = setup();
-
-      mocks.ddbDocClient.on(QueryCommand).resolves({
-        Items: [{ id: templateId, owner: 'someone-else' }],
-      });
-
-      const response = await templateRepository.proofRequestUpdate(
-        'template-id',
-        user
-      );
-
-      expect(response).toEqual({
-        error: {
-          errorMeta: {
-            code: 404,
-            description: 'Template not found',
-          },
-        },
-      });
-    });
-
     it('returns 404 error response when conditional check fails due to template not existing', async () => {
       const { templateRepository, mocks } = setup();
 
@@ -2218,13 +2139,7 @@ describe('templateRepository', () => {
         $metadata: {},
       });
 
-      mocks.ddbDocClient
-        .on(QueryCommand)
-        .resolvesOnce({
-          Items: [{ id: 'template-id', owner: clientOwner }],
-        })
-        .on(UpdateCommand)
-        .rejectsOnce(err);
+      mocks.ddbDocClient.on(UpdateCommand).rejectsOnce(err);
 
       const result = await templateRepository.proofRequestUpdate(
         'template-id',
@@ -2253,13 +2168,7 @@ describe('templateRepository', () => {
         },
       });
 
-      mocks.ddbDocClient
-        .on(QueryCommand)
-        .resolvesOnce({
-          Items: [{ id: 'template-id', owner: clientOwner }],
-        })
-        .on(UpdateCommand)
-        .rejectsOnce(err);
+      mocks.ddbDocClient.on(UpdateCommand).rejectsOnce(err);
 
       const result = await templateRepository.proofRequestUpdate(
         'template-id',
@@ -2282,13 +2191,7 @@ describe('templateRepository', () => {
 
       const err = new Error('!');
 
-      mocks.ddbDocClient
-        .on(QueryCommand)
-        .resolvesOnce({
-          Items: [{ id: 'template-id', owner: clientOwner }],
-        })
-        .on(UpdateCommand)
-        .rejectsOnce(err);
+      mocks.ddbDocClient.on(UpdateCommand).rejectsOnce(err);
 
       const result = await templateRepository.proofRequestUpdate(
         'template-id',
diff --git a/lambdas/backend-api/src/templates/api/process-proof.ts b/lambdas/backend-api/src/templates/api/process-proof.ts
@@ -29,12 +29,8 @@ export const createHandler =
 
     const { owner } = await templateRepository.getOwner(templateId);
 
-    const ownerS3Segment = owner.startsWith(clientOwnerPrefix)
-      ? owner.slice(clientOwnerPrefix.length)
-      : owner;
-
     const internalKey = LetterProofRepository.getInternalKey(
-      ownerS3Segment,
+      owner,
       templateId,
       fileName
     );
@@ -43,7 +39,7 @@ export const createHandler =
 
     if (virusScanResult === 'PASSED') {
       const downloadKey = LetterProofRepository.getDownloadKey(
-        ownerS3Segment,
+        owner,
         templateId,
         fileName
       );
diff --git a/lambdas/backend-api/src/templates/api/set-letter-upload-virus-scan-status.ts b/lambdas/backend-api/src/templates/api/set-letter-upload-virus-scan-status.ts
@@ -23,7 +23,7 @@ export const createHandler =
       'file-type': fileType,
       'version-id': versionId,
       'template-id': templateId,
-      'user-or-client-id': userOrClientId,
+      owner,
     } = LetterUploadRepository.parseKey(objectKey);
 
     const virusScanResult =
diff --git a/lambdas/backend-api/src/templates/api/validate-letter-template-files.ts b/lambdas/backend-api/src/templates/api/validate-letter-template-files.ts
@@ -57,7 +57,7 @@ export class ValidateLetterTemplateFilesLambda {
 
     const {
       'template-id': templateId,
-      'user-or-client-id': userOrClientId,
+      owner,
       'version-id': versionId,
     } = metadata;
 
@@ -69,8 +69,8 @@ export class ValidateLetterTemplateFilesLambda {
           is complete, we will know unambiguously that it's a clientId and this
           doubly attempted fetch won't be required
         */
-        userId: userOrClientId,
-        clientId: userOrClientId,
+        userId: owner,
+        clientId: owner,
       });
 
     if (getTemplateError) {
diff --git a/lambdas/backend-api/src/templates/domain/validate-letter-template-files.ts b/lambdas/backend-api/src/templates/domain/validate-letter-template-files.ts
@@ -48,7 +48,7 @@ export function validateLetterTemplateFiles(
 
   logger.info('Template file validation complete', {
     templateId: pdf.templateId,
-    userOrClientId: pdf.userOrClientId,
+    owner: pdf.owner,
     valid,
     failedTests,
   });
diff --git a/lambdas/backend-api/src/templates/infra/letter-upload-repository.ts b/lambdas/backend-api/src/templates/infra/letter-upload-repository.ts
@@ -11,7 +11,7 @@ import { LetterFileRepository } from './letter-file-repository';
 
 export type LetterUploadMetadata = {
   'file-type': FileType;
-  'user-or-client-id': string;
+  owner: string;
   'template-id': string;
   'version-id': string;
 };
@@ -24,7 +24,7 @@ const $FileType: z.ZodType<FileType> = z.enum([
 
 const $LetterUploadMetadata: z.ZodType<LetterUploadMetadata> = z.object({
   'file-type': $FileType,
-  'user-or-client-id': z.string(),
+  owner: z.string(),
   'template-id': z.string(),
   'version-id': z.string(),
 });
@@ -49,7 +49,7 @@ export class LetterUploadRepository extends LetterFileRepository {
         Body: await pdf.bytes(),
         ChecksumAlgorithm: 'SHA256',
         Metadata: LetterUploadRepository.metadata(
-          userOrClientId,
+          owner,
           templateId,
           versionId,
           'pdf-template'
@@ -58,12 +58,7 @@ export class LetterUploadRepository extends LetterFileRepository {
     ];
 
     if (csv) {
-      const csvKey = this.key(
-        'test-data',
-        userOrClientId,
-        templateId,
-        versionId
-      );
+      const csvKey = this.key('test-data', owner, templateId, versionId);
 
       commands.push(
         new PutObjectCommand({
@@ -72,7 +67,7 @@ export class LetterUploadRepository extends LetterFileRepository {
           Body: await csv.bytes(),
           ChecksumAlgorithm: 'SHA256',
           Metadata: LetterUploadRepository.metadata(
-            userOrClientId,
+            owner,
             templateId,
             versionId,
             'test-data'
@@ -119,7 +114,7 @@ export class LetterUploadRepository extends LetterFileRepository {
 
   static parseKey(key: string): LetterUploadMetadata {
     const keyParts = key.split('/');
-    const [type, userOrClientId, templateId, filename = ''] = keyParts;
+    const [type, owner, templateId, filename = ''] = keyParts;
     const filenameParts = filename.split('.');
     const [versionId, extension] = filenameParts;
 
@@ -128,7 +123,7 @@ export class LetterUploadRepository extends LetterFileRepository {
     }
 
     const parsed = LetterUploadRepository.metadata(
-      userOrClientId,
+      owner,
       templateId,
       versionId,
       type
@@ -146,21 +141,21 @@ export class LetterUploadRepository extends LetterFileRepository {
 
   private key(
     type: FileType,
-    userOrClientId: string,
+    owner: string,
     templateId: string,
     versionId: string
   ) {
-    return `${type}/${userOrClientId}/${templateId}/${versionId}.${type === 'pdf-template' ? 'pdf' : 'csv'}`;
+    return `${type}/${owner}/${templateId}/${versionId}.${type === 'pdf-template' ? 'pdf' : 'csv'}`;
   }
 
   private static metadata(
-    userOrClientId: string,
+    owner: string,
     templateId: string,
     versionId: string,
     type: string
   ): LetterUploadMetadata {
     return $LetterUploadMetadata.parse({
-      'user-or-client-id': userOrClientId,
+      owner,
       'file-type': type,
       'template-id': templateId,
       'version-id': versionId,
diff --git a/lambdas/sftp-letters/src/__tests__/app/request-proof.test.ts b/lambdas/sftp-letters/src/__tests__/app/request-proof.test.ts
@@ -234,7 +234,7 @@ describe('App', () => {
 
     expect(mocks.templateRepository.acquireLock).toHaveBeenCalledTimes(1);
     expect(mocks.templateRepository.acquireLock).toHaveBeenCalledWith(
-      `CLIENT#${clientId}`,
+      clientId,
       templateId
     );
 
diff --git a/tests/test-team/helpers/auth/cognito-auth-helper.ts b/tests/test-team/helpers/auth/cognito-auth-helper.ts
diff --git a/tests/test-team/helpers/db/template-storage-helper.ts b/tests/test-team/helpers/db/template-storage-helper.ts
diff --git a/tests/test-team/template-mgmt-api-tests/request-proof.api.spec.ts b/tests/test-team/template-mgmt-api-tests/request-proof.api.spec.ts
