Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: payment max and min limit #5972

Merged
merged 22 commits into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7e2f64f
feat: add paymentMaxLimit env var
tshuli Mar 22, 2023
bdc2d47
feat: paymentMaxLimit check on frontend
tshuli Mar 22, 2023
c45f0b4
chore: rename to maxPaymentAmount
tshuli Mar 22, 2023
5c730ca
lint: magic string in MACRO_CASE
tshuli Mar 22, 2023
7be7bb1
chore: create payment.errors file
tshuli Mar 22, 2023
e089d68
feat: add backend check for maxPaymentAmount
tshuli Mar 22, 2023
3577bd3
chore: add tests
tshuli Mar 22, 2023
8abadfc
chore: max payment amount in cents
tshuli Mar 22, 2023
5688716
fix: frontend validation in dollars
tshuli Mar 22, 2023
11cc661
feat: add minimum payment limit
tshuli Mar 22, 2023
760ea45
chore: map error route for InvalidPaymentAmountError
tshuli Mar 22, 2023
fa5308e
chore: use spread operator for updatedPaymentSettings
tshuli Mar 22, 2023
0bd5f0b
chore: combine min and max payment checks
tshuli Mar 22, 2023
0493f2d
nit: dangling import in other file
tshuli Mar 22, 2023
e5d988d
chore: use centsToDollars
tshuli Mar 22, 2023
243a664
chore: no need to convert env var values to string
tshuli Mar 23, 2023
663724e
chore: keep reference amount in cents for comparator
tshuli Mar 23, 2023
04a7165
Merge remote-tracking branch 'origin/feat/payment-max-limit' into fea…
tshuli Mar 23, 2023
665d045
Merge remote-tracking branch 'origin/feat/payment-mvp' into feat/paym…
tshuli Mar 23, 2023
3c0f0d1
Merge remote-tracking branch 'origin/feat/payment-mvp' into feat/paym…
tshuli Mar 23, 2023
0a40704
chore: fix tests
tshuli Mar 24, 2023
9b3d47f
Merge remote-tracking branch 'origin/feat/payment-mvp' into feat/paym…
tshuli Mar 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ services:
- PAYMENT_STRIPE_SECRET_KEY=secretKey
- PAYMENT_STRIPE_CLIENT_ID=clientId
- PAYMENT_STRIPE_WEBHOOK_SECRET=webhookSecret
- PAYMENT_MAX_PAYMENT_AMOUNT_CENTS=100000
- PAYMENT_MIN_PAYMENT_AMOUNT_CENTS=50

mockpass:
build: https://github.com/opengovsg/mockpass.git
depends_on:
Expand Down
23 changes: 14 additions & 9 deletions frontend/src/features/admin-form/create/payment/PaymentDrawer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import Toggle from '~components/Toggle'

import { useMutateFormPage } from '~features/admin-form/common/mutations'

import { useEnv } from '../../../env/queries'
import {
setIsDirtySelector,
useDirtyFieldStore,
Expand Down Expand Up @@ -54,6 +55,9 @@ export const PaymentInput = (): JSX.Element => {
const isMobile = useIsMobile()
const { paymentsMutation } = useMutateFormPage()

const { data: { maxPaymentAmountCents, minPaymentAmountCents } = {} } =
useEnv()

const setIsDirty = useDirtyFieldStore(setIsDirtySelector)

const {
Expand Down Expand Up @@ -127,9 +131,6 @@ export const PaymentInput = (): JSX.Element => {

const handleCloseDrawer = useCallback(() => handleClose(false), [handleClose])

const minPaymentAmount = 0.5 // stipulated by Stripe
const maxPaymentAmount = 1000 // due to IRAS requirements and agency financial institutions are expected to be in SG

const amountValidation: RegisterOptions<
FormPaymentsDisplay,
'display_amount'
Expand All @@ -147,24 +148,28 @@ export const PaymentInput = (): JSX.Element => {
)
},
validateMin: (val) => {
if (minPaymentAmountCents === undefined) return true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/justNoting (nothing to do): this defensive checks helps at release (new code hitting old server for env), but feels unnecessary at steady state thereafter 🤔 We should expect the client to be able to load valid env vars to operate, or things wouldn't work generally.

return (
Number(val?.trim()) >= minPaymentAmount ||
// val is in dollars
(val && dollarsToCents(val) >= minPaymentAmountCents) ||
`Please enter a payment amount above ${formatCurrency(
minPaymentAmount,
Number(centsToDollars(minPaymentAmountCents)),
LinHuiqing marked this conversation as resolved.
Show resolved Hide resolved
)}`
)
},
validateMax: (val) => {
if (maxPaymentAmountCents === undefined) return true
return (
Number(val?.trim()) <= maxPaymentAmount ||
`Please keep payment amount under ${formatCurrency(
maxPaymentAmount,
// val is in dollars
(val && dollarsToCents(val) <= maxPaymentAmountCents) ||
`Please enter a payment amount below ${formatCurrency(
Number(centsToDollars(maxPaymentAmountCents)),
)}`
)
},
},
}),
[],
[maxPaymentAmountCents, minPaymentAmountCents],
)

const handleUpdatePayments = handleSubmit(() => {
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/templates/Field/Nric/NricField.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* @precondition Must have a parent `react-hook-form#FormProvider` component.
*/
import { useMemo, useState } from 'react'
import { useMemo } from 'react'
import { useFormContext } from 'react-hook-form'

import { createNricValidationRules } from '~utils/fieldValidation'
Expand Down
2 changes: 2 additions & 0 deletions shared/types/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,6 @@ export type ClientEnvVars = {
removeAdminInfoboxThreshold: number
removeRespondentsInfoboxThreshold: number
stripePublishableKey: string
maxPaymentAmountCents: number
minPaymentAmountCents: number
}
14 changes: 14 additions & 0 deletions src/app/config/features/payment.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ export interface IStripe {
stripeSecretKey: string
stripeClientID: string
stripeWebhookSecret: string
maxPaymentAmountCents: number
minPaymentAmountCents: number
}

const paymentFeature: Schema<IStripe> = {
Expand Down Expand Up @@ -39,6 +41,18 @@ const paymentFeature: Schema<IStripe> = {
default: '',
env: 'PAYMENT_STRIPE_WEBHOOK_SECRET',
},
maxPaymentAmountCents: {
doc: 'Maximum amount that can be paid for a form',
format: Number,
default: 100000, // $1000, due to IRAS limit for invoice
env: 'PAYMENT_MAX_PAYMENT_AMOUNT_CENTS',
},
minPaymentAmountCents: {
doc: 'Minimum that can be paid for a form',
format: Number,
default: 50, // $0.50, as specified by stripe
env: 'PAYMENT_MIN_PAYMENT_AMOUNT_CENTS',
},
}

export const paymentConfig = convict(paymentFeature)
Expand Down
7 changes: 7 additions & 0 deletions src/app/loaders/express/locals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import ejs from 'ejs'
import config from '../../config/config'
import { captchaConfig } from '../../config/features/captcha.config'
import { googleAnalyticsConfig } from '../../config/features/google-analytics.config'
import { paymentConfig } from '../../config/features/payment.config'
import { sentryConfig } from '../../config/features/sentry.config'
import { spcpMyInfoConfig } from '../../config/features/spcp-myinfo.config'

Expand Down Expand Up @@ -33,6 +34,9 @@ const frontendVars = {
config.reactMigration.respondentRolloutStorage,
reactMigrationAdminRollout: config.reactMigration.adminRollout,
reactMigrationAngularPhaseOutDate: config.reactMigration.angularPhaseOutDate,
// payment variables
maxPaymentAmountCents: paymentConfig.maxPaymentAmountCents,
minPaymentAmountCents: paymentConfig.minPaymentAmountCents,
}
const environment = ejs.render(
`
Expand Down Expand Up @@ -64,6 +68,9 @@ const environment = ejs.render(
var reactMigrationRespondentRolloutStorage = "<%= reactMigrationRespondentRolloutStorage%>"
var reactMigrationAdminRollout = "<%= reactMigrationAdminRollout%>"
var reactMigrationAngularPhaseOutDate = "<%= reactMigrationAngularPhaseOutDate%>"
// Payment
var maxPaymentAmountCents = <%= maxPaymentAmountCents%>
var minPaymentAmountCents = <%= minPaymentAmountCents%>
`,
frontendVars,
)
Expand Down
117 changes: 117 additions & 0 deletions src/app/modules/form/admin-form/__tests__/admin-form.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,12 @@ import {
FormStatus,
LogicDto,
LogicType,
PaymentsUpdateDto,
SettingsUpdateDto,
} from '../../../../../../shared/types'
import { smsConfig } from '../../../../config/features/sms.config'
import * as SmsService from '../../../../services/sms/sms.service'
import { InvalidPaymentAmountError } from '../../../payments/payment.errors'
import {
FormNotFoundError,
LogicNotFoundError,
Expand Down Expand Up @@ -2628,4 +2630,119 @@ describe('admin-form.service', () => {
expect(twilioCacheSpy).toHaveBeenCalledWith(MSG_SRVC_NAME)
})
})

describe('updatePayments', () => {
// Arrange
const mockFormId = new ObjectId().toString()
const updatedPaymentSettings = {
enabled: true,
target_account_id: 'someId',
publishable_key: 'somekey',
amount_cents: 5000,
description: 'some description',
} as PaymentsUpdateDto

const mockUpdatedForm = {
_id: mockFormId,
payments: updatedPaymentSettings,
}

it('should return InvalidPaymentAmountError if payment amount exceeds maxPaymentAmountCents', async () => {
// Arrange

const updatedPaymentSettingsExceeded = {
...updatedPaymentSettings,
amount_cents: 500000,
} as PaymentsUpdateDto

// Act
const actualResult = await AdminFormService.updatePayments(
mockFormId,
updatedPaymentSettingsExceeded,
)

// Assert
expect(actualResult.isErr()).toEqual(true)
expect(actualResult._unsafeUnwrapErr()).toBeInstanceOf(
InvalidPaymentAmountError,
)
})

it('should return InvalidPaymentAmountError if payment amount is below minPaymentAmountCents', async () => {
// Arrange

const updatedPaymentSettingsBelow = {
...updatedPaymentSettings,
amount_cents: 49,
} as PaymentsUpdateDto

// Act
const actualResult = await AdminFormService.updatePayments(
mockFormId,
updatedPaymentSettingsBelow,
)

// Assert
expect(actualResult.isErr()).toEqual(true)
expect(actualResult._unsafeUnwrapErr()).toBeInstanceOf(
InvalidPaymentAmountError,
)
})

it('should successfuly call updatePaymentsById with formId and newPayments and return the updated payment settings', async () => {
// Arrange
const putSpy = jest
.spyOn(FormModel, 'updatePaymentsById')
.mockResolvedValueOnce(mockUpdatedForm as unknown as IFormDocument)

// Act
const actualResult = await AdminFormService.updatePayments(
mockFormId,
updatedPaymentSettings,
)

// Assert
expect(putSpy).toHaveBeenCalledWith(mockFormId, updatedPaymentSettings)

expect(actualResult.isOk()).toEqual(true)
// Should equal updatedPaymentSettings obj
expect(actualResult._unsafeUnwrap()).toEqual(updatedPaymentSettings)
})

it('should return PossibleDatabaseError if db update fails', async () => {
// Arrange
const putSpy = jest
.spyOn(FormModel, 'updatePaymentsById')
.mockRejectedValueOnce(new DatabaseError())

// Act
const actualResult = await AdminFormService.updatePayments(
mockFormId,
updatedPaymentSettings,
)

// Assert
expect(putSpy).toHaveBeenCalledWith(mockFormId, updatedPaymentSettings)
expect(actualResult.isErr()).toEqual(true)
expect(actualResult._unsafeUnwrapErr()).toBeInstanceOf(DatabaseError)
})

it('should return FormNotFoundError if no form is returned after updating db', async () => {
// Arrange
const putSpy = jest
.spyOn(FormModel, 'updatePaymentsById')
.mockResolvedValueOnce(null)

// Act
const actualResult = await AdminFormService.updatePayments(
mockFormId,
updatedPaymentSettings,
)

// Assert
expect(putSpy).toHaveBeenCalledWith(mockFormId, updatedPaymentSettings)
expect(actualResult.isErr()).toEqual(true)
expect(actualResult._unsafeUnwrapErr()).toBeInstanceOf(FormNotFoundError)
})
})
})
17 changes: 16 additions & 1 deletion src/app/modules/form/admin-form/admin-form.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {
} from '../../../../types'
import { EditFormFieldParams, FormUpdateParams } from '../../../../types/api'
import config, { aws as AwsConfig } from '../../../config/config'
import { paymentConfig } from '../../../config/features/payment.config'
import { createLoggerWithLabel } from '../../../config/logger'
import getFormModel, {
getEncryptedFormModel,
Expand All @@ -66,6 +67,7 @@ import {
SecretsManagerNotFoundError,
TwilioCacheError,
} from '../../core/core.errors'
import { InvalidPaymentAmountError } from '../../payments/payment.errors'
import { MissingUserError } from '../../user/user.errors'
import * as UserService from '../../user/user.service'
import { SmsLimitExceededError } from '../../verification/verification.errors'
Expand Down Expand Up @@ -1518,14 +1520,27 @@ const deleteTwilioTransaction = async (
* @returns ok(updated start page object) when update is successful
* @returns err(FormNotFoundError) if form cannot be found
* @returns err(PossibleDatabaseError) if start page update fails
* @returns err(InvalidPaymentAmountError) if payment amount exceeds MAX_PAYMENT_AMOUNT
*/
export const updatePayments = (
formId: string,
newPayments: PaymentsUpdateDto,
): ResultAsync<
IEncryptedFormDocument['payments_field'],
PossibleDatabaseError | FormNotFoundError
PossibleDatabaseError | FormNotFoundError | InvalidPaymentAmountError
> => {
const { amount_cents } = newPayments

// Check if payment amount exceeds maxPaymentAmountCents or below minPaymentAmountCents
if (amount_cents) {
if (
amount_cents > paymentConfig.maxPaymentAmountCents ||
amount_cents < paymentConfig.minPaymentAmountCents
) {
return errAsync(new InvalidPaymentAmountError())
}
}

return ResultAsync.fromPromise(
EncryptedFormModel.updatePaymentsById(formId, newPayments),
(error) => {
Expand Down
6 changes: 6 additions & 0 deletions src/app/modules/form/admin-form/admin-form.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
TwilioCacheError,
} from '../../core/core.errors'
import { ErrorResponseData } from '../../core/core.types'
import { InvalidPaymentAmountError } from '../../payments/payment.errors'
import {
StripeAccountError,
StripeAccountNotFoundError,
Expand Down Expand Up @@ -168,6 +169,11 @@ export const mapRouteError = (
statusCode: StatusCodes.INTERNAL_SERVER_ERROR,
errorMessage: coreErrorMessage ?? error.message,
}
case InvalidPaymentAmountError:
return {
statusCode: StatusCodes.BAD_REQUEST,
errorMessage: error.message,
}
default:
logger.error({
message: 'Unknown route error observed',
Expand Down
2 changes: 2 additions & 0 deletions src/app/modules/frontend/frontend.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,7 @@ export const getClientEnvVars = (): ClientEnvVars => {
removeRespondentsInfoboxThreshold:
config.reactMigration.removeRespondentsInfoboxThreshold,
stripePublishableKey: paymentConfig.stripePublishableKey,
maxPaymentAmountCents: paymentConfig.maxPaymentAmountCents,
minPaymentAmountCents: paymentConfig.minPaymentAmountCents,
}
}
13 changes: 13 additions & 0 deletions src/app/modules/payments/payment.errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { ApplicationError } from '../core/core.errors'

export class PaymentNotFoundError extends ApplicationError {
constructor(message = 'Payment not found') {
super(message)
}
}

export class InvalidPaymentAmountError extends ApplicationError {
constructor(message = 'Invalid payment amount') {
super(message)
}
}
10 changes: 3 additions & 7 deletions src/app/modules/payments/payments.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,13 @@ import { IPaymentSchema } from '../../../types'
import { createLoggerWithLabel } from '../../config/logger'
import getPaymentModel from '../../models/payment.server.model'
import { getMongoErrorMessage } from '../../utils/handle-mongo-error'
import { ApplicationError, DatabaseError } from '../core/core.errors'
import { DatabaseError } from '../core/core.errors'

import { PaymentNotFoundError } from './payment.errors'

const logger = createLoggerWithLabel(module)
const PaymentModel = getPaymentModel(mongoose)

export class PaymentNotFoundError extends ApplicationError {
constructor(message = 'Payment not found') {
super(message)
}
}

/**
* Retrieves submission document of the given SubmissionId.
* @param submissionId the submissionId of the payment to be retrieved
Expand Down