[freeside-commits] branch FREESIDE_3_BRANCH updated. 5156232ada5f9bd31aabb7def5cab671cd8e674f

Jonathan Prykop jonathan at 420.am
Mon Mar 9 20:08:26 PDT 2015


The branch, FREESIDE_3_BRANCH has been updated
       via  5156232ada5f9bd31aabb7def5cab671cd8e674f (commit)
       via  64489396dd98a369aa2c38d3802400c2c508f136 (commit)
      from  a4b5a3e0da1e747e5937be1cc69841158743f8a9 (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 5156232ada5f9bd31aabb7def5cab671cd8e674f
Author: Jonathan Prykop <jonathan at freeside.biz>
Date:   Mon Mar 9 22:06:10 2015 -0500

    RT#22952: Employee drop down list in reports shows employee users for all agents [show disabled users on 3 branch only]

diff --git a/httemplate/search/report_cust_bill_pkg_discount.html b/httemplate/search/report_cust_bill_pkg_discount.html
index 10ccba9..78c5e4f 100644
--- a/httemplate/search/report_cust_bill_pkg_discount.html
+++ b/httemplate/search/report_cust_bill_pkg_discount.html
@@ -44,6 +44,6 @@
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Financial reports');
 
-my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_pkg_discount');
+my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_pkg_discount', 'disabled' => 1);
 
 </%init>
diff --git a/httemplate/search/report_cust_credit.html b/httemplate/search/report_cust_credit.html
index 0d7a277..b283e05 100644
--- a/httemplate/search/report_cust_credit.html
+++ b/httemplate/search/report_cust_credit.html
@@ -38,7 +38,7 @@
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Financial reports');
 
-my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_credit');
+my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_credit', 'disabled' => 1);
 
 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 2849c8d..444bdb7 100644
--- a/httemplate/search/report_cust_credit_bill_pkg.html
+++ b/httemplate/search/report_cust_credit_bill_pkg.html
@@ -89,7 +89,7 @@
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Financial reports');
 
-my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_credit');
+my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_credit', 'disabled' => 1);
 
 my $conf = new FS::Conf;
 
diff --git a/httemplate/search/report_cust_credit_void.html b/httemplate/search/report_cust_credit_void.html
index 16a9e42..53a1712 100644
--- a/httemplate/search/report_cust_credit_void.html
+++ b/httemplate/search/report_cust_credit_void.html
@@ -37,7 +37,7 @@
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Financial reports');
 
-my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_credit_void');
+my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_credit_void', 'disabled' => 1);
 
 my $title = 'Voided credit report';
 
diff --git a/httemplate/search/report_cust_pkg_discount.html b/httemplate/search/report_cust_pkg_discount.html
index 2b9052f..3cd6c3e 100644
--- a/httemplate/search/report_cust_pkg_discount.html
+++ b/httemplate/search/report_cust_pkg_discount.html
@@ -45,6 +45,6 @@
 die "access denied"
   unless $FS::CurrentUser::CurrentUser->access_right('Financial reports');
 
-my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_pkg_discount');
+my $access_user = $FS::CurrentUser::CurrentUser->access_users_hashref('table' => 'cust_pkg_discount', 'disabled' => 1);
 
 </%init>

commit 64489396dd98a369aa2c38d3802400c2c508f136
Author: Jonathan Prykop <jonathan at freeside.biz>
Date:   Wed Mar 4 18:33:52 2015 -0600

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

diff --git a/FS/FS/access_user.pm b/FS/FS/access_user.pm
index 0f07e77..4b5a701 100644
--- a/FS/FS/access_user.pm
+++ b/FS/FS/access_user.pm
@@ -400,7 +400,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"
@@ -416,17 +418,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 ... ]
@@ -446,6 +455,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 ec2341b..e77788f 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 77affd1..10ccba9 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 dbab66a..0d7a277 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 1754032..2849c8d 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,
@@ -89,14 +89,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 e967080..16a9e42 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 7f0e55f..2b9052f 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>

-----------------------------------------------------------------------

Summary of changes:
 FS/FS/access_user.pm                               |  113 +++++++++++++++++++-
 httemplate/elements/select-user.html               |   13 +--
 .../search/report_cust_bill_pkg_discount.html      |   10 +-
 httemplate/search/report_cust_credit.html          |   10 +-
 httemplate/search/report_cust_credit_bill_pkg.html |   11 +-
 httemplate/search/report_cust_credit_void.html     |   10 +-
 httemplate/search/report_cust_pkg_discount.html    |   10 +-
 7 files changed, 121 insertions(+), 56 deletions(-)




More information about the freeside-commits mailing list