Conversation
….UK Pay integration
Created skeleton PaymentService
…engine-plugin into feat/df-623-payment
…engine-plugin into feat/df-623-payment
…s-engine-plugin into feat/df-623-payment-2
…s-engine-plugin into feat/df-623-payment-2
src/config/index.ts
Outdated
| paymentProviderApiKeyTest: { | ||
| doc: 'A test API key for integrating with a payment provider', | ||
| format: String, | ||
| nullable: true, | ||
| default: undefined, | ||
| env: 'PAYMENT_PROVIDER_API_KEY_TEST' | ||
| } as SchemaObj<string | undefined> |
There was a problem hiding this comment.
no need for a global value here, it'll be per form
| @@ -0,0 +1,31 @@ | |||
| --- | |||
| } | ||
|
|
||
| return this.isValue(value) ? value : null | ||
| return this.isValue(value) ? (value as Item['value']) : null |
| const amount = this.options.amount ?? 0 | ||
| const formattedAmount = amount.toFixed(2) |
There was a problem hiding this comment.
better if this comes from state, not the component options
we've got a difference in sources of truth:
getDisplayStringFromState's value comes from state
getViewModel's value comes from the component def
better if we keep this consistent and use paymentState everywhere. Maybe pull this out to a function getPaidAmount that returns the amount from state, which we can reuse in both places?
| throw new InvalidComponentStateError( | ||
| this, | ||
| 'Your payment authorisation has expired. Please add your payment details again.', | ||
| { shouldResetState: true, isPaymentExpired: true } | ||
| ) |
There was a problem hiding this comment.
glad to see this being used 💪
| /** | ||
| * Whether to reset the component state and redirect to the component's page. | ||
| * - `true`: Reset state and redirect (e.g., payment expired - user must re-enter) | ||
| * - `false`: Keep state and stay on current page with error (e.g., capture failed - user can retry) |
There was a problem hiding this comment.
this feels like the wrong exception to use for the false scenario. This exception is specifically for when the component state is invalid and should be cleared.
We can make another exception class or use Boom for 'standard' errors.
| * When true, an "Important" notification banner will be shown on the payment page. | ||
| * @default false | ||
| */ | ||
| isPaymentExpired?: boolean |
There was a problem hiding this comment.
could we make this a bit more generic? I generally would like to avoid page controllers being coupled to specific components
in this example, we could make the terminology a bit more generic but retain the same functionality. Maybe rather than isPaymentExpired we have notificationType: 'error' | 'important' or something to that effect? That way it's reusable between components.
| exampleField: 'hello world', | ||
| exampleField2: 'hello world' | ||
| }, | ||
| payments: {}, |
There was a problem hiding this comment.
I don't see a corresponding adapter/v1.ts change in this file, but why does payments need its own entry?
We could just have a single payment component (fixed component name). That way to access a payment you just do the usual data.main.payment rather than needing special handling via data.payments?
|
When I checkout out this branch in install npm deps, I get a change on |
Payment options now mandatory
…engine-plugin into feat/df-623-payment
| this.stateSchema = paymentStateSchema.default(null).allow(null) | ||
| // 'required()' forces the payment page to be invalid until we have valid payment state | ||
| // i.e. the user will automatically be directed back to the payment page | ||
| // if they attempt to access future pages wen no payment entered yet |
|
4 participants
Proposed change
Handle online payments
NOTE - requires a merge of designer PR DEFRA/forms-designer#1275 to change the structure of the PaymentField properties
Jira ticket: DF-623
Type of change
Checklist
README.mdanddocs/*(where appropriate, e.g. new features).npm run test).npm run lint).npm run format).