Skip to content

Commit

Permalink
Refactor BankData component
Browse files Browse the repository at this point in the history
Pass in BankDataValidator as a property
Simplify validation function
Make tests synchronous and clean them up.
Change construction in donationForm. The BankDataValidator instance
should passed in as a property in initialization, but can't be due to
nadimtuhin/redux-vue#6
  • Loading branch information
gbirke committed Oct 17, 2018
1 parent a727555 commit dee203f
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 134 deletions.
42 changes: 16 additions & 26 deletions skins/cat17/src/app/components/BankData.vue
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@
export default {
name: 'bank-data',
props: {
changeIban: Function,
changeBic: Function,
changeBankDataValidity: Function,
validateBankData: Function,
bankDataValidator: Object,
iban: String,
bic: String,
isValid: Boolean
Expand All @@ -35,62 +33,54 @@
bicValue: '',
bankName: '',
errorText: 'Placeholder error text',
hasError: false,
isValidBic: true,
bicFilled: false
hasError: false
};
},
watch: {
iban( v ) {
if ( this.looksLikeIban( v ) ) {
if ( this.isIbanEmpty() || this.looksLikeIban() ) {
this.ibanValue = v;
}
},
bic( v ) {
if ( this.looksLikeIban( this.ibanValue ) ) {
if ( this.isBicEmpty() || this.looksLikeIban() ) {
this.bicValue = v;
}
}
},
methods: {
handleIbanChange( evt ) {
this.ibanValue = evt.target.value;
if ( this.looksLikeIban( this.ibanValue ) ) {
this.changeIban( evt.target.value );
}
},
handleBicChange( evt ) {
this.bicValue = evt.target.value;
if ( this.looksLikeIban( this.ibanValue ) ) {
this.changeBic( evt.target.value );
}
},
validate() {
if ( this.ibanValue === '' || ( this.bicValue === '' && !this.looksLikeIban( this.ibanValue ) ) ) {
return this.changeBankDataValidity( { status: 'INCOMPLETE', iban: this.ibanValue, bic: this.bicValue } );
if ( this.isIbanEmpty() ) {
return;
}
if ( this.looksLikeIban( this.ibanValue ) ) {
this.changeBankDataValidity( this.bankDataValidator.validateSepaBankData( this.ibanValue ) );
return;
}
return this.validateBankData( this.ibanValue, this.bicValue, this.looksLikeIban( this.ibanValue ) )
.then( this.changeBankDataValidity )
.catch( () => {
this.errorText = 'An error has occurred. Please reload the page and try again.'; // TODO translate
this.hasError = true;
} );
this.changeBankDataValidity( this.bankDataValidator.validateClassicBankData( this.ibanValue, this.bicValue ) );
},
isIbanEmpty: function () {
return this.ibanValue === '';
},
isBicEmpty: function () {
return this.bicValue === '';
},
looksLikeIban: function ( ibanValue ) {
return /^[A-Z]+([0-9]+)?$/.test( ibanValue );
looksLikeIban: function () {
return /^[A-Z]+([0-9]+)?$/.test( this.ibanValue );
},
looksLikeBankAccountNumber: function () {
return /^\d+$/.test( this.ibanValue );
}
},
computed: {
labels() {
// TODO Translate these strings
if ( this.looksLikeIban() ) {
return {
iban: 'IBAN',
Expand All @@ -114,7 +104,7 @@
classesIBAN() {
return {
'field-grp': true,
'field-iba': true,
'field-iban': true,
'field-labeled': true,
invalid: !this.isValid,
valid: this.isValid && !this.isIbanEmpty()
Expand All @@ -123,7 +113,7 @@
classesBIC() {
return {
'field-grp': true,
'field-iba': true,
'field-bic': true,
'field-labeled': true,
invalid: !this.isValid && !this.isBicEmpty(),
valid: this.isValid && !this.isBicEmpty()
Expand Down
2 changes: 1 addition & 1 deletion skins/cat17/src/app/lib/form_validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ var objectAssign = require( 'object-assign' ),
* @return {BankDataValidator}
*/
createBankDataValidator = function ( validationUrlForSepaBankData, validationUrlForClassicBankData, transport ) {
return new BankDataValidator( validationUrlForSepaBankData, validationUrlForClassicBankData, transport );
return new BankDataValidator( validationUrlForSepaBankData, validationUrlForClassicBankData, transport || new JQueryTransport() );
}
;

Expand Down
137 changes: 55 additions & 82 deletions skins/cat17/src/app/tests/components/BankData.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@ import BankData from '../../components/BankData.vue';
function newTestProperties( overrides ) {
return Object.assign(
{
changeIban: function () {},
changeBic: function () {},
validateBankData: function () { return Promise.resolve(); },
bankDataValidator: {
validateClassicBankData: function () { },
validateSepaBankData: function () { }
},
changeBankDataValidity: function () {},
iban: '',
bic: '',
Expand All @@ -23,41 +24,6 @@ test( 'BankData.vue renders', t => {
t.equal( typeof wrapper, 'object' );
} );

test( 'Given IBAN value, BankData.vue calls changeIban if value looks like IBAN', t => {
t.plan( 1 );
let testIban = '';
const wrapper = shallowMount( BankData, {
propsData: newTestProperties( {
changeIban: function ( iban ) { testIban = iban; }
} )
} );

const ibanInput = wrapper.find( '#iban' );
ibanInput.setValue( 'DE123' );
ibanInput.trigger( 'input' );

t.equal( testIban, 'DE123' );
} );

test( 'Given non-German IBAN value and BIC, BankData.vue calls changeBIC if IBAN value looks like IBAN', t => {
t.plan( 1 );
let testBIC = '';
const wrapper = shallowMount( BankData, {
propsData: newTestProperties( {
changeBic: function ( bic ) { testBIC = bic; }
} )
} );

const ibanInput = wrapper.find( '#iban' );
const bicInput = wrapper.find( '#bic' );
ibanInput.setValue( 'AT123' );
ibanInput.trigger( 'input' );
bicInput.setValue( '98765' );
bicInput.trigger( 'input' );

t.equal( testBIC, '98765' );
} );

test( 'Given German IBAN value, BIC-field becomes disabled', t => {
t.plan( 2 );
const wrapper = shallowMount( BankData, {
Expand Down Expand Up @@ -87,17 +53,16 @@ test( 'Given IBAN value changes in IBAN and BIC property, BankData.vue renders t
t.equal( bicInput.element.value, '' );

wrapper.setProps( {
...initialProperties,
bic: '98765',
iban: 'DE123'
iban: 'DE123',
bic: '98765'
} );

t.equal( ibanInput.element.value, 'DE123' );
t.equal( bicInput.element.value, '98765' );
} );

test( 'Given classic account value, BankData.vue does not render changes to IBAN and BIC field values', t => {
t.plan( 4 );
test( 'Given classic account value, BankData.vue does only render changes to IBAN and BIC field values if they are empty', t => {
t.plan( 6 );
const initialProperties = newTestProperties( {} );
const wrapper = shallowMount( BankData, {
propsData: initialProperties
Expand All @@ -109,40 +74,61 @@ test( 'Given classic account value, BankData.vue does not render changes to IBAN
t.equal( bicInput.element.value, '' );

wrapper.setProps( {
...initialProperties,
bic: '98765',
iban: '123'
iban: '123',
bic: '98765'
} );

t.equal( ibanInput.element.value, '' );
t.equal( bicInput.element.value, '' );
t.equal( ibanInput.element.value, '123' );
t.equal( bicInput.element.value, '98765' );

wrapper.setProps( {
iban: '777',
bic: '888'
} );

t.equal( ibanInput.element.value, '123' );
t.equal( bicInput.element.value, '98765' );
} );

test( 'Given empty IBAN value on IBAN blur, validation is not triggered and validity is incomplete', t => {
t.plan( 1 );
test( 'Given empty IBAN value on IBAN blur, validation is not triggered and no result is set', t => {
const wrapper = shallowMount( BankData, {
propsData: newTestProperties( {
validateBankData: function () { return Promise.fail(); },
changeBankDataValidity: function ( validity ) {
t.equal( validity.status, 'INCOMPLETE' );
bankDataValidator: {
validateSepaBankData() {
t.fail();
},
validateClassicBankData() {
t.fail();
}
},
changeBankDataValidity() {
t.fail();
}
} )
} );

const ibanInput = wrapper.find( '#iban' );
ibanInput.trigger( 'blur' );
t.end();
} );

test( 'Given filled IBAN value on IBAN blur, validation is triggered and validity is set to validation result', t => {
t.plan( 1 );
test( 'Given filled IBAN value on IBAN blur, SEPA validation is triggered and validity is set to validation result', t => {
t.plan( 2 );
const validationResult = {
state: 'OK',
iban: 'DE12500105170648489890',
bic: 'INGDDEFFXXX'
};
const fakeBankDataValidator = {
validateClassicBankData: function () { },
validateSepaBankData: function ( v ) {
t.equals( v, 'DE12500105170648489890' );
return validationResult;
}
};
const wrapper = shallowMount( BankData, {
propsData: newTestProperties( {
validateBankData: function () { return Promise.resolve( validationResult ); },
bankDataValidator: fakeBankDataValidator,
changeBankDataValidity: function ( validity ) {
t.deepEqual( validity, validationResult );
}
Expand All @@ -155,16 +141,24 @@ test( 'Given filled IBAN value on IBAN blur, validation is triggered and validit
ibanInput.trigger( 'blur' );
} );

test( 'Given filled classic values on blur, validation is triggered and validity is set to validation result', t => {
t.plan( 1 );
test( 'Given filled classic values on blur, classic validation is triggered and validity is set to validation result', t => {
t.plan( 3 );
const validationResult = {
state: 'OK',
iban: 'DE12500105170648489890',
bic: 'INGDDEFFXXX'
};
const fakeBankDataValidator = {
validateClassicBankData: function ( accountNumber, bankNumber ) {
t.equals( accountNumber, '0648489890' );
t.equals( bankNumber, '50010517' );
return validationResult;
},
validateSepaBankData: function () { t.fail(); }
};
const wrapper = shallowMount( BankData, {
propsData: newTestProperties( {
validateBankData: function () { return Promise.resolve( validationResult ); },
bankDataValidator: fakeBankDataValidator,
changeBankDataValidity: function ( validity ) {
t.deepEqual( validity, validationResult );
}
Expand All @@ -182,27 +176,6 @@ test( 'Given filled classic values on blur, validation is triggered and validity
ibanInput.trigger( 'blur' );
} );

test( 'Given filled only one of the classic values on blur, validation is validation is not triggered and validity is incomplete', t => {
t.plan( 2 );
const wrapper = shallowMount( BankData, {
propsData: newTestProperties( {
validateBankData: function () { return Promise.fail(); },
changeBankDataValidity: function ( validity ) {
t.deepEqual( validity.status, 'INCOMPLETE' );
}
} )
} );

const ibanInput = wrapper.find( '#iban' );
const bicInput = wrapper.find( '#bic' );
// TODO test validity classes

ibanInput.setValue( '0648489890' );
ibanInput.trigger( 'input' );
ibanInput.trigger( 'blur' );

ibanInput.setValue( '' );
ibanInput.trigger( 'input' );
bicInput.setValue( '50010517' );
bicInput.trigger( 'input' );
bicInput.trigger( 'blur' );
} );
// TODO test labels when we have an i18n solution
44 changes: 19 additions & 25 deletions skins/cat17/src/scripts/donationForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -282,47 +282,41 @@ $( function () {
WMDE.Scrolling.scrollOnSuboptionChange( $( 'input[name="addressType"]' ), $( '#type-donor' ), scroller );
WMDE.Scrolling.scrollOnSuboptionChange( $( 'input[name="zahlweise"]' ), $( '#donation-payment' ), scroller );

var bankDataValidator = WMDE.FormValidation.createBankDataValidator(
initData.data( 'validate-iban-url' ),
initData.data( 'generate-iban-url' )
);

function mapStateToProps( state ) {
return {
iban: state.donationFormContent.iban,
bic: state.donationFormContent.bic,
isValid: state.validity.bankData !== false
isValid: state.validity.bankData !== false,

// The validator does not come from the store and should be passed
// in as a prop the initialization code,
// see https://github.com/nadimtuhin/redux-vue/issues/6
bankDataValidator: bankDataValidator
}
}

function mapActionToProps( dispatch ) {
return {
changeIban ( iban ) {
dispatch( WMDE.Actions.newChangeContentAction( 'iban', iban ) );
},
changeBic( bic ) {
dispatch( WMDE.Actions.newChangeContentAction( 'bic', bic ) );
},
changeBankDataValidity( validity ) {
dispatch( WMDE.Actions.newFinishBankDataValidationAction( validity ) );
},
validateBankData( iban, bic, isIBAN ) {
var bankDataValidator = WMDE.FormValidation.createBankDataValidator(
initData.data( 'validate-iban-url' ),
initData.data( 'generate-iban-url' )
);
if (isIBAN) {
return bankDataValidator.validateIban( iban );
}
return bankDataValidator.validateClassicAccountNumber( iban, bic );
}
}

}

/** global: WMDE */

WMDE.Vue.use(WMDE.VueRedux.reduxStorePlugin);

new WMDE.Vue({
// FIXME Import and create store directly when we no longer use the global variable anywhere else
store: store,
render: (h) => h( WMDE.VueRedux.connect( mapStateToProps, mapActionToProps )( WMDE.BankData ) )
}).$mount('#bankdata-app');
var ConnectedBankData = WMDE.VueRedux.connect( mapStateToProps, mapActionToProps )( WMDE.BankData );

new WMDE.Vue( {
// FIXME Import and create store directly when we no longer use the global variable anywhere else
store: store,
render: (h) => h( ConnectedBankData )

} ).$mount( '#bankdata-app' );

} );

0 comments on commit dee203f

Please sign in to comment.