[freeside-commits] branch FREESIDE_4_BRANCH updated. af5d7f1ca409f3679a76740bb5f29016963f99d3
Mitch Jackson
mitch at freeside.biz
Mon May 27 14:57:31 PDT 2019
The branch, FREESIDE_4_BRANCH has been updated
via af5d7f1ca409f3679a76740bb5f29016963f99d3 (commit)
via 57acf63e75062d7a598d2ba4042642f8287bf836 (commit)
via d9b63c1b63dcfd950f59caa04179d4eaacc6af40 (commit)
via f854721eb897e77cf6ec1f39d1073b493a0bd81c (commit)
via 1b832fbb0757e4f694528a0685a6875ab2abc50a (commit)
from 3bfedb7199c3de10c0f2d9a0c7de675fed63c4d9 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit af5d7f1ca409f3679a76740bb5f29016963f99d3
Author: Mitch Jackson <mitch at freeside.biz>
Date: Mon May 27 17:32:29 2019 -0400
RT# 83122 Fix update tool to properly display caught error
diff --git a/FS/bin/freeside-wa-tax-table-update b/FS/bin/freeside-wa-tax-table-update
index ad14687c9..b197ac845 100755
--- a/FS/bin/freeside-wa-tax-table-update
+++ b/FS/bin/freeside-wa-tax-table-update
@@ -106,8 +106,8 @@ $log->info('Begin wa_tax_rate_update');
};
if ( $@ ) {
- $log->error( "Error: $@" );
warn "Error: $@\n";
+ $log->error( "Error: $@" );
} else {
$log->info( 'Finished wa_tax_rate_update' );
warn "Finished wa_tax_rate_update\n";
commit 57acf63e75062d7a598d2ba4042642f8287bf836
Author: Mitch Jackson <mitch at freeside.biz>
Date: Mon May 27 17:26:06 2019 -0400
RT# 83122 Do not auto-repair wa state sales tax rows
Dupe rows may actually be dupes, or they may be
manually created taxes for other purposes!
Pulled out auto-repair code, because it could be
harming user's manually entered tax tables.
When dupes are detected, wa sales taxes will not
auto update, and instead generate error and log
messages pointing user towardst the
freeside-wa-tax-table-resolve CLI tool
diff --git a/FS/FS/Cron/tax_rate_update.pm b/FS/FS/Cron/tax_rate_update.pm
index 4383bc501..5111ef4d0 100755
--- a/FS/FS/Cron/tax_rate_update.pm
+++ b/FS/FS/Cron/tax_rate_update.pm
@@ -294,10 +294,13 @@ 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();
+ unless ( wa_sales_update_tax_table_sanity_check() ) {
+ log_error_and_die(
+ 'Duplicate district rows exist in the Washington state sales tax table. '.
+ 'These must be resolved before updating the tax tables. '.
+ 'See "freeside-wa-tax-table-resolve --check" to repair the tax tables. '
+ );
+ }
$args->{temp_dir} ||= tempdir();
@@ -356,7 +359,7 @@ sub wa_sales_update_cust_main_county {
cust_main_county => {
source => 'wa_sales',
district => { op => '!=', value => undef },
- tax_class => $taxclass,
+ taxclass => $taxclass,
}
)
) {
@@ -381,19 +384,30 @@ sub wa_sales_update_cust_main_county {
$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;
+ # # 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;
+ # }
+
+ # If there are duplicate rows, it may be unsafe to auto-resolve them
+ if ( %cust_main_county_dupe ) {
+ warn "Unable to continue!";
+ log_error_and_die( sprintf(
+ 'Tax district duplicate rows detected(%s) - '.
+ 'WA Sales tax tables cannot be updated without resolving duplicates - '.
+ 'Please use tool freeside-wa-tax-table-resolve for tax table repair',
+ join( ',', keys %cust_main_county_dupe )
+ ));
}
for my $district ( @{ $args->{tax_districts} } ) {
commit d9b63c1b63dcfd950f59caa04179d4eaacc6af40
Author: Mitch Jackson <mitch at freeside.biz>
Date: Mon May 27 17:20:27 2019 -0400
RT# 83320 Fix UI bug managing taxes
No longer incorrectly carrying the source and district
columns from other tax rows into new tax rows
diff --git a/httemplate/edit/process/cust_main_county-add.cgi b/httemplate/edit/process/cust_main_county-add.cgi
index fcc138f49..fcd6be48f 100755
--- a/httemplate/edit/process/cust_main_county-add.cgi
+++ b/httemplate/edit/process/cust_main_county-add.cgi
@@ -27,26 +27,59 @@ my @expansion = split /[\n\r]{1,2}/, $cgi->param('expansion');
$1;
} @expansion;
-foreach ( @expansion ) {
- my(%hash)=$cust_main_county->hash;
- my($new)=new FS::cust_main_county \%hash;
- $new->setfield('taxnum','');
- $new->setfield('taxclass', '');
- if ( $cgi->param('what') eq 'state' ) { #??
- $new->setfield('state',$_);
- $new->setfield('county', '');
- $new->setfield('city', '');
- } elsif ( $cgi->param('what') eq 'county' ) {
- $new->setfield('county',$_);
- $new->setfield('city', '');
- } elsif ( $cgi->param('what') eq 'city' ) {
- #uppercase cities in the US to try and agree with USPS validation
- $new->setfield('city', $new->country eq 'US' ? uc($_) : $_ );
- } else { #???
- die 'unknown what '. $cgi->param('what');
+my $what = $cgi->param('what');
+foreach my $new_tax_area ( @expansion ) {
+
+ # Clone specific tax columns from original tax row
+ #
+ # UI Note: Preserving original behavior, of cloning
+ # tax amounts into new tax record, against better
+ # judgement. If the new city/county/state has a
+ # different tax value than the one being populated
+ # (rather likely?) now the user must remember to
+ # revisit each newly created tax row, and correct
+ # the possibly incorrect tax values that were populated.
+ # Values would be easier to identify and correct if
+ # they were initially populated with 0% tax rates
+ # District Note: The 'district' column is NOT cloned
+ # to the new tax row. Manually entered taxes
+ # are not be divided into road maintenance districts
+ # like Washington state sales taxes
+ my $new = FS::cust_main_county->new({
+ map { $_ => $cust_main_county->getfield($_) }
+ qw/
+ charge_prediscount
+ exempt_amount
+ exempt_amount_currency
+ recurtax
+ setuptax
+ tax
+ taxname
+ /
+ });
+
+ # Clone additional location columns, based on the $what value
+ my %clone_cols_for = (
+ state => [qw/country /],
+ county => [qw/country state/],
+ city => [qw/country state county/],
+ );
+
+ die "unknown what: $what"
+ unless grep { $_ eq $what } keys %clone_cols_for;
+
+ $new->setfield( $_ => $cust_main_county->getfield($_) )
+ for @{ $clone_cols_for{ $cgi->param('what') } };
+
+ # In the US, store cities upper case for USPS validation
+ $new_tax_area = uc($new_tax_area)
+ if $what eq 'city'
+ && $new->country eq 'US';
+
+ $new->setfield( $what, $new_tax_area );
+ if ( my $error = $new->insert ) {
+ die $error;
}
- my $error = $new->insert;
- die $error if $error;
}
</%init>
commit f854721eb897e77cf6ec1f39d1073b493a0bd81c
Author: Mitch Jackson <mitch at freeside.biz>
Date: Mon May 27 17:12:47 2019 -0400
RT# 82906 Fix typos in script help text
diff --git a/FS/bin/freeside-issue-credit-for-taxnums b/FS/bin/freeside-issue-credit-for-taxnums
index 8b7a12252..016c14ed6 100755
--- a/FS/bin/freeside-issue-credit-for-taxnums
+++ b/FS/bin/freeside-issue-credit-for-taxnums
@@ -293,18 +293,18 @@ sub validate_opts {
error_and_help( '--csv_dir is required' )
unless $csv_dir;
- error_and_help( '--start_date is required' )
+ error_and_help( '--start-date is required' )
unless $start_date;
error_and_help( '--end-date is required' )
unless $end_date;
error_and_help( '--taxnums is required' )
unless @taxnums;
- error_and_help( '--credit-reasonnum is required with --apply-credits' )
+ error_and_help( '--credit-reasonnum is required with --insert-credits' )
if $insert_credits && !$credit_reasonnum;
- error_and_help( '--credit-addlinfo is required with --apply-credits' )
+ error_and_help( '--credit-addlinfo is required with --insert-credits' )
if $insert_credits && !$credit_addlinfo;
- error_and_help( "csv dir ($csv_dir) is not a writable directoryu" )
+ error_and_help( "csv dir ($csv_dir) is not a writable directory" )
unless -d $csv_dir && -r $csv_dir;
error_and_help( "start_date($start_date) is not a valid date string")
commit 1b832fbb0757e4f694528a0685a6875ab2abc50a
Author: Mitch Jackson <mitch at freeside.biz>
Date: Sun May 26 15:29:16 2019 -0400
RT# 83402 CLI tool to repair wa state tax tables
diff --git a/FS/FS/Cron/tax_rate_update.pm b/FS/FS/Cron/tax_rate_update.pm
index bb9d4d13d..4383bc501 100755
--- a/FS/FS/Cron/tax_rate_update.pm
+++ b/FS/FS/Cron/tax_rate_update.pm
@@ -294,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 );
@@ -635,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;
@@ -655,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" );
}
diff --git a/FS/FS/cust_main_county.pm b/FS/FS/cust_main_county.pm
index 2bd7342ca..958233440 100644
--- a/FS/FS/cust_main_county.pm
+++ b/FS/FS/cust_main_county.pm
@@ -562,6 +562,40 @@ sub taxline {
return $tax_item;
}
+=head1 find_wa_tax_dupes
+
+Return a list of cust_main_county Record objects that are detected
+as duplicate washington state sales tax rows (source=wa_state)
+within their respective tax classes
+
+=cut
+
+sub find_wa_tax_dupes {
+ my %cust_main_county;
+ my @dupes;
+
+ for my $row ( qsearch( cust_main_county => { source => 'wa_sales' } ) ) {
+ my $taxclass = $row->taxclass || 'none';
+ $cust_main_county{$taxclass} ||= {};
+
+ my $district = $row->district || 'none';
+ $cust_main_county{$taxclass}->{$district} ||= [];
+
+ push @{ $cust_main_county{$taxclass}->{$district} }, $row;
+ }
+
+ for my $taxclass ( keys %cust_main_county ) {
+ for my $district ( keys %{ $cust_main_county{$taxclass} } ) {
+ my $tax_rows = $cust_main_county{$taxclass}->{$district};
+ if ( scalar @$tax_rows > 1 ) {
+ push @dupes, @$tax_rows;
+ }
+ }
+ }
+
+ @dupes;
+}
+
=back
=head1 SUBROUTINES
diff --git a/FS/bin/freeside-wa-tax-table-resolve b/FS/bin/freeside-wa-tax-table-resolve
new file mode 100755
index 000000000..fa6db3e39
--- /dev/null
+++ b/FS/bin/freeside-wa-tax-table-resolve
@@ -0,0 +1,304 @@
+#!/usr/bin/env perl
+use v5.10;
+use strict;
+use warnings;
+
+our $VERSION = '1.0';
+
+use Data::Dumper;
+use FS::cust_main_county;
+use FS::Log;
+use FS::Record qw( qsearch qsearchs );
+use FS::UID qw( adminsuidsetup );
+use Getopt::Long;
+use Pod::Usage;
+
+# Begin transaction
+local $FS::UID::AutoCommit = 0;
+
+my(
+ $dbh,
+ $freeside_user,
+ $opt_check,
+ @opt_merge,
+ @opt_set_source_null,
+);
+
+GetOptions(
+ 'check' => \$opt_check,
+ 'merge=s' => \@opt_merge,
+ 'set-source-null=s' => \@opt_set_source_null,
+);
+ at opt_merge = split(',',join(',', at opt_merge));
+ at opt_set_source_null = split(',',join(',', at opt_set_source_null));
+
+
+# say Dumper({
+# check => $opt_check,
+# merge => \@opt_merge,
+# set_source_numm => \@opt_set_source_null,
+# });
+
+validate_opts();
+
+$dbh = adminsuidsetup( $freeside_user )
+ or die "Bad username: $freeside_user\n";
+
+my $log = FS::Log->new('freeside-wa-tax-table-resolve');
+
+if ( $opt_check ) {
+ check();
+} elsif ( @opt_merge ) {
+ merge();
+} elsif ( @opt_set_source_null ) {
+ set_source_null();
+} else {
+ error_and_help('No options selected');
+}
+
+# Commit transaction
+$dbh->commit;
+local $FS::UID::AutoCommit = 1;
+
+exit;
+
+
+sub set_source_null {
+ my @cust_main_county;
+ for my $taxnum ( @opt_set_source_null ) {
+ my $row = qsearchs( cust_main_county => { taxnum => $taxnum } );
+ if ( $row ) {
+ push @cust_main_county, $row;
+ } else {
+ error_and_help("Invalid taxnum specified: $taxnum");
+ }
+ }
+
+ say "=== Specified tax rows ===";
+ print_taxnum($_) for @cust_main_county;
+
+ confirm_to_continue("
+
+ The source column will be set to NULL for each of the
+ tax rows listed. The tax row will no longer be managed
+ by the washington state sales tax table update utilities.
+
+ The listed taxes should be manually created taxes, that
+ were never intended to be managed by the auto updater.
+
+ ");
+
+ for my $row ( @cust_main_county ) {
+
+ $row->setfield( source => undef );
+ my $error = $row->replace;
+
+ if ( $error ) {
+ $dbh->rollback;
+
+ my $message = sprintf 'Error setting source=null taxnum %s: %s',
+ $row->taxnum, $error;
+
+ $log->error( $message );
+ say $message;
+
+ return;
+ }
+
+ my $message = sprintf 'Source column set to null for taxnum %s',
+ $row->taxnum;
+
+ $log->warn( $message );
+ say $message;
+ }
+}
+
+sub merge {
+ my $source = qsearchs( cust_main_county => { taxnum => $opt_merge[0] });
+ my $target = qsearchs( cust_main_county => { taxnum => $opt_merge[1] });
+
+ error_and_help("Invalid source taxnum: $opt_merge[0]")
+ unless $source;
+ error_and_help("Invalid target taxnum: $opt_merge[1]")
+ unless $target;
+
+ local $| = 1; # disable output buffering
+
+ say '==== source row ====';
+ print_taxnum( $source );
+
+ say '==== target row ====';
+ print_taxnum( $target );
+
+ confirm_to_continue("
+
+ The source tax will be merged into the target tax.
+ All references to the source tax on customer invoices
+ will be replaced with references to the target tax.
+ The source tax will be removed from the tax tables.
+
+ ");
+
+ local $@;
+ eval { $source->_merge_into( $target, { identical_record_check => 0 } ) };
+ if ( $@ ) {
+ $dbh->rollback;
+
+ my $message = sprintf 'Failed to merge wa sales tax %s into %s: %s',
+ $source->taxnum, $target->taxnum, $@;
+
+ say $message;
+ $log->error( $message );
+
+ } else {
+ my $message = sprintf 'Merged wa sales tax %s into %s - success',
+ $source->taxnum, $target->taxnum;
+
+ say $message;
+ $log->warn( $message );
+ }
+}
+
+sub validate_opts {
+
+ $freeside_user = shift @ARGV
+ or error_and_help('freeside_user parameter required');
+
+ if ( @opt_merge ) {
+ error_and_help(( '--merge requires a comma separated list of two taxnums'))
+ unless scalar(@opt_merge) == 2
+ && $opt_merge[0] =~ /^\d+$/
+ && $opt_merge[1] =~ /^\d+$/;
+ }
+
+ for my $taxnum ( @opt_set_source_null ) {
+ if ( $taxnum =~ /\D/ ) {
+ error_and_help( "Invalid taxnum ($taxnum)" );
+ }
+ }
+}
+
+sub check {
+ my @dupes = FS::cust_main_county->find_wa_tax_dupes;
+
+ unless ( @dupes ) {
+ say 'No duplicate tax rows detected for WA sales tax districts';
+ return;
+ }
+
+ say sprintf '=== Detected %s duplicate tax rows ===', scalar @dupes;
+
+ print_taxnum($_) for @dupes;
+
+ $log->error(
+ sprintf 'Detected %s duplicate wa sales tax rows: %s',
+ scalar( @dupes ),
+ join( ',', map{ $_->taxnum } @dupes )
+ );
+
+}
+
+sub print_taxnum {
+ my $taxnum = shift;
+ die unless ref $taxnum;
+
+ say 'taxnum: '.$taxnum->taxnum;
+ say join "\n" => (
+ map { sprintf(' %s:%s', $_, $taxnum->$_ ) }
+ qw/district city county state tax taxname taxclass source/
+ );
+ print "\n";
+}
+
+sub confirm_to_continue {
+ say shift;
+ print "Confirm: [y/N]: ";
+ my $yn = <STDIN>;
+ chomp $yn;
+ if ( lc $yn ne 'y' ) {
+ say "\nAborted\n";
+ exit;
+ }
+}
+
+sub error_and_help {
+ pod2usage({
+ -message => sprintf( "\n\nError:\n\t%s\n\n", shift ),
+ -exitval => 2,
+ verbose => 1,
+ });
+ exit;
+}
+
+__END__
+
+=head1 name
+
+freeside-wa-tax-table-resolve
+
+=head1 SYNOPSIS
+
+freeside-issue-credit-for-taxnums --help
+freeside-issue-credit-for-taxnums --check [freeside_user]
+freeside-issue-credit-for-taxnums --merge 123,234 [freeside_user]
+freeside-issue-credit-for-taxnums --set-source-null 1337,6553 [freeside_user]
+
+=head1 OPTIONS
+
+=over 4
+
+=item B<--help>
+
+Display help and exit
+
+=item B<--check>
+
+Display info on any taxnums considered blocking duplicates
+
+=item B<--merge> [source-taxnum],[target-taxnum]
+
+Update all records referring to [source-taxnum], so they now
+refer to [target-taxnum]. [source-taxnum] is deleted.
+
+Used to merge duplicate taxnums
+
+=item B<--set-source-null> [taxnum],[taxnum],...
+
+Update all records for the given taxnums, by setting the
+I<source> column to NULL.
+
+Used for manually entered tax entries, incorrectly labelled
+as created and managed for Washington State Sales Taxes
+
+=back
+
+=head1 DESCRIPTION
+
+Tool to resolve tax table issues for customer using Washington state
+sales tax districts.
+
+If Freeside detects duplicate rows within the wa sales tax tables,
+tax table updates are blocked, and a log message directs the
+sysadmin to this tool.
+
+Duplicate rows may be manually entered taxes, not related
+to WA sales tax. Or duplicate rows may have been manually entered
+into freeside for other tax purposes.
+
+Use --check to display which tax entries were detected as dupes.
+
+For each tax entry, decide if it is a duplicate wa sales tax entry,
+or some other manually entered tax.
+
+if the row is a duplicate, merge the duplicates with the --merge
+option of this script
+
+If the row is a manually entered tax, not for WA state sales taxes,
+keep the tax but remove the flag incorrectly labeling it as WA state
+sales taxes with the --set-source-null option of this script
+
+Once --check no longer returns problematic tax entries, the
+wa state tax tables will be able to complete their automatic
+tax rate updates
+
+=cut
-----------------------------------------------------------------------
Summary of changes:
FS/FS/Cron/tax_rate_update.pm | 68 +++--
FS/FS/cust_main_county.pm | 34 +++
FS/bin/freeside-issue-credit-for-taxnums | 8 +-
FS/bin/freeside-wa-tax-table-resolve | 304 +++++++++++++++++++++++
FS/bin/freeside-wa-tax-table-update | 2 +-
httemplate/edit/process/cust_main_county-add.cgi | 71 ++++--
6 files changed, 449 insertions(+), 38 deletions(-)
create mode 100755 FS/bin/freeside-wa-tax-table-resolve
More information about the freeside-commits
mailing list