feat: stable submission id

src/app/constants/mongo-error.ts

@@ -0,0 +1,4 @@
+export const MONGO_ERROR_CODE = {
+  DuplicateKey: 11000,
+  InvalidBSON: 10334,
+}

src/app/modules/core/core.errors.ts

@@ -45,6 +45,12 @@ export class DatabasePayloadSizeError extends ApplicationError {
   }
 }
 
+export class DatabaseDuplicateKeyError extends ApplicationError {
+  constructor(message: string) {
+    super(message)
+  }
+}
+
 export class SecretsManagerError extends ApplicationError {
   constructor(message?: string) {
     super(message)
@@ -77,6 +83,7 @@ export type PossibleDatabaseError =
   | DatabaseValidationError
   | DatabaseConflictError
   | DatabasePayloadSizeError
+  | DatabaseDuplicateKeyError
 
 export class MalformedParametersError extends ApplicationError {
   constructor(message: string, meta?: unknown) {

src/app/modules/payments/stripe.utils.ts

@@ -7,10 +7,9 @@ import mongoose from 'mongoose'
 import { err, Ok, ok, Result } from 'neverthrow'
 import Stripe from 'stripe'
 
-import { StripePaymentMetadataDto } from 'src/types'
-
 import { Payment, PaymentStatus } from '../../../../shared/types'
 import { hasProp } from '../../../../shared/utils/has-prop'
+import { StripePaymentMetadataDto } from '../../../types'
 import { createLoggerWithLabel } from '../../config/logger'
 import { ApplicationError } from '../core/core.errors'
 import {
@@ -46,6 +45,7 @@ const isStripeMetadata = (
 ): obj is StripePaymentMetadataDto =>
   hasProp(obj, 'formTitle') &&
   hasProp(obj, 'formId') &&
+  hasProp(obj, 'submissionId') &&
   hasProp(obj, 'paymentId') &&
   hasProp(obj, 'paymentContactEmail')
 

src/app/modules/submission/__tests__/submission/submission.service.spec.ts

@@ -794,64 +794,91 @@ describe('submission.service', () => {
   describe('copyPendingSubmissionToSubmissions', () => {
     const MOCK_PENDING_SUBMISSION_ID = MOCK_SUBMISSION._id
 
+    const MOCK_PENDING_SUBMISSION = {
+      _id: MOCK_PENDING_SUBMISSION_ID,
+      submissionType: SubmissionType.Encrypt,
+      form: MOCK_FORM_ID,
+      encryptedContent: 'some random encrypted content',
+      version: 1,
+      responseHash: 'hash',
+      responseSalt: 'salt',
+    }
+
     beforeEach(async () => {
+      await dbHandler.clearCollection(PendingSubmission.collection.name)
       await dbHandler.clearCollection(Submission.collection.name)
     })
     afterEach(() => jest.clearAllMocks())
 
-    it('should return a submission document if copying from pending submissions to submissions was successful', async () => {
-      const pendingSubmission = await PendingSubmission.create({
-        _id: MOCK_PENDING_SUBMISSION_ID,
-        submissionType: SubmissionType.Encrypt,
-        form: MOCK_FORM_ID,
-        encryptedContent: 'some random encrypted content',
-        version: 1,
-        responseHash: 'hash',
-        responseSalt: 'salt',
-      })
+    it('should return a submission document with the same ObjectID if copying from pending submissions to submissions was successful', async () => {
+      // Arrange
+      const pendingSubmission = await PendingSubmission.create(
+        MOCK_PENDING_SUBMISSION,
+      )
 
+      // Act
       const session = await mongoose.startSession()
       const result = await SubmissionService.copyPendingSubmissionToSubmissions(
         MOCK_PENDING_SUBMISSION_ID,
         session,
       )
       session.endSession()
 
+      //Assert
       expect(result.isOk()).toEqual(true)
 
       const submission = result._unsafeUnwrap()
-
       Object.keys(pendingSubmission).forEach((key) => {
-        if (['_id', 'created', 'lastModified'].includes(key)) return
+        if (['created', 'lastModified'].includes(key)) return
         expect(submission.get(key)).toEqual(pendingSubmission.get(key))
       })
     })
 
-    it('should return false if pendingSubmissionId does not exist', async () => {
-      await Submission.create({
-        _id: MOCK_PENDING_SUBMISSION_ID,
-        submissionType: SubmissionType.Encrypt,
-        form: MOCK_FORM_ID,
-        encryptedContent: 'some random encrypted content',
-        version: 1,
-        responseHash: 'hash',
-        responseSalt: 'salt',
+    it('should return a submission document with a different ObjectID if an ObjectID collision occurs while copying the pending submission to the submissions collection', async () => {
+      // Arrange
+      const pendingSubmission = await PendingSubmission.create(
+        MOCK_PENDING_SUBMISSION,
+      )
+      await Submission.create(MOCK_PENDING_SUBMISSION)
+
+      // Act
+      const session = await mongoose.startSession()
+      const result = await SubmissionService.copyPendingSubmissionToSubmissions(
+        MOCK_PENDING_SUBMISSION_ID,
+        session,
+      )
+      session.endSession()
+
+      // Assert
+      expect(result.isOk()).toEqual(true)
+
+      const submission = result._unsafeUnwrap()
+      // Explicitly check the '_id' field to be different
+      expect(submission.get('_id')).not.toEqual(pendingSubmission._id)
+      Object.keys(pendingSubmission).forEach((key) => {
+        if (['_id', 'created', 'lastModified'].includes(key)) return
+        expect(submission.get(key)).toEqual(pendingSubmission.get(key))
       })
+    })
 
+    it('should return PendingSubmissionNotFoundError if pendingSubmissionId does not exist', async () => {
+      // Act
       const session = await mongoose.startSession()
       const result = await SubmissionService.copyPendingSubmissionToSubmissions(
         new ObjectId().toHexString(),
         session,
       )
       session.endSession()
 
+      // Assert
       expect(result.isErr()).toEqual(true)
       expect(result._unsafeUnwrapErr()).toBeInstanceOf(
         PendingSubmissionNotFoundError,
       )
     })
 
     it('should return DatabaseError when error occurs whilst querying database', async () => {
+      // Arrange
       const findSpy = jest.spyOn(PendingSubmission, 'findById')
       findSpy.mockImplementationOnce(
         () =>
@@ -860,13 +887,15 @@ describe('submission.service', () => {
           } as unknown as mongoose.Query<any, any>),
       )
 
+      // Act
       const session = await mongoose.startSession()
       const result = await SubmissionService.copyPendingSubmissionToSubmissions(
         MOCK_PENDING_SUBMISSION_ID,
         session,
       )
       session.endSession()
 
+      // Assert
       expect(findSpy).toHaveBeenCalledWith(
         MOCK_PENDING_SUBMISSION_ID,
         null,

src/app/modules/submission/encrypt-submission/encrypt-submission.controller.ts

@@ -8,8 +8,6 @@ import { okAsync } from 'neverthrow'
 import Stripe from 'stripe'
 import type { SetOptional } from 'type-fest'
 
-import { StripePaymentMetadataDto } from 'src/types'
-
 import {
   ErrorDto,
   FormAuthType,
@@ -21,6 +19,7 @@ import {
   SubmissionErrorDto,
   SubmissionResponseDto,
 } from '../../../../../shared/types'
+import { StripePaymentMetadataDto } from '../../../../types'
 import { EncryptSubmissionDto } from '../../../../types/api'
 import { paymentConfig } from '../../../config/features/payment.config'
 import { createLoggerWithLabel } from '../../../config/logger'
@@ -432,18 +431,25 @@ const submitEncryptModeForm: ControllerHandler<
     const metadata: StripePaymentMetadataDto = {
       formTitle: form.title,
       formId,
+      submissionId: pendingSubmissionId,
       paymentId,
       paymentContactEmail: paymentReceiptEmail,
     }
 
+    const paymentReceiptDescription = [
+      form.payments_field.description,
+      `FormSG form: ${form.title}`,
+      `Response ID: ${pendingSubmissionId}`,
+    ].join('\n')
+
     const createPaymentIntentParams: Stripe.PaymentIntentCreateParams = {
       amount,
       currency: paymentConfig.defaultCurrency,
       payment_method_types: [
         'card',
         /* 'grabpay', 'paynow'*/
       ],
-      description: form.payments_field.description,
+      description: paymentReceiptDescription,
       receipt_email: paymentReceiptEmail,
       metadata,
     }

src/app/modules/submission/submission.service.ts

@@ -14,8 +14,15 @@ import getSubmissionModel from '../../models/submission.server.model'
 import MailService from '../../services/mail/mail.service'
 import { AutoReplyMailData } from '../../services/mail/mail.types'
 import { createQueryWithDateParam, isMalformedDate } from '../../utils/date'
-import { getMongoErrorMessage } from '../../utils/handle-mongo-error'
-import { DatabaseError, MalformedParametersError } from '../core/core.errors'
+import {
+  getMongoErrorMessage,
+  transformMongoError,
+} from '../../utils/handle-mongo-error'
+import {
+  DatabaseDuplicateKeyError,
+  DatabaseError,
+  MalformedParametersError,
+} from '../core/core.errors'
 import { InvalidSubmissionIdError } from '../feedback/feedback.errors'
 
 import {
@@ -251,17 +258,63 @@ export const copyPendingSubmissionToSubmissions = (
       return okAsync(submission)
     })
     .andThen((pendingSubmission) => {
-      const submission = new SubmissionModel(
-        omit(pendingSubmission, ['_id', 'created', 'lastModified']),
-      )
+      const submissionContent = omit(pendingSubmission, [
+        '_id',
+        'created',
+        'lastModified',
+      ])
+      const submission = new SubmissionModel({
+        // Explicitly copy over the pending submission's _id
+        ...submissionContent,
+        _id: pendingSubmissionId,
+      })
 
       return ResultAsync.fromPromise(submission.save({ session }), (error) => {
         logger.error({
           message: 'Database save submission error',
           meta: logMeta,
           error,
         })
-        return new DatabaseError(getMongoErrorMessage(error))
+        return transformMongoError(error)
+      }).orElse((error) => {
+        const isDuplicateKeyError = error instanceof DatabaseDuplicateKeyError
+
+        if (!isDuplicateKeyError) {
+          return errAsync(new DatabaseError(getMongoErrorMessage(error)))
+        }
+
+        // Failed to save due to duplicate keys.
+        logger.error({
+          message:
+            'Failed to move pending submission to submission: duplicate key error in submission collection',
+          meta: logMeta,
+          error,
+        })
+
+        // Recover by attempting to save with a different id.
+        const recoverySubmission = new SubmissionModel(submissionContent)
+        // TODO: Set alarms for both branches
+        return ResultAsync.fromPromise(
+          recoverySubmission.save({ session }),
+          (error) => {
+            logger.error({
+              message: 'Failed to recover from duplicate key error',
+              meta: logMeta,
+              error,
+            })
+            return new DatabaseError(getMongoErrorMessage(error))
+          },
+        ).andThen((recoverySubmission) => {
+          logger.warn({
+            message: `Successfully recovered from duplicate key error`,
+            meta: {
+              submissionId: recoverySubmission._id,
+              ...logMeta,
+            },
+            error,
+          })
+          return okAsync(recoverySubmission)
+        })
       })
     })
 }

src/app/utils/handle-mongo-error.ts

@@ -1,7 +1,9 @@
 import { Error as MongooseError, mongo as mongodb } from 'mongoose'
 
+import { MONGO_ERROR_CODE } from '../constants/mongo-error'
 import {
   DatabaseConflictError,
+  DatabaseDuplicateKeyError,
   DatabaseError,
   DatabasePayloadSizeError,
   DatabaseValidationError,
@@ -31,10 +33,12 @@ export const getMongoErrorMessage = (
   // Handle base Mongo engine errors
   if (err instanceof MongoError) {
     switch (err.code) {
-      case 10334: // BSONObj size invalid error
+      case MONGO_ERROR_CODE.InvalidBSON: // BSONObj size invalid error
         return formatErrorRecoveryMessage(
           'Your form is too large to be supported by the system.',
         )
+      case MONGO_ERROR_CODE.DuplicateKey:
+        return formatErrorRecoveryMessage('Duplicate key error')
       default:
         return formatErrorRecoveryMessage(defaultErrorMessage)
     }
@@ -86,13 +90,21 @@ export const transformMongoError = (error: unknown): PossibleDatabaseError => {
     // Exception when Mongoose breaches Mongo 16MB size limit.
     error instanceof RangeError ||
     // MongoDB Invalid BSON error.
-    (error instanceof MongoError && error.code === 10334) ||
+    (error instanceof MongoError &&
+      error.code === MONGO_ERROR_CODE.InvalidBSON) ||
     // FormSG-imposed limit in pre-validate hook.
     error.name === 'FormSizeError'
   ) {
     return new DatabasePayloadSizeError(errorMessage)
   }
 
+  if (
+    error instanceof MongoError &&
+    error.code === MONGO_ERROR_CODE.DuplicateKey
+  ) {
+    return new DatabaseDuplicateKeyError(errorMessage)
+  }
+
   return new DatabaseError(errorMessage)
 }
 

src/types/payment.ts

@@ -10,6 +10,7 @@ export type IPaymentModel = Model<IPaymentSchema>
 export interface StripePaymentMetadataDto extends Stripe.Metadata {
   formTitle: string
   formId: string
+  submissionId: string
   paymentId: string
   paymentContactEmail: string
 }