From 2e5250afe123e83dea2bcb82b4fcd8002a58bc23 Mon Sep 17 00:00:00 2001 From: Martin Kamerbeek Date: Fri, 18 Dec 2009 13:45:50 +0100 Subject: [PATCH] Refactored EU tax plugin code to reduce code duplication and added tests. --- docs/upgrades/upgrade_7.8.8-7.8.9.pl | 25 +- lib/WebGUI/Shop/TaxDriver/EU.pm | 101 ++++--- t/Shop/TaxDriver/EU.t | 423 ++++++++++++++++----------- 3 files changed, 326 insertions(+), 223 deletions(-) diff --git a/docs/upgrades/upgrade_7.8.8-7.8.9.pl b/docs/upgrades/upgrade_7.8.8-7.8.9.pl index a59e6d4e7..d49ab1817 100644 --- a/docs/upgrades/upgrade_7.8.8-7.8.9.pl +++ b/docs/upgrades/upgrade_7.8.8-7.8.9.pl @@ -31,18 +31,29 @@ my $quiet; # this line required my $session = start(); # this line required # upgrade functions go here +addEuTaxRecheckWorkflow( $session ); finish($session); # this line required #---------------------------------------------------------------------------- -# Describe what our function does -#sub exampleFunction { -# my $session = shift; -# print "\tWe're doing some stuff here that you should know about... " unless $quiet; -# # and here's our code -# print "DONE!\n" unless $quiet; -#} +sub addEuTaxRecheckWorkflow { + my $session = shift; + + print "\tAdding EU Tax plugin VAT number recheck workflow..." unless $quiet; + + my $workflow = WebGUI::Workflow->create( $session, { + title => 'Recheck unverified EU VAT numbers', + description => + 'Utility workflow that automatically rechecks VAT numbers that could not be checked when they were submitted', + enabled => 1, + type => 'None', + mode => 'parallel', + }, 'taxeurecheckworkflow01' ); + $workflow->addActivity( 'WebGUI::Workflow::Activity::RecheckVATNumber', 'taxeurecheckactivity01' ); + + print "Done\n" unless $quiet; +} # -------------- DO NOT EDIT BELOW THIS LINE -------------------------------- diff --git a/lib/WebGUI/Shop/TaxDriver/EU.pm b/lib/WebGUI/Shop/TaxDriver/EU.pm index 11355983f..b5b3b1fb3 100644 --- a/lib/WebGUI/Shop/TaxDriver/EU.pm +++ b/lib/WebGUI/Shop/TaxDriver/EU.pm @@ -150,34 +150,13 @@ sub addVATNumber { my $db = $self->session->db; my $i18n = WebGUI::International->new( $self->session, 'TaxDriver_EU' ); - WebGUI::Error::InvalidParam->throw( 'A VAT number is required' ) - unless $number; - WebGUI::Error::InvalidParam->throw( 'The second argument must be an instanciated WebGUI::User object' ) - unless ref $user eq 'WebGUI::User'; - WebGUI::Error::InvalidParam->throw( 'Visitor cannot add VAT numbers' ) - if $user->isVisitor; - - # Check number - my $validator = Business::Tax::VAT::Validation->new; - my $numberIsValid = $localCheckOnly ? $validator->local_check( $number ) : $validator->check( $number ); - - # Number contains syntax error does not exist. Do not write the code to the db. - if ( !$numberIsValid && $validator->get_last_error_code <= 16 ) { + my $result = $self->updateVATNumber( $number, $user, $localCheckOnly ); + + if ( $result eq 'INVALID' ) { return $i18n->get('vat number invalid'); } - - # Write the code to the db. - $db->write( 'replace into tax_eu_vatNumbers (userId,countryCode,vatNumber,viesValidated,viesErrorCode,approved) values (?,?,?,?,?,?)', [ - $user->userId, - substr( $number, 0 , 2 ), - $number, - $numberIsValid ? 1 : 0, - $numberIsValid ? undef : $validator->get_last_error_code, - 0, - ] ); - - if ( $numberIsValid ) { - return undef; + elsif ( $result eq 'VALID' ) { + return; } else { my $workflow = WebGUI::Workflow::Instance->create( $self->session, { @@ -192,6 +171,49 @@ sub addVATNumber { } } + +sub updateVATNumber { + my $self = shift; + my $number = shift; + my $user = shift || $self->session->user; + my $localCheckOnly = shift; + my $db = $self->session->db; + + WebGUI::Error::InvalidParam->throw( 'A VAT number is required' ) + unless $number; + WebGUI::Error::InvalidParam->throw( 'The second argument must be an instanciated WebGUI::User object' ) + unless ref $user eq 'WebGUI::User'; + WebGUI::Error::InvalidParam->throw( 'Visitor cannot add VAT numbers' ) + if $user->isVisitor; + + # Check number + my $validator = Business::Tax::VAT::Validation->new; + my $numberIsValid = $localCheckOnly ? $validator->local_check( $number ) : $validator->check( $number ); + + # Number contains syntax error does not exist. Do not write the code to the db. + if ( !$numberIsValid && $validator->get_last_error_code <= 16 ) { + return 'INVALID'; + } + + # Write the code to the db. + $db->write( 'replace into tax_eu_vatNumbers (userId,countryCode,vatNumber,viesValidated,viesErrorCode,approved) values (?,?,?,?,?,?)', [ + $user->userId, + substr( $number, 0 , 2 ), + $number, + $numberIsValid ? 1 : 0, + $numberIsValid ? undef : $validator->get_last_error_code, + 0, + ] ); + + if ( $numberIsValid ) { + return 'VALID'; + } + else { + return 'UNKNOWN'; + } +} + + #------------------------------------------------------------------- =head2 appendCartItemVars ( var, cartItem ) @@ -845,32 +867,9 @@ sub recheckVATNumber { my $number = shift; my $user = shift || $self->session->user; - my $validator = Business::Tax::VAT::Validation->new; + my $result = $self->updateVATNumber( $number, $user ); - my $isValid = $validator->check( $number ); - my $errorCode = $validator->get_last_error_code; - - if ( $isValid ) { - $self->session->db->write( - 'update tax_eu_vatNumbers set viesValidated=?, viesErrorCode=? where vatNumber=? and userId=?', - [ - 1, - undef, - $number, - $user->userId, - ], - ); - - return 'VALID'; - } - elsif ( $errorCode < 17 ) { - $self->deleteVATNumber( $number, $user ); - - return 'INVALID'; - } - else { - return 'UNKNOWN'; - } + return $result; } #------------------------------------------------------------------- diff --git a/t/Shop/TaxDriver/EU.t b/t/Shop/TaxDriver/EU.t index d1dcfa1b6..5f6c77359 100644 --- a/t/Shop/TaxDriver/EU.t +++ b/t/Shop/TaxDriver/EU.t @@ -32,22 +32,54 @@ use WebGUI::Shop::AddressBook; # Init my $session = WebGUI::Test->session; +# Test user my $taxUser = WebGUI::User->new( $session, 'new' ); $taxUser->username( 'Tex Evasion' ); WebGUI::Test->usersToDelete($taxUser); +# Test VAT numbers +my $testVAT_NL = 'NL123456789B12'; +my $testVAT_BE = 'BE0123456789'; +my $noServiceVAT= 'NotGonnaWork'; +my $invalidVAT = 'ByNoMeansAllowed'; +my $visitorUser = WebGUI::User->new( $session, 1 ); + +# Test SKU my $sku = WebGUI::Asset->getRoot($session)->addChild( { className => 'WebGUI::Asset::Sku::Donation', title => 'Taxable donation', defaultPrice => 100.00, } ); +my $book = WebGUI::Shop::AddressBook->create($session); + +# setup address in EU but not in residential country of merchant +my $beAddress = $book->addAddress({ + label => 'BE', + city => 'Antwerpen', + country => 'Belgium', +}); + +# setup address in residential country of merchant +my $nlAddress = $book->addAddress({ + label => 'NL', + city => 'Delft', + country => 'Netherlands', +}); + +# setup address outside EU +my $usAddress = $book->addAddress({ + label => 'outside eu', + city => 'New Amsterdam', + country => 'US', +}); + my $cart; #---------------------------------------------------------------------------- # Tests -my $tests = 58; +my $tests = 317; plan tests => 1 + $tests; #---------------------------------------------------------------------------- @@ -59,12 +91,12 @@ SKIP: { skip 'Unable to load module WebGUI::Shop::TaxDriver::EU', $tests unless $loaded; - ####################################################################### - # - # new - # - ####################################################################### - +####################################################################### +# +# new +# +####################################################################### +{ my $taxer = WebGUI::Shop::TaxDriver::EU->new($session); isa_ok($taxer, 'WebGUI::Shop::TaxDriver::EU'); @@ -72,46 +104,49 @@ SKIP: { isa_ok($taxer->session, 'WebGUI::Session', 'session method returns a session object'); is($session->getId, $taxer->session->getId, 'session method returns OUR session object'); +} - ####################################################################### - # - # className - # - ####################################################################### - +####################################################################### +# +# className +# +####################################################################### +{ + my $taxer = WebGUI::Shop::TaxDriver::EU->new($session); + is( $taxer->className, 'WebGUI::Shop::TaxDriver::EU', 'className returns correct class name' ); +} - ####################################################################### - # - # getConfigurationScreen - # - ####################################################################### +####################################################################### +# +# getConfigurationScreen +# +####################################################################### - #### TODO: Figure out how to test this. - - ####################################################################### - # - # getCountryCode - # - ####################################################################### +#### TODO: Figure out how to test this. +####################################################################### +# +# getCountryCode / getCOuntryName +# +####################################################################### +{ + my $taxer = WebGUI::Shop::TaxDriver::EU->new($session); + is( $taxer->getCountryCode( 'Netherlands' ), 'NL', 'getCountryCode returns correct code for country inside EU.' ); is( $taxer->getCountryCode( 'United States' ), undef, 'getCountryCode returns undef for countries outside EU.' ); - ####################################################################### - # - # getCountryName - # - ####################################################################### - is( $taxer->getCountryName( 'NL' ), 'Netherlands', 'getCountryName returns correct name for country code within EU.' ); is( $taxer->getCountryName( 'US' ), undef, 'getCountryName returns undef for county codes outside EU.' ); +} - ####################################################################### - # - # addVATNumber - # - ####################################################################### +####################################################################### +# +# updateVATNumber +# +####################################################################### +{ + my $taxer = WebGUI::Shop::TaxDriver::EU->new($session); $session->user( {userId=>$taxUser->userId} ); @@ -120,59 +155,118 @@ SKIP: { local *Business::Tax::VAT::Validation::new; $validator->fake_new( 'Business::Tax::VAT::Validation' ); - my $testVAT_NL = 'NL123456789B12'; - my $testVAT_BE = 'BE0123456789'; - my $noServiceVAT= 'NotGonnaWork'; - my $invalidVAT = 'ByNoMeansAllowed'; - my $visitorUser = WebGUI::User->new( $session, 1 ); - - eval { $taxer->addVATNumber }; + eval { $taxer->updateVATNumber }; my $e = Exception::Class->caught(); isa_ok( $e, 'WebGUI::Error::InvalidParam', 'A VAT number is required' ); - is( $e, 'A VAT number is required', 'addVATNumber returns correct message for missing VAT number' ); + is( $e, 'A VAT number is required', 'updateVATNumber returns correct message for missing VAT number' ); - eval { $taxer->addVATNumber( $testVAT_NL, 'NotAUserObject' ) }; + eval { $taxer->updateVATNumber( $testVAT_NL, 'NotAUserObject' ) }; $e = Exception::Class->caught(); isa_ok( $e, 'WebGUI::Error::InvalidParam', 'Second argument must be a user object' ); - is( $e, 'The second argument must be an instanciated WebGUI::User object', 'addVATNumber returns correct message when user object is of wrong type' ); + is( $e, 'The second argument must be an instanciated WebGUI::User object', 'updateVATNumber returns correct message when user object is of wrong type' ); - eval { $taxer->addVATNumber( $testVAT_NL, $visitorUser ) }; + eval { $taxer->updateVATNumber( $testVAT_NL, $visitorUser ) }; $e = Exception::Class->caught(); isa_ok( $e, 'WebGUI::Error::InvalidParam', 'User may not be visitor' ); - is( $e, 'Visitor cannot add VAT numbers', 'addVATNumber returns correct message when user is visitor' ); + is( $e, 'Visitor cannot add VAT numbers', 'updateVATNumber returns correct message when user is visitor' ); + + for my $errorCode ( 0 .. 16 ) { + $validator->set_always( 'check', 0 ); + $validator->set_always( 'get_last_error_code', $errorCode ); + + is( + $taxer->updateVATNumber( $invalidVAT, $taxUser ), + 'INVALID', + "updateVATNumber returns INVALID for error $errorCode", + ); + } + + for my $errorCode ( 17 .. 255 ) { + $validator->set_always( 'check', 0 ); + $validator->set_always( 'get_last_error_code', $errorCode ); + + is( + $taxer->updateVATNumber( $invalidVAT, $taxUser ), + 'UNKNOWN', + "updateVATNumber returns UNKNOWN for error $errorCode", + ); + } + + $validator->set_always( 'check', 1 ); + $validator->set_always( 'get_last_error_code', undef ); + is( + $taxer->updateVATNumber( $testVAT_NL, $taxUser ), + 'VALID', + "updateVATNumber returns VALID for valid numbers", + ); +} + +####################################################################### +# +# addVATNumber +# +####################################################################### +{ + my $taxer = WebGUI::Shop::TaxDriver::EU->new($session); + + my $response; + local *WebGUI::Shop::TaxDriver::EU::updateVATNumber = sub { return $response }; #----- invalid vat number - $validator->set_always( 'check', 0 ); - $validator->set_always( 'get_last_error_code', 1 ); - my $response = $taxer->addVATNumber( $invalidVAT, $taxUser ); - is( $response, 'The entered VAT number is invalid.', 'Invalid VAT numbers return an error message' ); + $response = 'INVALID'; + is( + $taxer->addVATNumber( $invalidVAT, $taxUser ), + 'The entered VAT number is invalid.', + 'addVATNumber returns the correct error message for invalid numbers', + ); #----- service unavailable - $validator->set_always( 'check', 0 ); - $validator->set_always( 'get_last_error_code', 17 ); - my $response = $taxer->addVATNumber( $noServiceVAT, $taxUser ); - ok( $response =~ m{^Number validation is currently not available.}, 'When VIES is down a message is returned' ); + $response = 'UNKNOWN'; + like( + $taxer->addVATNumber( $noServiceVAT, $taxUser ), + qr{^Number validation is currently not available.}, + 'addVATNumber returns the correct message when VIES is unavailable', + ); my $workflows = WebGUI::Workflow::Instance->getAllInstances( $session ); my ($workflow) = grep { $_->get('parameters')->{ vatNumber } eq $noServiceVAT } @{ $workflows }; - ok( defined $workflow , 'A recheck workflow has been fired' ); + ok( defined $workflow , 'addVATNumber fires a recheck workflow when VIES is down' ); #----- valid number - $validator->set_always( 'check', 1 ); - $validator->set_always( 'get_last_error_code', undef ); - my $response = $taxer->addVATNumber( $testVAT_NL, $taxUser ); - ok( !defined $response , 'Valid VAT numbers return undef.' ); + $response = 'VALID'; + ok( + !defined $taxer->addVATNumber( $testVAT_NL, $taxUser ), + 'Valid VAT numbers return undef.', + ); +} - # Add another number for later testing purposes. - my $responseBE = $taxer->addVATNumber( $testVAT_BE, $taxUser ); +####################################################################### +# +# recheckVATNumber +# +####################################################################### +{ + my $taxer = WebGUI::Shop::TaxDriver::EU->new($session); + for my $response ( qw{ INVALID VALID UNKNOWN } ) { + local *WebGUI::Shop::TaxDriver::EU::updateVATNumber = sub { return $response }; - ####################################################################### - # - # getVATNumbers - # - ####################################################################### + is( + $taxer->recheckVATNumber( $invalidVAT, $taxUser ), + $response, + "recheckVATNumber returns correct value when updateVATNumber returns $response", + ); + } +} +####################################################################### +# +# getVATNumbers / deleteVATNumber +# +####################################################################### +{ + my $taxer = setupTestNumbers(); + my $expectNL = { userId => $taxUser->userId, countryCode => 'NL', @@ -196,25 +290,23 @@ SKIP: { $vatNumbers = $taxer->getVATNumbers( 'BE', $taxUser ); cmp_bag( $vatNumbers, [ $expectBE ], 'getVATNumbers filters on country code when one is passed' ); - ####################################################################### - # - # deleteVATNumber - # - ####################################################################### - $taxer->deleteVATNumber( $testVAT_BE, $taxUser ); $vatNumbers = $taxer->getVATNumbers( undef, $taxUser ); cmp_bag( $vatNumbers, [ $expectNL ], 'deleteVATNumber deletes number' ); $taxer->deleteVATNumber( $testVAT_NL, $taxUser ); - ####################################################################### - # - # addGroupRate - # - ####################################################################### +} + +####################################################################### +# +# addGroup / getGroupRate / deleteGroup +# +####################################################################### +{ + my $taxer = setupTestNumbers(); eval { $taxer->addGroup }; - $e = Exception::Class->caught(); + my $e = Exception::Class->caught(); isa_ok( $e, 'WebGUI::Error::InvalidParam', 'addGroup requires a group name' ); is( $e, 'A group name is required', 'addGroup returns correct message for omitted group name' ); @@ -265,51 +357,54 @@ SKIP: { ]; cmp_bag( $taxGroups, $expectGroups, 'addGroup saves correctly' ); - - ####################################################################### - # # getGroupRate - # - ####################################################################### - - ok( $taxer->getGroupRate( $id0 ) == 0 - && $taxer->getGroupRate( $id100 ) == 100 - && $taxer->getGroupRate( $id50_5 ) == 50.5, + ok( + $taxer->getGroupRate( $id0 ) == 0 + && $taxer->getGroupRate( $id100 ) == 100 + && $taxer->getGroupRate( $id50_5 ) == 50.5, 'getGroup rate gets correct rates' ); + + # deleteGroup + eval { $taxer->deleteGroup }; + my $e = Exception::Class->caught(); + isa_ok( $e, 'WebGUI::Error::InvalidParam', 'addGroup requires a group id' ); + is( $e, 'A group id is required', 'addGroup returns correct message for missing group id' ); - ####################################################################### - # - # getTaxRate - # - ####################################################################### + $taxer->deleteGroup( $id50_5 ); + + $taxGroups = $taxer->get( 'taxGroups' ); + cmp_bag( $taxGroups, [ + { + name => 'Group0', + rate => 0, + id => $id0, + }, + { + name => 'Group100', + rate => 100, + id => $id100, + }, + ], 'deleteGroup deletes correctly' ); + + # Clean up a bit. + $taxer->deleteGroup( $_ ) for ( $id0, $id100 ); +} + +####################################################################### +# +# getTaxRate +# +####################################################################### +{ + my $taxer = setupTestNumbers(); + my $id100 = $taxer->addGroup( 'Group100', 100 ); + my $id50_5 = $taxer->addGroup( 'Group50.5', 50.5 ); $taxer->update( { 'automaticViesApproval' => 1 } ); - my $book = WebGUI::Shop::AddressBook->create($session); - - # setup address in EU but not in residential country of merchant - my $beAddress = $book->addAddress({ - label => 'BE', - city => 'Antwerpen', - country => 'Belgium', - }); - - # setup address in residential country of merchant - my $nlAddress = $book->addAddress({ - label => 'NL', - city => 'Delft', - country => 'Netherlands', - }); - - # setup address outside EU - my $usAddress = $book->addAddress({ - label => 'outside eu', - city => 'New Amsterdam', - country => 'US', - }); eval { $taxer->getTaxRate(); }; - $e = Exception::Class->caught(); + my $e = Exception::Class->caught(); isa_ok($e, 'WebGUI::Error::InvalidParam', 'getTaxRate: error handling for not sending a sku'); is($e->error, 'Must pass in a WebGUI::Asset::Sku object', 'getTaxRate: error handling for not sending a sku'); @@ -323,27 +418,31 @@ SKIP: { $sku->setTaxConfiguration( 'WebGUI::Shop::TaxDriver::EU', { taxGroup => $id100 } ); is( $taxer->getTaxRate( $sku ), 100, 'getTaxRate returns tax group set by sku when no address is given'); - # Address outside EU - is( $taxer->getTaxRate( $sku, $usAddress ), 0, 'getTaxRate: shipping addresses outside EU are tax exempt' ); - - # Addresses inside EU - is( $taxer->getTaxRate( $sku, $beAddress ), 100, 'getTaxRate: shipping addresses inside EU w/o VAT number pay tax' ); - is( $taxer->getTaxRate( $sku, $nlAddress ), 100, 'getTaxRate: shipping addresses in country of merchant w/o VAT number pay tax' ); - - # Add VAT numbers - $taxer->addVATNumber( $testVAT_NL, $taxUser, 1); - $taxer->addVATNumber( $testVAT_BE, $taxUser, 1); - + # Addresses inside EU with VAT number is( $taxer->getTaxRate( $sku, $beAddress ), 0, 'getTaxRate: shipping addresses inside EU but other country than merchant w/ VAT number are tax exempt.' ); is( $taxer->getTaxRate( $sku, $nlAddress ), 100, 'getTaxRate: shipping addresses in country of merchant w/ VAT number pay tax' ); - ####################################################################### - # - # appendCartItemVars - # - ####################################################################### + $taxer->deleteVATNumber( $testVAT_NL, $taxUser ); + $taxer->deleteVATNumber( $testVAT_BE, $taxUser ); + + # Addresses inside EU without VAT number + is( $taxer->getTaxRate( $sku, $beAddress ), 100, 'getTaxRate: shipping addresses inside EU w/o VAT number pay tax' ); + is( $taxer->getTaxRate( $sku, $nlAddress ), 100, 'getTaxRate: shipping addresses in country of merchant w/o VAT number pay tax' ); + + # Address outside EU + is( $taxer->getTaxRate( $sku, $usAddress ), 0, 'getTaxRate: shipping addresses outside EU are tax exempt' ); + +} + +####################################################################### +# +# appendCartItemVars +# +####################################################################### +{ + my $taxer = setupTestNumbers(); eval { $taxer->appendCartItemVars }; my $e = Exception::Class->caught(); @@ -399,12 +498,16 @@ SKIP: { taxAmount => '0.00', must => 'be kept', }, 'appendCartItemVars returns correct data for address outside EU.' ); +} - ####################################################################### - # - # getTransactionTaxData - # - ####################################################################### +####################################################################### +# +# getTransactionTaxData +# +####################################################################### +{ + my $taxer = setupTestNumbers(); + $taxer->update( { 'automaticViesApproval' => 1 } ); my $details = $taxer->getTransactionTaxData( $sku, $usAddress ); cmp_deeply( $details, { @@ -433,35 +536,25 @@ SKIP: { className => 'WebGUI::Shop::TaxDriver::EU', useVATNumber => 0, }, 'getTransactionTaxData returns correct hashref for addresses in EU w/o VAT number' ); - - ####################################################################### - # - # deleteGroup - # - ####################################################################### - - eval { $taxer->deleteGroup }; - $e = Exception::Class->caught(); - isa_ok( $e, 'WebGUI::Error::InvalidParam', 'addGroup requires a group id' ); - is( $e, 'A group id is required', 'addGroup returns correct message for missing group id' ); - - $taxer->deleteGroup( $id50_5 ); - - $taxGroups = $taxer->get( 'taxGroups' ); - cmp_bag( $taxGroups, [ - { - name => 'Group0', - rate => 0, - id => $id0, - }, - { - name => 'Group100', - rate => 100, - id => $id100, - }, - ], 'deleteGroup deletes correctly' ); } + +} #SKIP BLOCK + +#---------------------------------------------------------------------------- +sub setupTestNumbers { + my $taxer = WebGUI::Shop::TaxDriver::EU->new($session); + + $session->db->write('delete from taxDriver where className=?', [ 'WebGUI::Shop::TaxDriver::EU' ]); + $session->db->write('delete from tax_eu_vatNumbers'); + + $taxer->addVATNumber( $testVAT_NL, $taxUser, 1); + $taxer->addVATNumber( $testVAT_BE, $taxUser, 1); + + return $taxer; +} + + #---------------------------------------------------------------------------- # Cleanup END {