fix: add Unconnected payment channel type to fix mongoose 'feature' (#…

…6036)

* fix: add Unconnected payment channel type to fix mongoose 'feature'

* chore: remove unneeded conditional

* chore: change enum translation for unconnected:

* chore: update default payment channel in form model tests

frontend/src/features/admin-form/settings/components/PaymentSettingsSection/PaymentSettingsSection.tsx

@@ -1,6 +1,6 @@
 import { Flex, FormControl, Icon, Skeleton, Text } from '@chakra-ui/react'
 
-import { FormResponseMode } from '~shared/types'
+import { FormResponseMode, PaymentChannel } from '~shared/types'
 
 import { BxsCheckCircle, BxsError, BxsInfoCircle } from '~assets/icons'
 import FormLabel from '~components/FormControl/FormLabel'
@@ -101,7 +101,7 @@ const PaymentsSectionText = () => {
 
   if (
     settings?.responseMode === FormResponseMode.Encrypt &&
-    settings?.payments_channel
+    settings?.payments_channel.channel !== PaymentChannel.Unconnected
   ) {
     return (
       <>

shared/types/form/form.ts

@@ -118,8 +118,8 @@ export interface EmailFormBase extends FormBase {
 export interface StorageFormBase extends FormBase {
   responseMode: FormResponseMode.Encrypt
   publicKey: string
-  payments_channel?: FormPaymentsChannel
-  payments_field?: FormPaymentsField
+  payments_channel: FormPaymentsChannel
+  payments_field: FormPaymentsField
 }
 
 /**

shared/types/payment.ts

@@ -12,6 +12,7 @@ export enum PaymentStatus {
 }
 
 export enum PaymentChannel {
+  Unconnected = 'Unconnected',
   Stripe = 'Stripe',
   // for extensibility to future payment options
 }

src/app/models/__tests__/form.server.model.spec.ts

@@ -91,7 +91,7 @@ const FORM_DEFAULTS = {
 
 const PAYMENTS_DEFAULTS = {
   payments_channel: {
-    channel: PaymentChannel.Stripe,
+    channel: PaymentChannel.Unconnected,
     target_account_id: '',
     publishable_key: '',
   },

src/app/models/form.server.model.ts

@@ -133,11 +133,12 @@ const EncryptedFormSchema = new Schema<IEncryptedFormSchema>({
     type: String,
     required: true,
   },
+
   payments_channel: {
     channel: {
       type: String,
       enum: Object.values(PaymentChannel),
-      default: PaymentChannel.Stripe,
+      default: PaymentChannel.Unconnected,
     },
     target_account_id: {
       type: String,
@@ -149,7 +150,6 @@ const EncryptedFormSchema = new Schema<IEncryptedFormSchema>({
       default: '',
       validate: [/^\S*$/i, 'publishable_key must not contain whitespace.'],
     },
-    required: false,
   },
 
   payments_field: {
@@ -159,6 +159,7 @@ const EncryptedFormSchema = new Schema<IEncryptedFormSchema>({
     },
     description: {
       type: String,
+      trim: true,
       default: '',
     },
     amount_cents: {
@@ -170,7 +171,6 @@ const EncryptedFormSchema = new Schema<IEncryptedFormSchema>({
         message: 'Payment amount must be at least 50 cents and an integer.',
       },
     },
-    required: false,
   },
 })
 
@@ -184,7 +184,7 @@ EncryptedFormDocumentSchema.methods.addPaymentAccountId = async function ({
   accountId: FormPaymentsChannel['target_account_id']
   publishableKey: FormPaymentsChannel['publishable_key']
 }) {
-  if (!this.payments_channel) {
+  if (this.payments_channel?.channel === PaymentChannel.Unconnected) {
     this.payments_channel = {
       // Definitely Stripe for now, may be different later on.
       channel: PaymentChannel.Stripe,
@@ -196,11 +196,13 @@ EncryptedFormDocumentSchema.methods.addPaymentAccountId = async function ({
 }
 
 EncryptedFormDocumentSchema.methods.removePaymentAccount = async function () {
-  if (this.payments_channel) {
-    this.payments_channel = undefined
-    if (this.payments_field) {
-      this.payments_field.enabled = false
-    }
+  this.payments_channel = {
+    channel: PaymentChannel.Unconnected,
+    target_account_id: '',
+    publishable_key: '',
+  }
+  if (this.payments_field) {
+    this.payments_field.enabled = false
   }
   return this.save()
 }

src/app/modules/form/admin-form/admin-form.payments.controller.ts

@@ -84,7 +84,7 @@ export const handleConnectAccount: ControllerHandler<{
 }
 
 /**
- * Handler for DELETE /:formId/stripe
+ * Handler for DELETE /:formId/stripe.
  * @security session
  *
  * @returns 200 when Stripe credentials successfully deleted
@@ -134,6 +134,20 @@ export const handleUnlinkAccount: ControllerHandler<{
   )
 }
 
+/**
+ * Handler for GET /:formId/stripe/validate.
+ * @security session
+ *
+ * @returns 200 when Stripe credentials have been validated
+ * @returns 401 when user is not logged in
+ * @returns 403 when user does not have permissions to update the form
+ * @returns 404 when form to update cannot be found
+ * @returns 410 when form to update has been deleted
+ * @returns 422 when id of user who is updating the form cannot be found
+ * @returns 422 when the form to be updated is not an encrypt mode form
+ * @returns 500 when database error occurs
+ * @returns 502 when the connected Stripe credentials are invalid
+ */
 export const handleValidatePaymentAccount: ControllerHandler<{
   formId: string
 }> = async (req, res) => {
@@ -155,7 +169,7 @@ export const handleValidatePaymentAccount: ControllerHandler<{
       .andThen(checkFormIsEncryptMode)
       // Step 4: Validate the associated Stripe account.
       .andThen((form) =>
-        validateAccount(form.payments_channel?.target_account_id),
+        validateAccount(form.payments_channel.target_account_id),
       )
       .map((account) => res.json({ account }))
       .mapErr((error) => {
@@ -175,12 +189,13 @@ export const handleValidatePaymentAccount: ControllerHandler<{
 }
 
 /**
+ * Private handler for PUT /:formId/payment
  * NOTE: Exported for testing.
- * Private handler for PUT /forms/:formId/payment
  * @precondition Must be preceded by request validation
  * @security session
  *
  * @returns 200 with updated payments
+ * @returns 400 when updated payment amount is out of bounds
  * @returns 403 when current user does not have permissions to update the payments
  * @returns 404 when form cannot be found
  * @returns 410 when updating the payments for an archived form
@@ -232,7 +247,7 @@ export const _handleUpdatePayments: ControllerHandler<
 }
 
 /**
- * Handler for PUT /forms/:formId/payment
+ * Handler for PUT /:formId/payment
  */
 export const handleUpdatePayments = [
   celebrate({

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

@@ -333,12 +333,10 @@ const _handleConnectOauthCallback: ControllerHandler<
 > = async (req, res) => {
   const { code, state } = req.query
 
-  //Extracting state parameter previously signed and stored in cookies
+  // Step 0: Extract state parameter previously signed and stored in cookies.
+  // Compare state values to ensure that no tampering has occurred.
   const { stripeState } = req.signedCookies
-
-  //Comparing state parameters
   if (state !== stripeState) {
-    //throwing unprocessable entity error
     return res.status(StatusCodes.UNPROCESSABLE_ENTITY).json({
       message: 'Invalid state parameter',
     })

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

@@ -50,12 +50,6 @@ export class StripeAccountError extends ApplicationError {
   }
 }
 
-export class StripeAccountNotFoundError extends ApplicationError {
-  constructor(message = 'Stripe account not found') {
-    super(message)
-  }
-}
-
 export class ComputePaymentStateError extends ApplicationError {
   constructor(message = 'Error while computing payment state') {
     super(message)

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

@@ -8,7 +8,11 @@ import Stripe from 'stripe'
 import { MarkRequired } from 'ts-essentials'
 import isURL from 'validator/lib/isURL'
 
-import { IPaymentSchema, IPopulatedEncryptedForm } from '../../../types'
+import {
+  IEncryptedFormSchema,
+  IPaymentSchema,
+  IPopulatedEncryptedForm,
+} from '../../../types'
 import config from '../../config/config'
 import { paymentConfig } from '../../config/features/payment.config'
 import { createLoggerWithLabel } from '../../config/logger'
@@ -306,14 +310,8 @@ export const linkStripeAccountToForm = (
     accountId: string
     publishableKey: string
   },
-): ResultAsync<string, DatabaseError> => {
-  // Check if form already has account id
-  if (form.payments_channel) {
-    return okAsync(form.payments_channel.target_account_id)
-  }
-
-  // No account id, create and inject into form
-  return ResultAsync.fromPromise(
+): ResultAsync<string, DatabaseError> =>
+  ResultAsync.fromPromise(
     form.addPaymentAccountId({ accountId, publishableKey }),
     (error) => {
       const errMsg = 'Failed to update payment account id'
@@ -330,14 +328,11 @@ export const linkStripeAccountToForm = (
       return new DatabaseError(errMsg)
     },
   ).map((updatedForm) => updatedForm.payments_channel.target_account_id)
-}
-
-export const unlinkStripeAccountFromForm = (form: IPopulatedEncryptedForm) => {
-  if (!form.payments_channel) {
-    return okAsync(true)
-  }
 
-  return ResultAsync.fromPromise(form.removePaymentAccount(), (error) => {
+export const unlinkStripeAccountFromForm = (
+  form: IPopulatedEncryptedForm,
+): ResultAsync<IEncryptedFormSchema, DatabaseError> =>
+  ResultAsync.fromPromise(form.removePaymentAccount(), (error) => {
     const errMsg = 'Failed to remove payment account from form'
     logger.error({
       message: errMsg,
@@ -349,7 +344,6 @@ export const unlinkStripeAccountFromForm = (form: IPopulatedEncryptedForm) => {
     })
     return new DatabaseError(errMsg)
   })
-}
 
 export const createAccountLink = (
   accountId: string,

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

@@ -350,7 +350,7 @@ const submitEncryptModeForm: ControllerHandler<
   // Handle submissions for payments forms
   if (
     form.payments_field?.enabled &&
-    form.payments_channel?.channel === PaymentChannel.Stripe
+    form.payments_channel.channel === PaymentChannel.Stripe
   ) {
     // Step 0: Perform validation checks
     const amount = form.payments_field.amount_cents

src/types/form.ts

@@ -272,8 +272,10 @@ export interface IPopulatedForm extends Omit<IFormDocument, 'toJSON'> {
 
 export interface IEncryptedForm extends IForm {
   publicKey: string
-  payments_channel?: FormPaymentsChannel
-  payments_field?: FormPaymentsField
+  // Nested objects will always be returned from mongoose finds, even if they
+  // are not defined in DB. See https://github.com/Automattic/mongoose/issues/5310
+  payments_channel: FormPaymentsChannel
+  payments_field: FormPaymentsField
   emails?: never
 }