From a2f5c762693af15bb88fc32bfc1c54054568adf3 Mon Sep 17 00:00:00 2001 From: Abban Dunne Date: Wed, 15 Nov 2023 14:25:58 +0100 Subject: [PATCH] Reset Invalid Payment Data The store will now check if the payment type is Sofort and the interval is blank or 0. If not it will clear the payment type. Ticket: https://phabricator.wikimedia.org/T350848 --- src/pages/donation_form.ts | 6 +- src/store/intervalValidator.ts | 7 + src/store/payment/actions.ts | 31 ++- src/store/paymentTypeValidator.ts | 14 ++ .../PaymentInitialisationPayload.ts | 7 + src/view_models/PaymentType.ts | 7 + .../pages/donation_form/AddressPage.spec.ts | 13 +- tests/unit/store/donation_store.spec.ts | 20 +- tests/unit/store/payment_store.spec.ts | 190 +++++++++++++++--- 9 files changed, 241 insertions(+), 54 deletions(-) create mode 100644 src/store/intervalValidator.ts create mode 100644 src/store/paymentTypeValidator.ts create mode 100644 src/view_models/PaymentInitialisationPayload.ts create mode 100644 src/view_models/PaymentType.ts diff --git a/src/pages/donation_form.ts b/src/pages/donation_form.ts index fc229bb46..12a8e89e4 100644 --- a/src/pages/donation_form.ts +++ b/src/pages/donation_form.ts @@ -50,7 +50,11 @@ dataPersister.initialize( persistenceItems ).then( () => { Promise.all( [ store.dispatch( action( NS_PAYMENT, initializePayment ), - createInitialDonationPaymentValues( dataPersister, pageData.applicationVars.initialFormValues ) + { + initialValues: createInitialDonationPaymentValues( dataPersister, pageData.applicationVars.initialFormValues ), + allowedIntervals: pageData.applicationVars.paymentIntervals, + allowedPaymentTypes: pageData.applicationVars.paymentTypes, + } ), store.dispatch( action( NS_ADDRESS, initializeAddress ), diff --git a/src/store/intervalValidator.ts b/src/store/intervalValidator.ts new file mode 100644 index 000000000..6a4cf7dca --- /dev/null +++ b/src/store/intervalValidator.ts @@ -0,0 +1,7 @@ +export function isValidInterval( interval: string, allowedIntervals: number[] ): boolean { + if ( !allowedIntervals.includes( Number( interval ) ) ) { + return false; + } + + return true; +} diff --git a/src/store/payment/actions.ts b/src/store/payment/actions.ts index a062b22ee..01e587845 100644 --- a/src/store/payment/actions.ts +++ b/src/store/payment/actions.ts @@ -1,5 +1,5 @@ import { ActionContext } from 'vuex'; -import { InitialPaymentValues, IntervalData, TypeData } from '@src/view_models/Payment'; +import { IntervalData, TypeData } from '@src/view_models/Payment'; import { DonationPayment } from '@src/store/payment/types'; import { @@ -23,28 +23,37 @@ import { } from '@src/store/payment/mutationTypes'; import { Validity } from '@src/view_models/Validity'; import { isValidAmount } from '@src/store/amountValidator'; +import { PaymentInitialisationPayload } from '@src/view_models/PaymentInitialisationPayload'; +import { isValidPaymentType } from '@src/store/paymentTypeValidator'; +import { isValidInterval } from '@src/store/intervalValidator'; export const actions = { [ discardInitialization ]( context: ActionContext ): void { context.commit( SET_INITIALIZED, false ); }, - [ initializePayment ]( context: ActionContext, initialValues: InitialPaymentValues ): Promise { - let amountIsFilledAndValid = false, paymentIsFilled = false; - if ( initialValues.amount && initialValues.amount !== '0' ) { - amountIsFilledAndValid = isValidAmount( Number( initialValues.amount ) ); + [ initializePayment ]( context: ActionContext, payload: PaymentInitialisationPayload ): Promise { + const { initialValues, allowedPaymentTypes, allowedIntervals } = payload; + + const amountIsValid = isValidAmount( Number( initialValues.amount ) ); + const intervalIsValid = isValidInterval( initialValues.paymentIntervalInMonths, allowedIntervals ); + const paymentTypeIsValid = isValidPaymentType( initialValues.type, initialValues.paymentIntervalInMonths, allowedPaymentTypes ); + + if ( amountIsValid ) { context.commit( SET_AMOUNT, initialValues.amount ); - context.commit( SET_AMOUNT_VALIDITY, amountIsFilledAndValid ? Validity.VALID : Validity.INVALID ); + context.commit( SET_AMOUNT_VALIDITY, Validity.VALID ); } - if ( initialValues.type && initialValues.type !== '' ) { + context.commit( SET_INTERVAL, intervalIsValid ? initialValues.paymentIntervalInMonths : '0' ); + + if ( paymentTypeIsValid ) { context.commit( SET_TYPE, initialValues.type ); context.commit( SET_TYPE_VALIDITY, Validity.VALID ); - paymentIsFilled = true; } - context.commit( SET_INTERVAL, initialValues.paymentIntervalInMonths ); - context.commit( SET_INITIALIZED, amountIsFilledAndValid && paymentIsFilled ); - return Promise.resolve( amountIsFilledAndValid && paymentIsFilled ); + const paymentIsFilledAndValid = amountIsValid && paymentTypeIsValid; + context.commit( SET_INITIALIZED, paymentIsFilledAndValid ); + + return Promise.resolve( paymentIsFilledAndValid ); }, [ markEmptyValuesAsInvalid ]( context: ActionContext ): void { context.commit( MARK_EMPTY_FIELDS_INVALID ); diff --git a/src/store/paymentTypeValidator.ts b/src/store/paymentTypeValidator.ts new file mode 100644 index 000000000..70de1ab05 --- /dev/null +++ b/src/store/paymentTypeValidator.ts @@ -0,0 +1,14 @@ +import { PaymentType } from '@src/view_models/PaymentType'; + +export function isValidPaymentType( paymentType: string, interval: string, allowedPaymentTypes: string[] ): boolean { + if ( !allowedPaymentTypes.includes( paymentType ) ) { + return false; + } + + // Sofort payments can't be recurring so clear the type if it is initialised with an interval + if ( paymentType === PaymentType.SOFORT && ![ '', '0' ].includes( interval ) ) { + return false; + } + + return true; +} diff --git a/src/view_models/PaymentInitialisationPayload.ts b/src/view_models/PaymentInitialisationPayload.ts new file mode 100644 index 000000000..32004f5d1 --- /dev/null +++ b/src/view_models/PaymentInitialisationPayload.ts @@ -0,0 +1,7 @@ +import { InitialPaymentValues } from '@src/view_models/Payment'; + +export interface PaymentInitialisationPayload { + initialValues: InitialPaymentValues, + allowedIntervals: number[], + allowedPaymentTypes: string[] +} diff --git a/src/view_models/PaymentType.ts b/src/view_models/PaymentType.ts new file mode 100644 index 000000000..955b01e0e --- /dev/null +++ b/src/view_models/PaymentType.ts @@ -0,0 +1,7 @@ +export enum PaymentType { + DIRECT_DEBIT = 'BEZ', + BANK_TRANSFER = 'UEB', + CREDIT_CARD = 'MCP', + PAYPAL = 'PPL', + SOFORT = 'SUB' +} diff --git a/tests/unit/components/pages/donation_form/AddressPage.spec.ts b/tests/unit/components/pages/donation_form/AddressPage.spec.ts index d57192961..10a872afb 100644 --- a/tests/unit/components/pages/donation_form/AddressPage.spec.ts +++ b/tests/unit/components/pages/donation_form/AddressPage.spec.ts @@ -17,6 +17,7 @@ import { nextTick } from 'vue'; import AddressTypeBasic from '@src/components/pages/donation_form/AddressTypeBasic.vue'; import { Validity } from '@src/view_models/Validity'; import { Salutation } from '@src/view_models/Salutation'; +import { PaymentInitialisationPayload } from '@src/view_models/PaymentInitialisationPayload'; const testCountry = { countryCode: 'de', @@ -72,10 +73,14 @@ describe( 'AddressPage.vue', () => { const setPaymentType = ( store: Store, paymentType: string ): Promise => { return store.dispatch( action( NS_PAYMENT, initializePayment ), { - amount: '100', - type: paymentType, - paymentIntervalInMonths: '0', - isCustomAmount: false, + allowedIntervals: [ 0 ], + allowedPaymentTypes: [ paymentType ], + initialValues: { + amount: '100', + type: paymentType, + paymentIntervalInMonths: '0', + isCustomAmount: false, + }, } ); }; diff --git a/tests/unit/store/donation_store.spec.ts b/tests/unit/store/donation_store.spec.ts index f0341358a..b49567e62 100644 --- a/tests/unit/store/donation_store.spec.ts +++ b/tests/unit/store/donation_store.spec.ts @@ -5,6 +5,8 @@ import { action } from '@src/store/util'; import { NS_ADDRESS, NS_PAYMENT } from '@src/store/namespaces'; import { initializeAddress } from '@src/store/address/actionTypes'; import { initializePayment } from '@src/store/payment/actionTypes'; +import { PaymentInitialisationPayload } from '@src/view_models/PaymentInitialisationPayload'; +import { PaymentType } from '@src/view_models/PaymentType'; describe( 'Donation Store', () => { @@ -29,15 +31,19 @@ describe( 'Donation Store', () => { const type = 'person'; const paymentIntervalInMonths = '1'; const isCustomAmount = false; - - const initialData = { - amount, - type, - paymentIntervalInMonths, - isCustomAmount, + const payload: PaymentInitialisationPayload = { + allowedIntervals: [ 1 ], + allowedPaymentTypes: [ 'person' ], + initialValues: { + amount, + type, + paymentIntervalInMonths, + isCustomAmount, + }, }; + const store = createStore(); - await store.dispatch( action( NS_PAYMENT, initializePayment ), initialData ); + await store.dispatch( action( NS_PAYMENT, initializePayment ), payload ); expect( store.state.payment.values.amount ).toBe( amount ); expect( store.state.payment.values.type ).toBe( type ); diff --git a/tests/unit/store/payment_store.spec.ts b/tests/unit/store/payment_store.spec.ts index 2a549bb07..a8ed75520 100644 --- a/tests/unit/store/payment_store.spec.ts +++ b/tests/unit/store/payment_store.spec.ts @@ -17,6 +17,8 @@ import { import each from 'jest-each'; import { DonationPayment } from '@src/store/payment/types'; import { ActionContext } from 'vuex'; +import { PaymentType } from '@src/view_models/PaymentType'; +import { PaymentInitialisationPayload } from '@src/view_models/PaymentInitialisationPayload'; function newMinimalStore( overrides: Object ): DonationPayment { return Object.assign( @@ -171,24 +173,36 @@ describe( 'Payment', () => { it( 'does not commit empty amount', () => { const commit = jest.fn(); const action = actions[ initializePayment ] as any; - const initialPayment = { - amount: '0', - type: 'BEZ', - paymentIntervalInMonths: 12, + const payload: PaymentInitialisationPayload = { + allowedIntervals: [ 12 ], + allowedPaymentTypes: [ 'BEZ' ], + initialValues: { + amount: '0', + type: 'BEZ', + paymentIntervalInMonths: '12', + isCustomAmount: false, + }, }; - action( { commit }, initialPayment ); + + action( { commit }, payload ); expect( commit ).not.toBeCalledWith( SET_AMOUNT, '0' ); } ); it( 'commits amount and sets it to valid when amount is set', () => { const commit = jest.fn(); const action = actions[ initializePayment ] as any; - const initialPayment = { - amount: '2399', - type: 'BEZ', - paymentIntervalInMonths: 12, + const payload: PaymentInitialisationPayload = { + allowedIntervals: [ 12 ], + allowedPaymentTypes: [ 'BEZ' ], + initialValues: { + amount: '2399', + type: 'BEZ', + paymentIntervalInMonths: '12', + isCustomAmount: true, + }, }; - action( { commit }, initialPayment ); + + action( { commit }, payload ); expect( commit ).toBeCalledWith( SET_AMOUNT, '2399' ); expect( commit ).toBeCalledWith( SET_AMOUNT_VALIDITY, Validity.VALID ); } ); @@ -196,24 +210,36 @@ describe( 'Payment', () => { it( 'does not commit empty payment type', () => { const commit = jest.fn(); const action = actions[ initializePayment ] as any; - const initialPayment = { - amount: '123', - type: '', - paymentIntervalInMonths: 12, + const payload: PaymentInitialisationPayload = { + allowedIntervals: [ 12 ], + allowedPaymentTypes: [ 'BEZ' ], + initialValues: { + amount: '123', + type: '', + paymentIntervalInMonths: '12', + isCustomAmount: true, + }, }; - action( { commit }, initialPayment ); - expect( commit ).not.toBeCalledWith( SET_TYPE, '' ); + + action( { commit }, payload ); + expect( commit ).not.toBeCalledWith( SET_TYPE ); } ); it( 'commits payment type and set it to valid when payment type is set', () => { const commit = jest.fn(); const action = actions[ initializePayment ] as any; - const initialPayment = { - amount: '2399', - type: 'BEZ', - paymentIntervalInMonths: 12, + const payload: PaymentInitialisationPayload = { + allowedIntervals: [ 12 ], + allowedPaymentTypes: [ 'BEZ' ], + initialValues: { + amount: '2399', + type: 'BEZ', + paymentIntervalInMonths: '12', + isCustomAmount: true, + }, }; - action( { commit }, initialPayment ); + + action( { commit }, payload ); expect( commit ).toBeCalledWith( SET_TYPE, 'BEZ' ); expect( commit ).toBeCalledWith( SET_TYPE_VALIDITY, Validity.VALID ); } ); @@ -221,12 +247,18 @@ describe( 'Payment', () => { it( 'commits interval', () => { const commit = jest.fn(); const action = actions[ initializePayment ] as any; - const initialPayment = { - amount: '2399', - type: 'BEZ', - paymentIntervalInMonths: '12', + const payload: PaymentInitialisationPayload = { + allowedIntervals: [ 12 ], + allowedPaymentTypes: [ 'BEZ' ], + initialValues: { + amount: '2399', + type: 'BEZ', + paymentIntervalInMonths: '12', + isCustomAmount: true, + }, }; - action( { commit }, initialPayment ); + + action( { commit }, payload ); expect( commit ).toBeCalledWith( SET_INTERVAL, '12' ); } ); @@ -242,15 +274,111 @@ describe( 'Payment', () => { it( `whose amount is ${ data.amount } and type is ${ data.type } should be ${ data.expectedResolution }`, () => { const commit = jest.fn(); const action = actions[ initializePayment ] as any; - const initialValues = { - amount: data.amount, - type: data.type, - paymentIntervalInMonths: '0', + const payload: PaymentInitialisationPayload = { + allowedIntervals: [ 0, 12 ], + allowedPaymentTypes: [ 'BEZ', 'PPL' ], + initialValues: { + amount: data.amount, + type: data.type, + paymentIntervalInMonths: '0', + isCustomAmount: false, + }, }; + expect.assertions( 1 ); - return expect( action( { commit }, initialValues ) ).resolves.toBe( data.expectedResolution ); + return expect( action( { commit }, payload ) ).resolves.toBe( data.expectedResolution ); } ); } ); + + it( 'does not initialise Sofort payment type if initialised with an interval', () => { + const commit = jest.fn(); + const action = actions[ initializePayment ] as any; + const payload: PaymentInitialisationPayload = { + allowedIntervals: [ 0, 12 ], + allowedPaymentTypes: [ PaymentType.SOFORT ], + initialValues: { + amount: '2399', + type: PaymentType.SOFORT, + paymentIntervalInMonths: '12', + isCustomAmount: false, + }, + }; + + action( { commit }, payload ); + expect( commit ).not.toBeCalledWith( SET_TYPE ); + } ); + + it( 'initialises Sofort payment type if interval is 0', () => { + const commit = jest.fn(); + const action = actions[ initializePayment ] as any; + const payload: PaymentInitialisationPayload = { + allowedIntervals: [ 0, 12 ], + allowedPaymentTypes: [ PaymentType.SOFORT ], + initialValues: { + amount: '2399', + type: PaymentType.SOFORT, + paymentIntervalInMonths: '0', + isCustomAmount: false, + }, + }; + + action( { commit }, payload ); + expect( commit ).toBeCalledWith( SET_TYPE, PaymentType.SOFORT ); + } ); + + it( 'initialises Sofort payment type if interval is emtpy', () => { + const commit = jest.fn(); + const action = actions[ initializePayment ] as any; + const payload: PaymentInitialisationPayload = { + allowedIntervals: [ 0, 12 ], + allowedPaymentTypes: [ PaymentType.SOFORT ], + initialValues: { + amount: '2399', + type: PaymentType.SOFORT, + paymentIntervalInMonths: '', + isCustomAmount: false, + }, + }; + + action( { commit }, payload ); + expect( commit ).toBeCalledWith( SET_TYPE, PaymentType.SOFORT ); + } ); + + it( 'does not initialise payment type if it is not in allowed list', () => { + const commit = jest.fn(); + const action = actions[ initializePayment ] as any; + const payload: PaymentInitialisationPayload = { + allowedIntervals: [ 0, 12 ], + allowedPaymentTypes: [ PaymentType.PAYPAL ], + initialValues: { + amount: '2399', + type: PaymentType.DIRECT_DEBIT, + paymentIntervalInMonths: '0', + isCustomAmount: false, + }, + }; + + action( { commit }, payload ); + expect( commit ).not.toBeCalledWith( SET_TYPE, PaymentType.DIRECT_DEBIT ); + } ); + + it( 'does not initialise interval if it is not in allowed list', () => { + const commit = jest.fn(); + const action = actions[ initializePayment ] as any; + const payload: PaymentInitialisationPayload = { + allowedIntervals: [ 0, 12 ], + allowedPaymentTypes: [ PaymentType.PAYPAL ], + initialValues: { + amount: '2399', + type: PaymentType.DIRECT_DEBIT, + paymentIntervalInMonths: '42', + isCustomAmount: false, + }, + }; + + action( { commit }, payload ); + expect( commit ).not.toBeCalledWith( SET_INTERVAL, PaymentType.DIRECT_DEBIT ); + } ); } ); describe( 'Actions/markEmptyAmountAsInvalid', () => {