refactor svc_acct and svc_broadband search into svc_Common, RT#21054

4 files changed, 60 insertions, 184 deletions

diff --git a/FS/FS/svc_Common.pm b/FS/FS/svc_Common.pm
index f7f0b40..ff465bf 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 6adbc6f..8508053 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 39f969f..e36dbbd 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 3b0b015..87c73f2 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