X-Git-Url: http://git.freeside.biz/gitweb/?p=freeside.git;a=blobdiff_plain;f=FS%2FFS%2FCron%2Ftax_rate_update.pm;h=4383bc501f8b31bbea9d1eb7c0cc30da651920e7;hp=b0745e409c1cf1a692d1f5f596e0216abd644c7d;hb=d397c0135075feca088abf09e801ceb18d425f10;hpb=83a6052bc16ed5cff28e32613f20dc4b1156bac6 diff --git a/FS/FS/Cron/tax_rate_update.pm b/FS/FS/Cron/tax_rate_update.pm index b0745e409..4383bc501 100755 --- a/FS/FS/Cron/tax_rate_update.pm +++ b/FS/FS/Cron/tax_rate_update.pm @@ -117,10 +117,14 @@ sub wa_sales { =head2 wa_sales_log_customer_without_tax_district -For any active customers with cust_location records in WA state, -if a cust_location record has no tax district, find the correct -district using WA DOR API, or if not possible, generate an error -message into system log so address can be corrected +For any cust_location records +* In WA state +* Attached to non cancelled packages +* With no tax district + +Classify the tax district for the record using the WA State Dept of +Revenue API. If this fails, generate an error into system log so +address can be corrected =cut @@ -140,16 +144,35 @@ sub wa_sales_log_customer_without_tax_district { cust_location.state, cust_location.zip ', - hashref => { - state => 'WA', - district => undef, - }, - addl_from => 'LEFT JOIN cust_main USING (custnum)', - extra_sql => sprintf 'AND ( %s ) ', FS::cust_main->active_sql, + addl_from => ' + LEFT JOIN cust_main USING (custnum) + LEFT JOIN cust_pkg ON cust_location.locationnum = cust_pkg.locationnum + ', + extra_sql => sprintf(q{ + WHERE cust_location.state = 'WA' + AND ( + cust_location.district IS NULL + or cust_location.district = '' + ) + AND cust_pkg.pkgnum IS NOT NULL + AND ( + cust_pkg.cancel > %s + OR cust_pkg.cancel IS NULL + ) + }, time() + ), ); for my $cust_location ( qsearch( \%qsearch_cust_location )) { local $@; + log_info_and_warn( + sprintf + 'Attempting to classify district for cust_location ' . + 'locationnum(%s) address(%s)', + $cust_location->locationnum, + $cust_location->address1, + ); + eval { FS::geocode_Mixin::process_district_update( 'FS::cust_location', @@ -158,16 +181,13 @@ sub wa_sales_log_customer_without_tax_district { }; if ( $@ ) { + # Error indicates a crash, not an error looking up district + # process_district_udpate will generate log messages for those errors log_error_and_warn( - sprintf "Failed to classify district for cust_location(%s): %s", + sprintf "Classify district error for cust_location(%s): %s", $cust_location->locationnum, $@ ); - } else { - log_info_and_warn( - sprintf "Classified district for cust_location(%s)", - $cust_location->locationnum - ); } sleep 1; # Be polite to WA DOR API @@ -274,6 +294,11 @@ sub wa_sales_update_tax_table { ) ); + # The checks themselves will fully log details about the problem, + # so simple error message is sufficient here + log_error_and_die('abort tax table update, sanity checks failed') + unless wa_sales_update_tax_table_sanity_check(); + $args->{temp_dir} ||= tempdir(); $args->{filename} ||= wa_sales_fetch_xlsx_file( $args ); @@ -289,8 +314,6 @@ sub wa_sales_update_tax_table { Create or update the L records with new data - - =cut sub wa_sales_update_cust_main_county { @@ -314,17 +337,64 @@ sub wa_sales_update_cust_main_county { for my $taxclass ( FS::part_pkg_taxclass->taxclass_names ) { $taxclass ||= undef; # trap empty string when taxclasses are disabled - my %cust_main_county = - map { $_->district => $_ } + # Dupe detection/remediation: + # + # Previous code for washington state tax district was creating + # duplicate entries for tax districts. This could lead to customers + # being double-taxed + # + # The following code detects and eliminates duplicates that + # were created by wa_sales district code (source=wa_sales) + # before updating the tax table with the newly downloaded + # data + + my %cust_main_county; + my %cust_main_county_dupe; + + for my $row ( qsearch( cust_main_county => { - district => { op => '!=', value => undef }, - state => 'WA', - country => 'US', - source => 'wa_sales', - taxclass => $taxclass, + source => 'wa_sales', + district => { op => '!=', value => undef }, + tax_class => $taxclass, } - ); + ) + ) { + my $district = $row->district; + + # Row belongs to a known dupe group of districts + if ( $cust_main_county_dupe{$district} ) { + push @{ $cust_main_county_dupe{$district} }, $row; + next; + } + + # Row is the first seen dupe for the given district + if ( $cust_main_county{$district} ) { + $cust_main_county_dupe{$district} = [ + delete $cust_main_county{$district}, + $row + ]; + next; + } + + # Row is the first seen with this district + $cust_main_county{$district} = $row; + } + + # Merge any dupes, place resulting non-dupe row in %cust_main_county + # Merge, even if one of the dupes has a $0 tax, or some other + # variation on tax row data. Data for this row will get corrected + # during the following tax import + for my $dupe_district_aref ( values %cust_main_county_dupe ) { + my $row_to_keep = shift @$dupe_district_aref; + while ( my $row_to_merge = shift @$dupe_district_aref ) { + $row_to_merge->_merge_into( + $row_to_keep, + { identical_record_check => 0 }, + ); + } + $cust_main_county{$row_to_keep->district} = $row_to_keep; + } for my $district ( @{ $args->{tax_districts} } ) { if ( my $row = $cust_main_county{ $district->{district} } ) { @@ -335,12 +405,19 @@ sub wa_sales_update_cust_main_county { # creating an entire separate set of tax rows with # the new taxname, update the taxname on existing records - if ( - $row->tax == ( $district->{tax_combined} * 100 ) - && $row->taxname eq $args->{taxname} - ) { - $same_count++; - next; + { + # Supress warning on taxname comparison, when taxname is undef + no warnings 'uninitialized'; + + if ( + $row->tax == ( $district->{tax_combined} * 100 ) + && $row->taxname eq $args->{taxname} + && uc $row->county eq uc $district->{county} + && uc $row->city eq uc $district->{city} + ) { + $same_count++; + next; + } } $row->city( uc $district->{city} ); @@ -563,6 +640,26 @@ sub wa_sales_fetch_xlsx_file { } +=head2 wa_sales_update_tax_table_sanity_check + +There should be no duplicate tax table entries in the tax table, +with the same district value, within a tax class, where source=wa_sales. + +If there are, custome taxes may have been user-entered in the +freeside UI, and incorrectly labelled as source=wa_sales. Or, the +dupe record may have been created by issues with older wa_sales code. + +If these dupes exist, the sysadmin must solve the problem by hand +with the freeeside-wa-tax-table-resolve script + +Returns 1 unless problem sales tax entries are detected + +=cut + +sub wa_sales_update_tax_table_sanity_check { + FS::cust_main_county->find_wa_tax_dupes ? 0 : 1; +} + sub log { state $log = FS::Log->new('tax_rate_update'); $log; @@ -583,6 +680,7 @@ sub log_warn_and_warn { sub log_error_and_die { my $log_message = shift; &log()->error( $log_message ); + warn( "$log_message\n" ); die( "$log_message\n" ); }