From efa799391aeba085b65145428cab1cf1074d3f6d Mon Sep 17 00:00:00 2001 From: Ivan Kohler Date: Wed, 27 Feb 2013 01:50:04 -0800 Subject: [PATCH] refactor svc_acct and svc_broadband search into svc_Common, RT#21054 --- FS/FS/svc_Common.pm | 42 ++++++++++++++++---- FS/FS/svc_Tower_Mixin.pm | 6 +-- FS/FS/svc_acct.pm | 99 ++++++------------------------------------------ FS/FS/svc_broadband.pm | 97 +++++++---------------------------------------- 4 files changed, 60 insertions(+), 184 deletions(-) diff --git a/FS/FS/svc_Common.pm b/FS/FS/svc_Common.pm index f7f0b4094..ff465bf17 100644 --- a/FS/FS/svc_Common.pm +++ b/FS/FS/svc_Common.pm @@ -1315,8 +1315,7 @@ Parameters: =cut -# based on FS::svc_acct::search, both that and svc_broadband::search should -# eventually use this instead +# svc_broadband::search should eventually use this instead sub search { my ($class, $params) = @_; @@ -1329,6 +1328,8 @@ sub search { my @where = (); + $class->_search_svc($params, \@from, \@where) if $class->can('_search_svc'); + # # domain # if ( $params->{'domain'} ) { # my $svc_domain = qsearchs('svc_domain', { 'domain'=>$params->{'domain'} } ); @@ -1377,15 +1378,40 @@ sub search { } #pkgpart - if ( $params->{'pkgpart'} && scalar(@{ $params->{'pkgpart'} }) ) { - my @pkgpart = grep /^(\d+)$/, @{ $params->{'pkgpart'} }; - push @where, 'cust_pkg.pkgpart IN ('. join(',', @pkgpart ). ')'; + ##pkgpart, now properly untainted, can be arrayref + #for my $pkgpart ( $params->{'pkgpart'} ) { + # if ( ref $pkgpart ) { + # my $where = join(',', map { /^(\d+)$/ ? $1 : () } @$pkgpart ); + # push @where, "cust_pkg.pkgpart IN ($where)" if $where; + # } + # elsif ( $pkgpart =~ /^(\d+)$/ ) { + # push @where, "cust_pkg.pkgpart = $1"; + # } + #} + if ( $params->{'pkgpart'} ) { + my @pkgpart = ref( $params->{'pkgpart'} ) + ? @{ $params->{'pkgpart'} } + : $params->{'pkgpart'} + ? ( $params->{'pkgpart'} ) + : (); + @pkgpart = grep /^(\d+)$/, @pkgpart; + push @where, 'cust_pkg.pkgpart IN ('. join(',', @pkgpart ). ')' if @pkgpart; + } + + #svcnum + if ( $params->{'svcnum'} =~ /^(\d+)$/ ) { + push @where, "svcnum = $1"; } # svcpart - if ( $params->{'svcpart'} && scalar(@{ $params->{'svcpart'} }) ) { - my @svcpart = grep /^(\d+)$/, @{ $params->{'svcpart'} }; - push @where, 'svcpart IN ('. join(',', @svcpart ). ')'; + if ( $params->{'svcpart'} ) { + my @svcpart = ref( $params->{'svcpart'} ) + ? @{ $params->{'svcpart'} } + : $params->{'svcpart'} + ? ( $params->{'svcpart'} ) + : (); + @svcpart = grep /^(\d+)$/, @svcpart; + push @where, 'svcpart IN ('. join(',', @svcpart ). ')' if @svcpart; } if ( $params->{'exportnum'} =~ /^(\d+)$/ ) { diff --git a/FS/FS/svc_Tower_Mixin.pm b/FS/FS/svc_Tower_Mixin.pm index 6adbc6f5e..850805357 100644 --- a/FS/FS/svc_Tower_Mixin.pm +++ b/FS/FS/svc_Tower_Mixin.pm @@ -27,10 +27,8 @@ towernum or sectornum can also contain 'none' to allow null values. =cut sub tower_sector_sql { - my $class = shift; - my $params = shift; - return '' unless keys %$params; - my $where = ''; + my( $class, $params ) = @_; + return () unless keys %$params; my @where; for my $field (qw(towernum sectornum)) { diff --git a/FS/FS/svc_acct.pm b/FS/FS/svc_acct.pm index 39f969f09..e36dbbd69 100644 --- a/FS/FS/svc_acct.pm +++ b/FS/FS/svc_acct.pm @@ -2822,116 +2822,39 @@ Arrayref of additional WHERE clauses, will be ANDed together. =cut -sub search { - my ($class, $params) = @_; - - my @from = ( - ' LEFT JOIN cust_svc USING ( svcnum ) ', - ' LEFT JOIN part_svc USING ( svcpart ) ', - ' LEFT JOIN cust_pkg USING ( pkgnum ) ', - FS::UI::Web::join_cust_main('cust_pkg', 'cust_pkg') - ); +sub _search_svc { + my( $class, $params, $from, $where ) = @_; - my @where = (); + #these two should probably move to svc_Domain_Mixin ? # domain if ( $params->{'domain'} ) { my $svc_domain = qsearchs('svc_domain', { 'domain'=>$params->{'domain'} } ); #preserve previous behavior & bubble up an error if $svc_domain not found? - push @where, 'domsvc = '. $svc_domain->svcnum if $svc_domain; + push @$where, 'domsvc = '. $svc_domain->svcnum if $svc_domain; } # domsvc if ( $params->{'domsvc'} =~ /^(\d+)$/ ) { - push @where, "domsvc = $1"; - } - - #unlinked - push @where, 'pkgnum IS NULL' if $params->{'unlinked'}; - - #agentnum - if ( $params->{'agentnum'} =~ /^(\d+)$/ and $1 ) { - push @where, "cust_main.agentnum = $1"; - } - - #custnum - if ( $params->{'custnum'} =~ /^(\d+)$/ and $1 ) { - push @where, "custnum = $1"; + push @$where, "domsvc = $1"; } - #pkgpart - if ( $params->{'pkgpart'} && scalar(@{ $params->{'pkgpart'} }) ) { - #XXX untaint or sql quote - push @where, - 'cust_pkg.pkgpart IN ('. join(',', @{ $params->{'pkgpart'} } ). ')'; - } # popnum if ( $params->{'popnum'} =~ /^(\d+)$/ ) { - push @where, "popnum = $1"; + push @$where, "popnum = $1"; } - # svcpart - if ( $params->{'svcpart'} =~ /^(\d+)$/ ) { - push @where, "svcpart = $1"; - } - if ( $params->{'exportnum'} =~ /^(\d+)$/ ) { - push @from, ' LEFT JOIN export_svc USING ( svcpart )'; - push @where, "exportnum = $1"; - } + #and these in svc_Tower_Mixin, or maybe we never should have done svc_acct + # towers (or, as mark thought, never should have done svc_broadband) # sector and tower my @where_sector = $class->tower_sector_sql($params); if ( @where_sector ) { - push @where, @where_sector; - push @from, ' LEFT JOIN tower_sector USING ( sectornum )'; - } - - # here is the agent virtualization - #if ($params->{CurrentUser}) { - # my $access_user = - # qsearchs('access_user', { username => $params->{CurrentUser} }); - # - # if ($access_user) { - # push @where, $access_user->agentnums_sql('table'=>'cust_main'); - # }else{ - # push @where, "1=0"; - # } - #} else { - push @where, $FS::CurrentUser::CurrentUser->agentnums_sql( - 'table' => 'cust_main', - 'null_right' => 'View/link unlinked services', - ); - #} - - push @where, @{ $params->{'where'} } if $params->{'where'}; - - my $addl_from = join(' ', @from); - my $extra_sql = scalar(@where) ? ' WHERE '. join(' AND ', @where) : ''; - - my $count_query = "SELECT COUNT(*) FROM svc_acct $addl_from $extra_sql"; - #if ( keys %svc_acct ) { - # $count_query .= ' WHERE '. - # join(' AND ', map "$_ = ". dbh->quote($svc_acct{$_}), - # keys %svc_acct - # ); - #} - - my $sql_query = { - 'table' => 'svc_acct', - 'hashref' => {}, # \%svc_acct, - 'select' => join(', ', - 'svc_acct.*', - 'part_svc.svc', - 'cust_main.custnum', - FS::UI::Web::cust_sql_fields($params->{'cust_fields'}), - ), - 'addl_from' => $addl_from, - 'extra_sql' => $extra_sql, - 'order_by' => $params->{'order_by'}, - 'count_query' => $count_query, - }; + push @$where, @where_sector; + push @$from, ' LEFT JOIN tower_sector USING ( sectornum )'; + } } diff --git a/FS/FS/svc_broadband.pm b/FS/FS/svc_broadband.pm index 3b0b01534..87c73f2f0 100755 --- a/FS/FS/svc_broadband.pm +++ b/FS/FS/svc_broadband.pm @@ -175,115 +175,44 @@ Parameters: =cut -sub search { - my ($class, $params) = @_; - my @where = (); - my @from = ( - 'LEFT JOIN cust_svc USING ( svcnum )', - 'LEFT JOIN part_svc USING ( svcpart )', - 'LEFT JOIN cust_pkg USING ( pkgnum )', - FS::UI::Web::join_cust_main('cust_pkg', 'cust_pkg'), - ); - - # based on FS::svc_acct::search, probably the most mature of the bunch - #unlinked - push @where, 'pkgnum IS NULL' if $params->{'unlinked'}; - - #agentnum - if ( $params->{'agentnum'} =~ /^(\d+)$/ and $1 ) { - push @where, "cust_main.agentnum = $1"; - } - push @where, $FS::CurrentUser::CurrentUser->agentnums_sql( - 'null_right' => 'View/link unlinked services', - 'table' => 'cust_main' - ); - - #custnum - if ( $params->{'custnum'} =~ /^(\d+)$/ and $1 ) { - push @where, "custnum = $1"; - } - - #pkgpart, now properly untainted, can be arrayref - for my $pkgpart ( $params->{'pkgpart'} ) { - if ( ref $pkgpart ) { - my $where = join(',', map { /^(\d+)$/ ? $1 : () } @$pkgpart ); - push @where, "cust_pkg.pkgpart IN ($where)" if $where; - } - elsif ( $pkgpart =~ /^(\d+)$/ ) { - push @where, "cust_pkg.pkgpart = $1"; - } - } +sub _search_svc { + my( $class, $params, $from, $where ) = @_; #routernum, can be arrayref for my $routernum ( $params->{'routernum'} ) { # this no longer uses addr_block if ( ref $routernum and grep { $_ } @$routernum ) { my $in = join(',', map { /^(\d+)$/ ? $1 : () } @$routernum ); - my @orwhere; + my @orwhere = (); push @orwhere, "svc_broadband.routernum IN ($in)" if $in; push @orwhere, "svc_broadband.routernum IS NULL" if grep /^none$/, @$routernum; - push @where, '( '.join(' OR ', @orwhere).' )'; + push @$where, '( '.join(' OR ', @orwhere).' )'; } elsif ( $routernum =~ /^(\d+)$/ ) { - push @where, "svc_broadband.routernum = $1"; + push @$where, "svc_broadband.routernum = $1"; } elsif ( $routernum eq 'none' ) { - push @where, "svc_broadband.routernum IS NULL"; + push @$where, "svc_broadband.routernum IS NULL"; } } + #this should probably move to svc_Tower_Mixin, or maybe we never should have + # done svc_acct # towers (or, as mark thought, never should have done + # svc_broadband) + #sector and tower, as above my @where_sector = $class->tower_sector_sql($params); if ( @where_sector ) { - push @where, @where_sector; - push @from, 'LEFT JOIN tower_sector USING ( sectornum )'; + push @$where, @where_sector; + push @$from, 'LEFT JOIN tower_sector USING ( sectornum )'; } - #svcnum - if ( $params->{'svcnum'} =~ /^(\d+)$/ ) { - push @where, "svcnum = $1"; - } - - #svcpart - if ( $params->{'svcpart'} =~ /^(\d+)$/ ) { - push @where, "svcpart = $1"; - } - - #exportnum - if ( $params->{'exportnum'} =~ /^(\d+)$/ ) { - push @from, 'LEFT JOIN export_svc USING ( svcpart )'; - push @where, "exportnum = $1"; - } - #ip_addr if ( $params->{'ip_addr'} =~ /^(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/ ) { - push @where, "ip_addr = '$1'"; + push @$where, "ip_addr = '$1'"; } - #custnum - if ( $params->{'custnum'} =~ /^(\d+)$/ and $1) { - push @where, "custnum = $1"; - } - - my $addl_from = join(' ', @from); - my $extra_sql = ''; - $extra_sql = 'WHERE '.join(' AND ', @where) if @where; - my $count_query = "SELECT COUNT(*) FROM svc_broadband $addl_from $extra_sql"; - return( { - 'table' => 'svc_broadband', - 'hashref' => {}, - 'select' => join(', ', - 'svc_broadband.*', - 'part_svc.svc', - 'cust_main.custnum', - FS::UI::Web::cust_sql_fields($params->{'cust_fields'}), - ), - 'extra_sql' => $extra_sql, - 'addl_from' => $addl_from, - 'order_by' => "ORDER BY ".($params->{'order_by'} || 'svcnum'), - 'count_query' => $count_query, - } ); } =item search_sql STRING -- 2.11.0