RT#22952: Employee drop down list in reports shows employee users for all agents

8 files changed, 123 insertions, 65 deletions

diff --git a/FS/FS/access_user.pm b/FS/FS/access_user.pm
index 44d3beec1..68d2deaba 100644
--- a/FS/FS/access_user.pm
+++ b/FS/FS/access_user.pm
@@ -354,7 +354,9 @@ sub agentnums_sql {
   if ( $self->access_right($viewall_right) ) {
     push @or, "$agentnum IS NOT NULL";
   } else {
-    push @or, "$agentnum IN (". join(',', $self->agentnums). ')';
+    my @agentnums = $self->agentnums;
+    push @or, "$agentnum IN (". join(',', @agentnums). ')'
+      if @agentnums;
   }
 
   push @or, "$agentnum IS NULL"
@@ -370,17 +372,24 @@ sub agentnums_sql {
 
 Returns true if the user can view the specified agent.
 
+Also accepts optional hashref cache, to avoid redundant database calls.
+
 =cut
 
 sub agentnum {
-  my( $self, $agentnum ) = @_;
+  my( $self, $agentnum, $cache ) = @_;
+  $cache ||= {};
+  return $cache->{$self->usernum}->{$agentnum}
+    if $cache->{$self->usernum}->{$agentnum};
   my $sth = dbh->prepare(
     "SELECT COUNT(*) FROM access_usergroup
                      JOIN access_groupagent USING ( groupnum )
        WHERE usernum = ? AND agentnum = ?"
   ) or die dbh->errstr;
   $sth->execute($self->usernum, $agentnum) or die $sth->errstr;
-  $sth->fetchrow_arrayref->[0];
+  $cache->{$self->usernum}->{$agentnum} = $sth->fetchrow_arrayref->[0];
+  $sth->finish;
+  return $cache->{$self->usernum}->{$agentnum};
 }
 
 =item agents [ HASHREF | OPTION => VALUE ... ]
@@ -400,6 +409,104 @@ sub agents {
   });
 }
 
+=item access_users [ HASHREF | OPTION => VALUE ... ]
+
+Returns an array of FS::access_user objects, one for each non-disabled 
+access_user in the system that shares an agent (via group membership) with 
+the invoking object.  Regardless of options and agents, will always at
+least return the invoking user and any users who have viewall_right.
+
+Accepts the following options:
+
+=over 4
+
+=item table
+
+Only return users who appear in the usernum field of this table
+
+=item disabled
+
+Include disabled users if true (defaults to false)
+
+=item viewall_right
+
+All users will be returned if the current user has the provided 
+access right, regardless of agents (other filters still apply.)  
+Defaults to 'View customers of all agents'
+
+=cut
+
+#Leaving undocumented until such time as this functionality is actually used
+#
+#=item null
+#
+#Users with no agents will be returned.
+#
+#=item null_right
+#
+#Users with no agents will be returned if the current user has the provided
+#access right.
+
+sub access_users {
+  my $self = shift;
+  my %opt = ref($_[0]) ? %{$_[0]} : @_;
+  my $table = $opt{'table'};
+  my $search = { 'table' => 'access_user' };
+  $search->{'hashref'} = $opt{'disabled'} ? {} : { 'disabled' => '' };
+  $search->{'addl_from'} = "INNER JOIN $table ON (access_user.usernum = $table.usernum)"
+    if $table;
+  my @access_users = qsearch($search);
+  my $viewall_right = $opt{'viewall_right'} || 'View customers of all agents';
+  return @access_users if $self->access_right($viewall_right);
+  #filter for users with agents $self can view
+  my @out;
+  my $agentnum_cache = {};
+ACCESS_USER:
+  foreach my $access_user (@access_users) {
+    # you can always view yourself, regardless of agents,
+    # and you can always view someone who can view you, 
+    # since they might have affected your customers
+    if ( ($self->usernum eq $access_user->usernum) 
+         || $access_user->access_right($viewall_right)
+    ) {
+      push(@out,$access_user);
+      next;
+    }
+    # if user has no agents, you need null or null_right to view
+    my @agents = $access_user->agents('viewall_right'=>'NONE'); #handled viewall_right above
+    if (!@agents) {
+      if ( $opt{'null'} ||
+           ( $opt{'null_right'} && $self->access_right($opt{'null_right'}) )
+      ) {
+        push(@out,$access_user);
+      }
+      next;
+    }
+    # otherwise, you need an agent in common
+    foreach my $agent (@agents) {
+      if ($self->agentnum($agent->agentnum,$agentnum_cache)) {
+        push(@out,$access_user);
+        next ACCESS_USER;
+      }
+    }
+  }
+  return @out;
+}
+
+=item access_users_hashref  [ HASHREF | OPTION => VALUE ... ]
+
+Accepts same options as L</access_users>.  Returns a hashref of
+users, with keys of usernum and values of username.
+
+=cut
+
+sub access_users_hashref {
+  my $self = shift;
+  my %access_users = map { $_->usernum => $_->username } 
+                       $self->access_users(@_);
+  return \%access_users;
+}
+
 =item access_right RIGHTNAME | LISTREF
 
 Given a right name or a list reference of right names, returns true if this
diff --git a/httemplate/elements/select-user.html b/httemplate/elements/select-user.html
index ec2341be6..e77788f61 100644
--- a/httemplate/elements/select-user.html
+++ b/httemplate/elements/select-user.html
@@ -17,17 +17,6 @@
 
 my %opt = @_;
 
-unless ( $opt{'access_user'} ) {
-
-  my $sth = dbh->prepare("
-    SELECT usernum, username FROM access_user
-      WHERE disabled = '' or disabled IS NULL
-  ") or die dbh->errstr;
-  $sth->execute or die $sth->errstr;
-  while ( my $row = $sth->fetchrow_arrayref ) {
-    $opt{'access_user'}->{$row->[0]} = $row->[1];
-  }
-
-}
+$opt{'access_user'} ||= $FS::CurrentUser::CurrentUser->access_users_hashref();
 
 </%init>
diff --git a/httemplate/search/report_cust_bill_pkg_discount.html b/httemplate/search/report_cust_bill_pkg_discount.html
index 77affd19d..10ccba912 100644
--- a/httemplate/search/report_cust_bill_pkg_discount.html
+++ b/httemplate/search/report_cust_bill_pkg_discount.html
@@ -13,7 +13,7 @@
 
   <& /elements/tr-select-user.html,
        'label'       => 'Discounts by employee: ',
-       'access_user' => \%access_user,
+       'access_user' => $access_user,
   &>
 
   <& /elements/tr-select-agent.html,
@@ -44,12 +44,6 @@
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Financial reports');
 
-my $sth = dbh->prepare("SELECT DISTINCT usernum FROM cust_pkg_discount")
-  or die dbh->errstr;
-$sth->execute or die $sth->errstr;
-my @usernum = map $_->[0], @{$sth->fetchall_arrayref};
-my %access_user =
-  map { $_ => qsearchs('access_user',{'usernum'=>$_})->username }
-      @usernum;
+my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_pkg_discount');
 
 </%init>
diff --git a/httemplate/search/report_cust_credit.html b/httemplate/search/report_cust_credit.html
index dbab66ae5..0d7a2770a 100644
--- a/httemplate/search/report_cust_credit.html
+++ b/httemplate/search/report_cust_credit.html
@@ -8,7 +8,7 @@
 
   <& /elements/tr-select-user.html,
                 'label'       => emt('Credits by employee: '),
-                'access_user' => \%access_user,
+                'access_user' => $access_user,
   &>
 
   <& /elements/tr-select-agent.html,
@@ -38,13 +38,7 @@
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Financial reports');
 
-my $sth = dbh->prepare("SELECT DISTINCT usernum FROM cust_credit")
-  or die dbh->errstr;
-$sth->execute or die $sth->errstr;
-my @usernum = map $_->[0], @{$sth->fetchall_arrayref};
-my %access_user =
-  map { $_ => qsearchs('access_user',{'usernum'=>$_})->username }
-      @usernum;
+my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_credit');
 
 my $unapplied = $cgi->param('unapplied') ? 1 : 0;
 
diff --git a/httemplate/search/report_cust_credit_bill_pkg.html b/httemplate/search/report_cust_credit_bill_pkg.html
index ad0f3f62e..1a54a122b 100644
--- a/httemplate/search/report_cust_credit_bill_pkg.html
+++ b/httemplate/search/report_cust_credit_bill_pkg.html
@@ -7,7 +7,7 @@
 
 <& /elements/tr-select-user.html,
               'label'       => emt('Employee: '),
-              'access_user' => \%access_user,
+              'access_user' => $access_user,
 &>
 
 <& /elements/tr-select-agent.html,
@@ -80,14 +80,7 @@
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Financial reports');
 
-#false laziness w/report_cust_credit.html
-my $sth = dbh->prepare("SELECT DISTINCT usernum FROM cust_credit")
-  or die dbh->errstr;
-$sth->execute or die $sth->errstr;
-my @usernum = map $_->[0], @{$sth->fetchall_arrayref};
-my %access_user =
-  map { $_ => qsearchs('access_user',{'usernum'=>$_})->username }
-      @usernum;
+my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_credit');
 
 my $conf = new FS::Conf;
 
diff --git a/httemplate/search/report_cust_credit_source_bill_pkg.html b/httemplate/search/report_cust_credit_source_bill_pkg.html
index b579b92cc..7bfacc4fa 100644
--- a/httemplate/search/report_cust_credit_source_bill_pkg.html
+++ b/httemplate/search/report_cust_credit_source_bill_pkg.html
@@ -7,7 +7,7 @@
 
 <& /elements/tr-select-user.html,
               'label'       => emt('Employee: '),
-              'access_user' => \%access_user,
+              'access_user' => $access_user,
 &>
 
 <& /elements/tr-select-agent.html,
@@ -58,14 +58,7 @@
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Financial reports');
 
-#false laziness w/report_cust_credit.html
-my $sth = dbh->prepare("SELECT DISTINCT usernum FROM cust_credit")
-  or die dbh->errstr;
-$sth->execute or die $sth->errstr;
-my @usernum = map $_->[0], @{$sth->fetchall_arrayref};
-my %access_user =
-  map { $_ => qsearchs('access_user',{'usernum'=>$_})->username }
-      @usernum;
+my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_credit');
 
 my $conf = new FS::Conf;
 
diff --git a/httemplate/search/report_cust_credit_void.html b/httemplate/search/report_cust_credit_void.html
index e96708090..16a9e42d6 100644
--- a/httemplate/search/report_cust_credit_void.html
+++ b/httemplate/search/report_cust_credit_void.html
@@ -7,7 +7,7 @@
 
   <& /elements/tr-select-user.html,
                 'label'       => emt('Credit voids by employee: '),
-                'access_user' => \%access_user,
+                'access_user' => $access_user,
   &>
 
   <& /elements/tr-select-agent.html,
@@ -37,13 +37,7 @@
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Financial reports');
 
-my $sth = dbh->prepare("SELECT DISTINCT usernum FROM cust_credit_void")
-  or die dbh->errstr;
-$sth->execute or die $sth->errstr;
-my @usernum = map $_->[0], @{$sth->fetchall_arrayref};
-my %access_user =
-  map { $_ => qsearchs('access_user',{'usernum'=>$_})->username }
-      @usernum;
+my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_credit_void');
 
 my $title = 'Voided credit report';
 
diff --git a/httemplate/search/report_cust_pkg_discount.html b/httemplate/search/report_cust_pkg_discount.html
index 7f0e55fed..2b9052f5b 100644
--- a/httemplate/search/report_cust_pkg_discount.html
+++ b/httemplate/search/report_cust_pkg_discount.html
@@ -23,7 +23,7 @@
 
   <& /elements/tr-select-user.html,
        'label'       => 'Discounts by employee: ',
-       'access_user' => \%access_user,
+       'access_user' => $access_user,
   &>
 
   <& /elements/tr-select-agent.html,
@@ -45,12 +45,6 @@
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Financial reports');
 
-my $sth = dbh->prepare("SELECT DISTINCT usernum FROM cust_pkg_discount")
-  or die dbh->errstr;
-$sth->execute or die $sth->errstr;
-my @usernum = map $_->[0], @{$sth->fetchall_arrayref};
-my %access_user =
-  map { $_ => qsearchs('access_user',{'usernum'=>$_})->username }
-      @usernum;
+my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_pkg_discount');
 
 </%init>