From 7c7d54475dcdb041af4b42b13d3d4084627a8e9f Mon Sep 17 00:00:00 2001 From: Jonathan Prykop Date: Wed, 4 Mar 2015 18:33:52 -0600 Subject: [PATCH] RT#22952: Employee drop down list in reports shows employee users for all agents --- 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 +- .../search/report_cust_credit_source_bill_pkg.html | 11 +- httemplate/search/report_cust_credit_void.html | 10 +- httemplate/search/report_cust_pkg_discount.html | 10 +- 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. 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(); 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'); 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'); -- 2.11.0