From 13e9b14349aad53868689d8cfbc1ddb71d675b8a Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Wed, 10 Apr 2013 13:51:50 +0000 Subject: [PATCH] Bug 7295 [follow-up] More granular permissions for baskets - updatedatabase.pl now fills the new aqbasket.branch with branchcode of basket owner (aqbasket.authorisedby). So the behavior is unchanged if you have set your syspref AcqViewBaskets to 'branch' - If AcqViewBaskets='branch', prevent librarian to add users that are not from the same branch than the basket. --- C4/Acquisition.pm | 23 ++++++++++++--- acqui/aqbasketuser_search.pl | 30 ++++++++++--------- installer/data/mysql/updatedatabase.pl | 5 +++ .../prog/en/modules/acqui/aqbasketuser_search.tt | 4 ++- .../intranet-tmpl/prog/en/modules/acqui/basket.tt | 2 +- 5 files changed, 43 insertions(+), 21 deletions(-) diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index 4e464a9..51e4b1b 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -689,13 +689,24 @@ C4::Acquisition::GetBasket. The third parameter is optional. If given, it should be a hashref as returned by C4::Auth::getuserflags. If not, getuserflags is called. +Fourth parameter is a hash of options. +Avaiable options are: + +=over 2 + +=item * B: +Assume the borrower is in the basket's users list. So don't check this. This +is useful to check if a borrower can be added to the users list. + +=back + If user is authorised to manage basket, returns 1. Otherwise returns 0. =cut sub CanUserManageBasket { - my ($borrower, $basket, $userflags) = @_; + my ($borrower, $basket, $userflags, %options) = @_; if (!ref $borrower) { $borrower = C4::Members::GetMember(borrowernumber => $borrower); @@ -734,10 +745,12 @@ sub CanUserManageBasket { return 0; } - if ($AcqViewBaskets eq 'user' - && $basket->{authorisedby} != $borrowernumber - && grep($borrowernumber, GetBasketUsers($basketno)) == 0) { - return 0; + unless ($options{assume_in_user_list}) { + if ($AcqViewBaskets eq 'user' + && $basket->{authorisedby} != $borrowernumber + && grep($borrowernumber, GetBasketUsers($basketno)) == 0) { + return 0; + } } if ($AcqViewBaskets eq 'branch' && defined $basket->{branch} diff --git a/acqui/aqbasketuser_search.pl b/acqui/aqbasketuser_search.pl index 512c10a..a6a2ffe 100755 --- a/acqui/aqbasketuser_search.pl +++ b/acqui/aqbasketuser_search.pl @@ -25,6 +25,7 @@ use CGI; use C4::Auth; use C4::Output; use C4::Members; +use C4::Acquisition; my $input = new CGI; @@ -41,29 +42,28 @@ my ( $template, $loggedinuser, $cookie, $staff_flags ) = get_template_and_user( my $q = $input->param('q') || ''; my $op = $input->param('op') || ''; +my $basketno = $input->param('basketno'); +my $basket = C4::Acquisition::GetBasket($basketno); if( $op eq "do_search" ) { my $results = C4::Members::Search( $q, "surname"); my @users_loop; my $nresults = 0; - foreach my $res (@$results) { - my $perms = haspermission( $res->{userid} ); - my $subperms = get_user_subpermissions( $res->{userid} ); - - if( $perms->{superlibrarian} == 1 - || $perms->{acquisition} == 1 - || $subperms->{acquisition}->{'order_manage'} ) { + foreach my $borrower (@$results) { + if (C4::Acquisition::CanUserManageBasket( + $borrower, $basket, undef, assume_in_user_list => 1)) + { my %row = ( - borrowernumber => $res->{borrowernumber}, - cardnumber => $res->{cardnumber}, - surname => $res->{surname}, - firstname => $res->{firstname}, - categorycode => $res->{categorycode}, - branchcode => $res->{branchcode}, + borrowernumber => $borrower->{borrowernumber}, + cardnumber => $borrower->{cardnumber}, + surname => $borrower->{surname}, + firstname => $borrower->{firstname}, + categorycode => $borrower->{categorycode}, + branchcode => $borrower->{branchcode}, ); push( @users_loop, \%row ); - $nresults ++; + $nresults++; } } @@ -74,4 +74,6 @@ if( $op eq "do_search" ) { ); } +$template->param(basketno => $basketno); + output_html_with_http_headers( $input, $cookie, $template->output ); diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 887899d..e6ef7b9 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -6783,6 +6783,11 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { ON UPDATE CASCADE ON DELETE SET NULL "); $dbh->do(" + UPDATE aqbasket, borrowers + SET aqbasket.branch = borrowers.branchcode + WHERE aqbasket.authorisedby = borrowers.borrowernumber + "); + $dbh->do(" DROP TABLE IF EXISTS aqbasketusers "); $dbh->do(" diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/aqbasketuser_search.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/aqbasketuser_search.tt index 9b7f52b..884f450 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/aqbasketuser_search.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/aqbasketuser_search.tt @@ -34,10 +34,12 @@
+
-
Only staff with superlibrarian or acquisitions permissions (or order_manage permission if granular permissions are enabled) are returned in the search results
+

Only staff with superlibrarian or acquisitions permissions (or order_manage permission if granular permissions are enabled) are returned in the search results.

+

Note that if system preference AcqViewBasket is set to "from staff member's library", only staff from the same branch than the basket will be returned.

[% IF (q) %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt index 28b6f4e..9faa9dd 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt @@ -117,7 +117,7 @@ function basketUserSearchPopup(f) { window.open( - "/cgi-bin/koha/acqui/aqbasketuser_search.pl", + "/cgi-bin/koha/acqui/aqbasketuser_search.pl?basketno=[% basketno %]", 'BasketUserSearchPopup', 'width=740,height=450,toolbar=no,' ); -- 1.7.2.5