From 48bade3f01a672f235d61a29ad0d0b792fc80eab Mon Sep 17 00:00:00 2001 From: ivan Date: Sun, 21 Dec 2008 21:33:28 +0000 Subject: [PATCH] unique checking for svc_phone like svc_acct, closes: RT#4204 (also a few lines of the new per-agent config snuck in Conf.pm from RT#3989) --- FS/FS/Conf.pm | 13 +++++++++- FS/FS/Record.pm | 44 ++++++++++++++++++++++++++++++++ FS/FS/Setup.pm | 4 +++ FS/FS/msgcat.pm | 34 ++++++++++++++++++++++++- FS/FS/svc_Common.pm | 24 ++++++++++++++++++ FS/FS/svc_phone.pm | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 6 files changed, 188 insertions(+), 4 deletions(-) diff --git a/FS/FS/Conf.pm b/FS/FS/Conf.pm index eba04d426..5d5616938 100644 --- a/FS/FS/Conf.pm +++ b/FS/FS/Conf.pm @@ -97,7 +97,7 @@ sub _config { $hashref->{agentnum} = $agentnum; local $FS::Record::conf = undef; # XXX evil hack prevents recursion my $cv = FS::Record::qsearchs('conf', $hashref); - if (!$cv && defined($agentnum)) { + if (!$cv && defined($agentnum) && $agentnum) { $hashref->{agentnum} = ''; $cv = FS::Record::qsearchs('conf', $hashref); } @@ -763,6 +763,7 @@ worry that config_items is freeside-specific and icky. 'section' => 'required', 'description' => 'Return address on email invoices', 'type' => 'text', + 'per_agent' => 1, }, { @@ -1701,6 +1702,14 @@ worry that config_items is freeside-specific and icky. }, { + 'key' => 'global_unique-phonenum', + 'section' => '', + 'description' => 'Global phone number uniqueness control: none (usual setting - check countrycode+phonenumun uniqueness per exports), or countrycode+phonenum (all countrycode+phonenum pairs are globally unique, regardless of exports). disabled turns off duplicate checking completely and is STRONGLY NOT RECOMMENDED unless you REALLY need to turn this off.', + 'type' => 'select', + 'select_enum' => [ 'none', 'countrycode+phonenum', 'disabled' ], + }, + + { 'key' => 'svc_external-skip_manual', 'section' => 'UI', 'description' => 'When provisioning svc_external services, skip manual entry of id and title fields in the UI. Usually used in conjunction with an export that populates these fields (i.e. artera_turbo).', @@ -1799,6 +1808,7 @@ worry that config_items is freeside-specific and icky. 'section' => 'required', 'description' => 'Your company name', 'type' => 'text', + 'per_agent' => 1, }, { @@ -1806,6 +1816,7 @@ worry that config_items is freeside-specific and icky. 'section' => 'required', 'description' => 'Your company address', 'type' => 'textarea', + 'per_agent' => 1, }, { diff --git a/FS/FS/Record.pm b/FS/FS/Record.pm index 1f0b140e2..acec9458d 100644 --- a/FS/FS/Record.pm +++ b/FS/FS/Record.pm @@ -766,6 +766,50 @@ sub select_for_update { } ); } +=item lock_table + +Locks this table with a database-driver specific lock method. This is used +as a mutex in order to do a duplicate search. + +For PostgreSQL, does "LOCK TABLE tablename IN SHARE ROW EXCLUSIVE MODE". + +For MySQL, does a SELECT FOR UPDATE on the duplicate_lock table. + +Errors are fatal; no useful return value. + +Note: To use this method for new tables other than svc_acct and svc_phone, +edit freeside-upgrade and add those tables to the duplicate_lock list. + +=cut + +sub lock_table { + my $self = shift; + my $table = $self->table; + + warn "$me locking $table table\n" if $DEBUG; + + if ( driver_name =~ /^Pg/i ) { + + dbh->do("LOCK TABLE $table IN SHARE ROW EXCLUSIVE MODE") + or die dbh->errstr; + + } elsif ( driver_name =~ /^mysql/i ) { + + dbh->do("SELECT * FROM duplicate_lock + WHERE lockname = '$table' + FOR UPDATE" + ) or die dbh->errstr; + + } else { + + die "unknown database ". driver_name. "; don't know how to lock table"; + + } + + warn "$me acquired $table table lock\n" if $DEBUG; + +} + =item insert Inserts this record to the database. If there is an error, returns the error, diff --git a/FS/FS/Setup.pm b/FS/FS/Setup.pm index 2b392e54a..3c8e817d6 100644 --- a/FS/FS/Setup.pm +++ b/FS/FS/Setup.pm @@ -461,6 +461,10 @@ sub msgcat_messages { 'en_US' => 'Username in use', }, + 'phonenum_in_use' => { + 'en_US' => 'Phone number in use', + }, + 'illegal_email_invoice_address' => { 'en_US' => 'Illegal email invoice address', }, diff --git a/FS/FS/msgcat.pm b/FS/FS/msgcat.pm index b512a813c..4f60109f3 100644 --- a/FS/FS/msgcat.pm +++ b/FS/FS/msgcat.pm @@ -4,7 +4,7 @@ use strict; use vars qw( @ISA ); use Exporter; use FS::UID; -use FS::Record; +use FS::Record qw( qsearchs ); @ISA = qw(FS::Record); @@ -117,6 +117,38 @@ sub check { $self->SUPER::check } + +sub _upgrade_data { #class method + my( $class, %opts) = @_; + + eval "use FS::Setup;"; + die $@ if $@; + + #"repopulate_msgcat", false laziness w/FS::Setup::populate_msgcat + + my %messages = FS::Setup::msgcat_messages(); + + foreach my $msgcode ( keys %messages ) { + foreach my $locale ( keys %{$messages{$msgcode}} ) { + my %msgcat = ( + 'msgcode' => $msgcode, + 'locale' => $locale, + #'msg' => $messages{$msgcode}{$locale}, + ); + my $msgcat = qsearchs('msgcat', \%msgcat); + next if $msgcat; + + $msgcat = new FS::msgcat( { + %msgcat, + 'msg' => $messages{$msgcode}{$locale}, + } ); + my $error = $msgcat->insert; + die $error if $error; + } + } + +} + =back =head1 BUGS diff --git a/FS/FS/svc_Common.pm b/FS/FS/svc_Common.pm index 32c1d90e8..2866bfea4 100644 --- a/FS/FS/svc_Common.pm +++ b/FS/FS/svc_Common.pm @@ -249,6 +249,7 @@ sub insert { my $error = $self->set_auto_inventory || $self->check + || $self->_check_duplicate || $self->SUPER::insert; if ( $error ) { $dbh->rollback if $oldAutoCommit; @@ -314,6 +315,10 @@ sub insert { ''; } +#fallbacks +sub _check_duplcate { ''; } +sub table_dupcheck_fields { (); } + =item delete [ , OPTION => VALUE ... ] Deletes this account from the database. If there is an error, returns the @@ -390,6 +395,25 @@ sub replace { return $error; } + #redundant, but so any duplicate fields are maniuplated as appropriate + # (svc_phone.phonenum) + my $error = $new->check; + if ( $error ) { + $dbh->rollback if $oldAutoCommit; + return $error; + } + + #if ( $old->username ne $new->username || $old->domsvc != $new->domsvc ) { + if ( grep { $old->$_ ne $new->$_ } $new->table_dupcheck_fields ) { + + $new->svcpart( $new->cust_svc->svcpart ) unless $new->svcpart; + $error = $new->_check_duplicate; + if ( $error ) { + $dbh->rollback if $oldAutoCommit; + return $error; + } + } + $error = $new->SUPER::replace($old); if ($error) { $dbh->rollback if $oldAutoCommit; diff --git a/FS/FS/svc_phone.pm b/FS/FS/svc_phone.pm index 5dcb39ef8..5cf3985c0 100644 --- a/FS/FS/svc_phone.pm +++ b/FS/FS/svc_phone.pm @@ -1,16 +1,22 @@ package FS::svc_phone; use strict; -use vars qw( @ISA @pw_set ); +use vars qw( @ISA @pw_set $conf ); use FS::Conf; -#use FS::Record qw( qsearch qsearchs ); +use FS::Record qw( qsearch qsearchs ); use FS::svc_Common; +use FS::part_svc; @ISA = qw( FS::svc_Common ); #avoid l 1 and o O 0 @pw_set = ( 'a'..'k', 'm','n', 'p-z', 'A'..'N', 'P'..'Z' , '2'..'9' ); +#ask FS::UID to run this stuff for us later +$FS::UID::callback{'FS::svc_acct'} = sub { + $conf = new FS::Conf; +}; + =head1 NAME FS::svc_phone - Object methods for svc_phone records @@ -102,6 +108,8 @@ sub table_info { sub table { 'svc_phone'; } +sub table_dupcheck_fields { ( 'countrycode', 'phonenum' ); } + =item search_sql STRING Class method which returns an SQL fragment to search for the given string. @@ -215,6 +223,67 @@ sub check { $self->SUPER::check; } +=item _check duplicate + +Internal method to check for duplicate phone numers. + +=cut + +#false laziness w/svc_acct.pm's _check_duplicate. +sub _check_duplicate { + my $self = shift; + + my $global_unique = $conf->config('global_unique-phonenum') || 'none'; + return '' if $global_unique eq 'disabled'; + + $self->lock_table; + + my @dup_ccphonenum = + grep { !$self->svcnum || $_->svcnum != $self->svcnum } + qsearch( 'svc_phone', { + 'countrycode' => $self->countrycode, + 'phonenum' => $self->phonenum, + }); + + return gettext('phonenum_in_use') + if $global_unique eq 'countrycode+phonenum' && @dup_ccphonenum; + + my $part_svc = qsearchs('part_svc', { 'svcpart' => $self->svcpart } ); + unless ( $part_svc ) { + return 'unknown svcpart '. $self->svcpart; + } + + if ( @dup_ccphonenum ) { + + my $exports = FS::part_export::export_info('svc_phone'); + my %conflict_ccphonenum_svcpart = ( $self->svcpart => 'SELF', ); + + foreach my $part_export ( $part_svc->part_export ) { + + #this will catch to the same exact export + my @svcparts = map { $_->svcpart } $part_export->export_svc; + + $conflict_ccphonenum_svcpart{$_} = $part_export->exportnum + foreach @svcparts; + + } + + foreach my $dup_ccphonenum ( @dup_ccphonenum ) { + my $dup_svcpart = $dup_ccphonenum->cust_svc->svcpart; + if ( exists($conflict_ccphonenum_svcpart{$dup_svcpart}) ) { + return "duplicate phone number ". + $self->countrycode. ' '. $self->phonenum. + ": conflicts with svcnum ". $dup_ccphonenum->svcnum. + " via exportnum ". $conflict_ccphonenum_svcpart{$dup_svcpart}; + } + } + + } + + return ''; + +} + =item check_pin Checks the supplied PIN against the PIN in the database. Returns true for a -- 2.11.0