Merge pull request #588 from wmde/fix-iban-check

Fix IBAN conversion validation
wmde · Aug 3, 2016 · eaff563 · eaff563
2 parents 0c97813 + c48f8c0
commit eaff563
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 26 deletions.
diff --git a/app/js/lib/form_validation.js b/app/js/lib/form_validation.js
@@ -4,6 +4,17 @@ var jQuery = require( 'jquery' ),
 	objectAssign = require( 'object-assign' ),
 	_ = require( 'underscore' ),
 
+	ValidationStates = {
+		OK: 'OK',
+		ERR: 'ERR',
+		INCOMPLETE: 'INCOMPLETE',
+		NOT_APPLICABLE: 'NOT_APPLICABLE'
+	},
+
+	isEmptyString = function ( value ) {
+		return value === '';
+	},
+
 	DefaultRequiredFieldsForAddressType = {
 		person: [ 'salutation', 'firstName', 'lastName', 'street', 'postcode', 'city', 'email' ],
 		firma: [ 'companyName', 'street', 'postcode', 'city', 'email' ],
@@ -17,17 +28,16 @@ var jQuery = require( 'jquery' ),
 		validate: function ( formValues ) {
 			var requiredFields = this.getRequiredFieldsForAddressType( formValues.addressType );
 			if ( this.formValuesHaveEmptyRequiredFields( formValues, requiredFields ) ) {
-				return { status: 'INCOMPLETE' };
+				return { status: ValidationStates.INCOMPLETE };
 			}
 			// Don't send anything to server if there are no fields to validate
 			if ( requiredFields.length === 0 ) {
-				return { status: 'OK' };
+				return { status: ValidationStates.OK };
 			}
 			return this.sendFunction( this.validationUrl, formValues, null, 'json' );
 		},
 		formValuesHaveEmptyRequiredFields: function ( formValues, requiredFields ) {
-			var objectWithOnlyTheRequiredFields = _.pick( formValues, requiredFields ),
-				isEmptyString = function ( value ) { return value === ''; };
+			var objectWithOnlyTheRequiredFields = _.pick( formValues, requiredFields );
 			return _.find( objectWithOnlyTheRequiredFields, isEmptyString ) !== undefined;
 		},
 		getRequiredFieldsForAddressType: function ( addressType ) {
@@ -79,25 +89,59 @@ var jQuery = require( 'jquery' ),
 		validationUrlForNonSepa: '',
 		sendFunction: null,
 		validate: function ( formValues ) {
-			var data, validationUrl;
 			if ( formValues.paymentType && formValues.paymentType !== 'BEZ' ) {
 				return {
-					status: 'OK'
+					status: ValidationStates.NOT_APPLICABLE
 				};
 			}
+
 			if ( formValues.debitType === 'sepa' ) {
-				data = {
+				return this.validateSepa( formValues );
+			}
+
+			return this.validateNonSepa( formValues );
+		},
+		/**
+		 * @private
+		 */
+		validateSepa: function ( formValues ) {
+			if ( formValues.iban === '' ) {
+				return { status: ValidationStates.INCOMPLETE };
+			}
+
+			return this.getValidationResultFromApi(
+				this.validationUrlForSepa,
+				{
 					iban: formValues.iban
-				};
-				validationUrl = this.validationUrlForSepa;
-			} else {
-				data = {
+				}
+			);
+		},
+		/**
+		 * @private
+		 */
+		validateNonSepa: function ( formValues ) {
+			if ( formValues.accountNumber === '' || formValues.bankCode === '' ) {
+				return { status: ValidationStates.INCOMPLETE };
+			}
+
+			return this.getValidationResultFromApi(
+				this.validationUrlForNonSepa,
+				{
 					accountNumber: formValues.accountNumber,
 					bankCode: formValues.bankCode
-				};
-				validationUrl = this.validationUrlForNonSepa;
-			}
-			return this.sendFunction( validationUrl, data, null, 'json' );
+				}
+			);
+		},
+		/**
+		 * @private
+		 */
+		getValidationResultFromApi: function ( apiUrl, urlArguments ) {
+			return this.sendFunction(
+				apiUrl,
+				urlArguments,
+				null,
+				'json'
+			);
 		}
 	},
 
@@ -160,5 +204,6 @@ module.exports = {
 	createSepaConfirmationValidator: function () {
 		return Object.create( SepaConfirmationValidator );
 	},
-	DefaultRequiredFieldsForAddressType: DefaultRequiredFieldsForAddressType
+	DefaultRequiredFieldsForAddressType: DefaultRequiredFieldsForAddressType,
+	ValidationStates: ValidationStates
 };
diff --git a/app/js/lib/reducers/input_validation.js b/app/js/lib/reducers/input_validation.js
@@ -1,7 +1,8 @@
 'use strict';
 
 var _ = require( 'underscore' ),
-	objectAssign = require( 'object-assign' );
+	objectAssign = require( 'object-assign' ),
+	ValidationStates = require( '../form_validation' ).ValidationStates;
 
 function inputIsValid( value, pattern ) {
 	if ( pattern === null ) {
@@ -36,19 +37,25 @@ function inputValidation( validationState, action ) {
 			} );
 			return newValidationState;
 		case 'FINISH_AMOUNT_VALIDATION':
-			newValidationState.amount = { dataEntered: true, isValid: action.payload.status !== 'ERR' };
+			newValidationState.amount = { dataEntered: true, isValid: action.payload.status !== ValidationStates.ERR };
 			return newValidationState;
 		case 'FINISH_BANK_DATA_VALIDATION':
-			bankDataIsValid = action.payload.status !== 'ERR';
+			if ( action.payload.status === ValidationStates.INCOMPLETE || action.payload.status === ValidationStates.NOT_APPLICABLE ) {
+				return newValidationState;
+			}
+			bankDataIsValid = action.payload.status !== ValidationStates.ERR;
 			newValidationState.iban = { dataEntered: true, isValid: bankDataIsValid };
 			newValidationState.bic = { dataEntered: true, isValid: bankDataIsValid };
 			newValidationState.account = { dataEntered: true, isValid: bankDataIsValid };
 			newValidationState.bankCode = { dataEntered: true, isValid: bankDataIsValid };
 			return newValidationState;
 		case 'FINISH_EMAIL_ADDRESS_VALIDATION':
-			newValidationState.email = { dataEntered: true, isValid: action.payload.status !== 'ERR' };
+			newValidationState.email = { dataEntered: true, isValid: action.payload.status !== ValidationStates.ERR };
 			return newValidationState;
 		case 'FINISH_ADDRESS_VALIDATION':
+			if ( action.payload.status === ValidationStates.INCOMPLETE ) {
+				return newValidationState;
+			}
 			_.forEach( newValidationState, function ( value, key ) {
 				if ( newValidationState[ key ].dataEntered === true ) {
 					newValidationState[ key ] = {

diff --git a/app/js/tests/test_form_validation.js b/app/js/tests/test_form_validation.js
@@ -157,14 +157,14 @@ test( 'Given non-sepa debit type, bank data validation sends account number and
 	t.end();
 } );
 
-test( 'Given a non-debit payment type, bank data validation succeeds', function ( t ) {
+test( 'Given a non-debit payment type, bank data validation is not applicable', function ( t ) {
 	var postFunctionSpy = sinon.spy(),
 		bankDataValidator = validation.createBankDataValidator(
 			'http://spenden.wikimedia.org/check-iban',
 			'http://spenden.wikimedia.org/generate-iban',
 			postFunctionSpy
 		),
-		expectedValidationResult = { status: 'OK' },
+		expectedValidationResult = { status: 'NOT_APPLICABLE' },
 		validationResult;
 
 	validationResult = bankDataValidator.validate( {
@@ -178,7 +178,7 @@ test( 'Given a non-debit payment type, bank data validation succeeds', function
 	} );
 
 	t.equal( postFunctionSpy.callCount, 0, 'data is not sent' );
-	t.deepEqual( validationResult, expectedValidationResult, 'validation is successful' );
+	t.deepEqual( validationResult, expectedValidationResult, 'validation result ' );
 	t.end();
 } );
 

diff --git a/web/res/js/donationForm.js b/web/res/js/donationForm.js
@@ -64,9 +64,12 @@ $( function () {
 					initialValues
 				),
 				WMDE.ReduxValidation.createValidationDispatcher(
-					WMDE.FormValidation.createBankDataValidator( initData.data( 'validate-iban-url' ), initData.data( 'generate-iban-url' ) ),
+					WMDE.FormValidation.createBankDataValidator(
+						initData.data( 'validate-iban-url' ),
+						initData.data( 'generate-iban-url' )
+					),
 					actions.newFinishBankDataValidationAction,
-					[ 'iban', 'accountNumber', 'bankCode' ],
+					[ 'iban', 'accountNumber', 'bankCode', 'debitType', 'paymentType' ],
 					initialValues
 				),
 				WMDE.ReduxValidation.createValidationDispatcher(

diff --git a/web/res/js/membershipForm.js b/web/res/js/membershipForm.js
@@ -82,7 +82,7 @@ $( function () {
 						initData.data( 'generate-iban-url' )
 					),
 					actions.newFinishBankDataValidationAction,
-					[ 'iban', 'accountNumber', 'bankCode' ],
+					[ 'iban', 'accountNumber', 'bankCode', 'debitType', 'paymentType' ],
 					initialValues
 				),
 				WMDE.ReduxValidation.createValidationDispatcher(