From d4cc1d8e60e11b7f2bc9e68cc514161e7168ab8b Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Fri, 20 Apr 2012 17:12:29 +0200 Subject: [PATCH 1/2] Bug 7295: More granular permissions for baskets Add branch info to baskets Add a list of borrowers that are allowed to manage a basket (one list for each basket). Add a new subpermission: acquisition => order_manage_all If user is superlibrarian, or if he has permission acquisition = 1 (GranularPermissions = OFF), or subpermission acquisition => order_manage_all (GranularPermissions = ON). He's authorised to manage all baskets. Depending on syspref AcqViewBaskets: 'all': user can manage all baskets 'branch': user can manage baskets of his branch (the basket branch is taken into account, not the branch of the basket's creator). If basket branch is not defined, all user can manage this basket. 'user': user can manage baskets he created, and baskets he's in their user list There are unit tests in t/Acquisition/CanUserManageBasket.t, which require Test::MockModule You can edit basket's branch and users list in basket modification page (acqui/basket.pl) --- C4/Acquisition.pm | 149 +++++++++++++- acqui/aqbasketuser_search.pl | 77 ++++++++ acqui/basket.pl | 61 +++++- acqui/booksellers.pl | 26 +-- .../data/mysql/de-DE/mandatory/userpermissions.sql | 1 + .../data/mysql/en/mandatory/userpermissions.sql | 1 + .../data/mysql/es-ES/mandatory/userpermissions.sql | 1 + .../mysql/fr-FR/1-Obligatoire/userpermissions.sql | 1 + .../data/mysql/it-IT/necessari/userpermissions.sql | 1 + installer/data/mysql/kohastructure.sql | 15 ++ .../mysql/nb-NO/1-Obligatorisk/userpermissions.sql | 1 + .../data/mysql/pl-PL/mandatory/userpermissions.sql | 1 + .../ru-RU/mandatory/permissions_and_user_flags.sql | 1 + .../uk-UA/mandatory/permissions_and_user_flags.sql | 1 + installer/data/mysql/updatedatabase.pl | 34 ++++ .../prog/en/modules/acqui/aqbasketuser_search.tt | 79 ++++++++ .../intranet-tmpl/prog/en/modules/acqui/basket.tt | 81 +++++++- .../en/modules/admin/preferences/acquisitions.pref | 2 +- t/Acquisition/CanUserManageBasket.t | 204 ++++++++++++++++++++ 19 files changed, 704 insertions(+), 33 deletions(-) create mode 100755 acqui/aqbasketuser_search.pl create mode 100644 koha-tmpl/intranet-tmpl/prog/en/modules/acqui/aqbasketuser_search.tt create mode 100644 t/Acquisition/CanUserManageBasket.t diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index 9678ffc..a01065c 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -48,6 +48,9 @@ BEGIN { &GetBasketsByBookseller &GetBasketsByBasketgroup &GetBasketsInfosByBookseller + &GetBasketUsers &ModBasketUsers + &CanUserManageBasket + &ModBasketHeader &ModBasketgroup &NewBasketgroup &DelBasketgroup &GetBasketgroup &CloseBasketgroup @@ -148,8 +151,7 @@ sub GetBasket { my $dbh = C4::Context->dbh; my $query = " SELECT aqbasket.*, - concat( b.firstname,' ',b.surname) AS authorisedbyname, - b.branchcode AS branch + concat( b.firstname,' ',b.surname) AS authorisedbyname FROM aqbasket LEFT JOIN borrowers b ON aqbasket.authorisedby=b.borrowernumber WHERE basketno=? @@ -576,6 +578,149 @@ sub GetBasketsInfosByBookseller { return $sth->fetchall_arrayref({}); } +=head3 GetBasketUsers + + $basketusers_ids = &GetBasketUsers($basketno); + +Returns a list of all borrowernumbers that are in basket users list + +=cut + +sub GetBasketUsers { + my $basketno = shift; + + return unless $basketno; + + my $query = qq{ + SELECT borrowernumber + FROM aqbasketusers + WHERE basketno = ? + }; + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare($query); + $sth->execute($basketno); + my $results = $sth->fetchall_arrayref( {} ); + $sth->finish(); + + my @borrowernumbers; + foreach (@$results) { + push @borrowernumbers, $_->{'borrowernumber'}; + } + + return @borrowernumbers; +} + +=head3 ModBasketUsers + + my @basketusers_ids = (1, 2, 3); + &ModBasketUsers($basketno, @basketusers_ids); + +Delete all users from basket users list, and add users in C<@basketusers_ids> +to this users list. + +=cut + +sub ModBasketUsers { + my ($basketno, @basketusers_ids) = @_; + + return unless $basketno; + + my $dbh = C4::Context->dbh; + my $query = qq{ + DELETE FROM aqbasketusers + WHERE basketno = ? + }; + my $sth = $dbh->prepare($query); + $sth->execute($basketno); + $sth->finish(); + + $query = qq{ + INSERT INTO aqbasketusers (basketno, borrowernumber) + VALUES (?, ?) + }; + $sth = $dbh->prepare($query); + foreach my $basketuser_id (@basketusers_ids) { + $sth->execute($basketno, $basketuser_id); + } +} + +=head3 CanUserManageBasket + + my $bool = CanUserManageBasket($borrower, $basket[, $userflags]); + my $bool = CanUserManageBasket($borrowernumber, $basketno[, $userflags]); + +Check if a borrower can manage a basket, according to system preference +AcqViewBaskets, user permissions and basket properties (creator, users list, +branch). + +First parameter can be either a borrowernumber or a hashref as returned by +C4::Members::GetMember. + +Second parameter can be either a basketno or a hashref as returned by +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. + +If user is authorised to manage basket, returns 1. +Otherwise returns 0. + +=cut + +sub CanUserManageBasket { + my ($borrower, $basket, $userflags) = @_; + + if (!ref $borrower) { + $borrower = C4::Members::GetMember(borrowernumber => $borrower); + } + if (!ref $basket) { + $basket = GetBasket($basket); + } + + return 0 unless ($basket and $borrower); + + my $borrowernumber = $borrower->{borrowernumber}; + my $basketno = $basket->{basketno}; + + my $AcqViewBaskets = C4::Context->preference('AcqViewBaskets'); + + if (!defined $userflags) { + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare("SELECT flags FROM borrowers WHERE borrowernumber = ?"); + $sth->execute($borrowernumber); + my ($flags) = $sth->fetchrow_array; + $sth->finish; + + $userflags = C4::Auth::getuserflags($flags, $borrower->{userid}, $dbh); + } + + unless ($userflags->{superlibrarian} + || (ref $userflags->{acquisition} && $userflags->{acquisition}->{order_manage_all}) + || (!ref $userflags->{acquisition} && $userflags->{acquisition})) + { + if (not exists $userflags->{acquisition}) { + return 0; + } + + if ( (ref $userflags->{acquisition} && !$userflags->{acquisition}->{order_manage}) + || (!ref $userflags->{acquisition} && !$userflags->{acquisition}) ) { + return 0; + } + + if ($AcqViewBaskets eq 'user' + && $basket->{authorisedby} != $borrowernumber + && grep($borrowernumber, GetBasketUsers($basketno)) == 0) { + return 0; + } + + if ($AcqViewBaskets eq 'branch' && defined $basket->{branch} + && $basket->{branch} ne $borrower->{branchcode}) { + return 0; + } + } + + return 1; +} #------------------------------------------------------------# diff --git a/acqui/aqbasketuser_search.pl b/acqui/aqbasketuser_search.pl new file mode 100755 index 0000000..512c10a --- /dev/null +++ b/acqui/aqbasketuser_search.pl @@ -0,0 +1,77 @@ +#!/usr/bin/perl + +# script to find a basket user + +# Copyright 2012 BibLibre SARL +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 2 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +use Modern::Perl; + +use CGI; +use C4::Auth; +use C4::Output; +use C4::Members; + +my $input = new CGI; + +my $dbh = C4::Context->dbh; + +my ( $template, $loggedinuser, $cookie, $staff_flags ) = get_template_and_user( + { template_name => "acqui/aqbasketuser_search.tmpl", + query => $input, + type => "intranet", + authnotrequired => 0, + flagsrequired => { acquisition => 'order_manage' }, + } +); + +my $q = $input->param('q') || ''; +my $op = $input->param('op') || ''; + +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'} ) { + my %row = ( + borrowernumber => $res->{borrowernumber}, + cardnumber => $res->{cardnumber}, + surname => $res->{surname}, + firstname => $res->{firstname}, + categorycode => $res->{categorycode}, + branchcode => $res->{branchcode}, + ); + push( @users_loop, \%row ); + $nresults ++; + } + } + + $template->param( + q => $q, + nresults => $nresults, + users_loop => \@users_loop, + ); +} + +output_html_with_http_headers( $input, $cookie, $template->output ); diff --git a/acqui/basket.pl b/acqui/basket.pl index 9b36c14..839efa5 100755 --- a/acqui/basket.pl +++ b/acqui/basket.pl @@ -28,6 +28,7 @@ use C4::Output; use CGI; use C4::Acquisition; use C4::Budgets; +use C4::Branch; use C4::Bookseller qw( GetBookSellerFromId); use C4::Debug; use C4::Biblio; @@ -80,13 +81,25 @@ my ( $template, $loggedinuser, $cookie, $userflags ) = get_template_and_user( ); my $basket = GetBasket($basketno); +$booksellerid = $basket->{booksellerid} unless $booksellerid; +my ($bookseller) = GetBookSellerFromId($booksellerid); + +unless (CanUserManageBasket($loggedinuser, $basket, $userflags)) { + $template->param( + cannot_manage_basket => 1, + basketno => $basketno, + basketname => $basket->{basketname}, + booksellerid => $booksellerid, + name => $bookseller->{name} + ); + output_html_with_http_headers $query, $cookie, $template->output; + exit; +} # FIXME : what about the "discount" percentage? # FIXME : the query->param('booksellerid') below is probably useless. The bookseller is always known from the basket # if no booksellerid in parameter, get it from basket # warn "=>".$basket->{booksellerid}; -$booksellerid = $basket->{booksellerid} unless $booksellerid; -my ($bookseller) = GetBookSellerFromId($booksellerid); my $op = $query->param('op'); if (!defined $op) { $op = q{}; @@ -188,6 +201,21 @@ if ( $op eq 'delete_confirm' ) { $basket->{closedate} = undef; ModBasket($basket); print $query->redirect('/cgi-bin/koha/acqui/basket.pl?basketno='.$basket->{'basketno'}) +} elsif ( $op eq 'mod_users' ) { + my $basketusers_ids = $query->param('basketusers_ids'); + my @basketusers = split( /:/, $basketusers_ids ); + ModBasketUsers($basketno, @basketusers); + print $query->redirect("/cgi-bin/koha/acqui/basket.pl?basketno=$basketno"); + exit; +} elsif ( $op eq 'mod_branch' ) { + my $branch = $query->param('branch'); + $branch = undef if(defined $branch and $branch eq ''); + ModBasket({ + basketno => $basket->{basketno}, + branch => $branch + }); + print $query->redirect("/cgi-bin/koha/acqui/basket.pl?basketno=$basketno"); + exit; } else { # get librarian branch... if ( C4::Context->preference("IndependantBranches") ) { @@ -203,6 +231,17 @@ if ( $op eq 'delete_confirm' ) { } } } + # get branches + my $branches = C4::Branch::GetBranches; + my @branches_loop; + foreach my $branch (sort keys %$branches) { + push @branches_loop, { + branchcode => $branch, + branchname => $branches->{$branch}->{branchname}, + selected => (defined $basket->{branch} and $branch eq $basket->{branch}) ? 1 : 0 + }; + } + #if the basket is closed,and the user has the permission to edit basketgroups, display a list of basketgroups my $basketgroups; my $member = GetMember(borrowernumber => $loggedinuser); @@ -238,6 +277,13 @@ if ( $op eq 'delete_confirm' ) { "loggedinuser: $loggedinuser; creationdate: %s; authorisedby: %s", $basket->{creationdate}, $basket->{authorisedby}; + my @basketusers_ids = GetBasketUsers($basketno); + my @basketusers; + foreach my $basketuser_id (@basketusers_ids) { + my $basketuser = GetMember(borrowernumber => $basketuser_id); + push @basketusers, $basketuser if $basketuser; + } + #to get active currency my $cur = GetCurrency(); @@ -331,7 +377,7 @@ if ( $op eq 'delete_confirm' ) { push @books_loop, \%line; } -my $total_est_gste; + my $total_est_gste; my $total_est_gsti; my $gist_est; if ($gist){ # if we have GST @@ -350,9 +396,9 @@ my $total_est_gste; } $gist_est = $gist_rrp - ( $gist_rrp * $discount ); } else { - $total_rrp_gsti = $total_rrp; - $total_est_gsti = $total_rrp_est; -} + $total_rrp_gsti = $total_rrp; + $total_est_gsti = $total_rrp_est; + } my $contract = &GetContract($basket->{contractnumber}); my @orders = GetOrders($basketno); @@ -382,9 +428,12 @@ my $total_est_gste; basketbooksellernote => $basket->{booksellernote}, basketcontractno => $basket->{contractnumber}, basketcontractname => $contract->{contractname}, + branches_loop => \@branches_loop, creationdate => $basket->{creationdate}, authorisedby => $basket->{authorisedby}, authorisedbyname => $basket->{authorisedbyname}, + basketusers_ids => join(':', @basketusers_ids), + basketusers => \@basketusers, closedate => $basket->{closedate}, estimateddeliverydate=> $estimateddeliverydate, active => $bookseller->{'active'}, diff --git a/acqui/booksellers.pl b/acqui/booksellers.pl index 5a1c248..37a7bd2 100755 --- a/acqui/booksellers.pl +++ b/acqui/booksellers.pl @@ -56,13 +56,13 @@ use C4::Biblio; use C4::Output; use CGI; -use C4::Acquisition qw/ GetBasketsInfosByBookseller /; +use C4::Acquisition qw/ GetBasketsInfosByBookseller CanUserManageBasket /; use C4::Bookseller qw/ GetBookSellerFromId GetBookSeller /; use C4::Members qw/GetMember/; use C4::Context; my $query = CGI->new; -my ( $template, $loggedinuser, $cookie ) = get_template_and_user( +my ( $template, $loggedinuser, $cookie, $userflags ) = get_template_and_user( { template_name => 'acqui/booksellers.tmpl', query => $query, type => 'intranet', @@ -96,11 +96,6 @@ if ($loggedinuser) { $uid = GetMember( borrowernumber => $loggedinuser )->{userid}; } -my $userenv = C4::Context::userenv; -my $viewbaskets = C4::Context->preference('AcqViewBaskets'); - -my $userbranch = $userenv->{branch}; - #build result page my $loop_suppliers = []; @@ -110,21 +105,8 @@ for my $vendor (@suppliers) { my $loop_basket = []; for my $basket ( @{$baskets} ) { - my $authorisedby = $basket->{authorisedby}; - my $basketbranch = ''; # set a blank branch to start with - my $member = GetMember( borrowernumber => $authorisedby ); - if ( $member ) { - $basketbranch = $member->{branchcode}; - } - - if ($userenv->{'flags'} & 1 || #user is superlibrarian - (haspermission( $uid, { acquisition => q{*} } ) && #user has acq permissions and - ($viewbaskets eq 'all' || #user is allowed to see all baskets - ($viewbaskets eq 'branch' && $authorisedby && $userbranch eq $basketbranch) || #basket belongs to user's branch - ($basket->{authorisedby} && $viewbaskets eq 'user' && $authorisedby == $loggedinuser) #user created this basket - ) - ) - ) { + if (CanUserManageBasket($loggedinuser, $basket, $userflags)) { + my $member = GetMember( borrowernumber => $basket->{authorisedby} ); foreach (qw(total_items total_biblios expected_items)) { $basket->{$_} ||= 0; } diff --git a/installer/data/mysql/de-DE/mandatory/userpermissions.sql b/installer/data/mysql/de-DE/mandatory/userpermissions.sql index 7282cce..b123ea6 100644 --- a/installer/data/mysql/de-DE/mandatory/userpermissions.sql +++ b/installer/data/mysql/de-DE/mandatory/userpermissions.sql @@ -15,6 +15,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'budget_modify', 'Konten verändern (keine neuen anlegen, aber bestehende ändern)'), (11, 'planning_manage', 'Etatplanung verwalten'), (11, 'order_manage', 'Bestellungen verwalten'), + (11, 'order_manage_all', 'Manage all orders & baskets'), (11, 'group_manage', 'Bestellgruppen vewalten'), (11, 'order_receive', 'Lieferungen verwalten'), (11, 'budget_add_del', 'Konten hinzufügen/ändern, aber bestehende nicht ändern'), diff --git a/installer/data/mysql/en/mandatory/userpermissions.sql b/installer/data/mysql/en/mandatory/userpermissions.sql index 070e8f5..a6a0005 100644 --- a/installer/data/mysql/en/mandatory/userpermissions.sql +++ b/installer/data/mysql/en/mandatory/userpermissions.sql @@ -15,6 +15,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'budget_modify', 'Modify budget (can''t create lines, but can modify existing ones)'), (11, 'planning_manage', 'Manage budget plannings'), (11, 'order_manage', 'Manage orders & basket'), + (11, 'order_manage_all', 'Manage all orders & baskets'), (11, 'group_manage', 'Manage orders & basketgroups'), (11, 'order_receive', 'Manage orders & basket'), (11, 'budget_add_del', 'Add and delete budgets (but cant modify budgets)'), diff --git a/installer/data/mysql/es-ES/mandatory/userpermissions.sql b/installer/data/mysql/es-ES/mandatory/userpermissions.sql index 070e8f5..a6a0005 100644 --- a/installer/data/mysql/es-ES/mandatory/userpermissions.sql +++ b/installer/data/mysql/es-ES/mandatory/userpermissions.sql @@ -15,6 +15,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'budget_modify', 'Modify budget (can''t create lines, but can modify existing ones)'), (11, 'planning_manage', 'Manage budget plannings'), (11, 'order_manage', 'Manage orders & basket'), + (11, 'order_manage_all', 'Manage all orders & baskets'), (11, 'group_manage', 'Manage orders & basketgroups'), (11, 'order_receive', 'Manage orders & basket'), (11, 'budget_add_del', 'Add and delete budgets (but cant modify budgets)'), diff --git a/installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql b/installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql index 4b8c66e..1cff561 100644 --- a/installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql +++ b/installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql @@ -32,6 +32,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'budget_modify', 'Modifier les budgets (impossible de créer les lignes, mais possible de modifier celles qui existent'), (11, 'planning_manage', 'Gérer de la planification des budgets'), (11, 'order_manage', 'Gérer les commandes et les paniers'), + (11, 'order_manage_all', 'Gérer toutes les commandes et panier'), (11, 'group_manage', 'Gérer les commandes et les bons de commande'), (11, 'order_receive', 'Gérer les réceptions'), (11, 'budget_add_del', 'Ajouter et supprimer les budgets (mais pas modifier)'), diff --git a/installer/data/mysql/it-IT/necessari/userpermissions.sql b/installer/data/mysql/it-IT/necessari/userpermissions.sql index d0b5348..a35e23d 100644 --- a/installer/data/mysql/it-IT/necessari/userpermissions.sql +++ b/installer/data/mysql/it-IT/necessari/userpermissions.sql @@ -17,6 +17,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'budget_modify', 'Modifica budget (non li crea ma modifica gli esistenti)'), (11, 'planning_manage', 'Intervieni sulla pianificazione dei budgets'), (11, 'order_manage', 'Gestisci ordini e raccoglitori'), + (11, 'order_manage_all', 'Manage all orders & baskets'), (11, 'group_manage', 'Gestisci ordini e raccoglitori raggruppati'), (11, 'order_receive', 'Gestisci arrivi'), (11, 'budget_add_del', 'Aggiungi e cancella budgets (senza modificarli)'), diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index 69304e1..b03097a 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -2568,6 +2568,7 @@ CREATE TABLE `aqbasket` ( -- stores data about baskets in acquisitions `authorisedby` varchar(10) default NULL, -- the borrowernumber of the person who created the basket `booksellerinvoicenumber` mediumtext, -- appears to always be NULL `basketgroupid` int(11), -- links this basket to its group (aqbasketgroups.id) + branch varchar(10) default NULL, -- basket branch PRIMARY KEY (`basketno`), KEY `booksellerid` (`booksellerid`), KEY `basketgroupid` (`basketgroupid`), @@ -2575,6 +2576,20 @@ CREATE TABLE `aqbasket` ( -- stores data about baskets in acquisitions CONSTRAINT `aqbasket_ibfk_1` FOREIGN KEY (`booksellerid`) REFERENCES `aqbooksellers` (`id`) ON UPDATE CASCADE, CONSTRAINT `aqbasket_ibfk_2` FOREIGN KEY (`contractnumber`) REFERENCES `aqcontract` (`contractnumber`), CONSTRAINT `aqbasket_ibfk_3` FOREIGN KEY (`basketgroupid`) REFERENCES `aqbasketgroups` (`id`) ON UPDATE CASCADE + CONSTRAINT aqbasket_ibfk_4 FOREIGN KEY (branch) REFERENCES branches (branchcode) ON UPDATE CASCADE ON DELETE SET NULL, +) ENGINE=InnoDB DEFAULT CHARSET=utf8; + +-- +-- Table structure for table aqbasketusers +-- + +DROP TABLE IF EXISTS aqbasketusers; +CREATE TABLE aqbasketusers ( + basketno int(11) NOT NULL, + borrowernumber int(11) NOT NULL, + PRIMARY KEY (basketno,borrowernumber), + CONSTRAINT aqbasketusers_ibfk_1 FOREIGN KEY (basketno) REFERENCES aqbasket (basketno) ON UPDATE CASCADE ON DELETE CASCADE, + CONSTRAINT aqbasketusers_ibfk_2 FOREIGN KEY (borrowernumber) REFERENCES borrowers (borrowernumber) ON UPDATE CASCADE ON DELETE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8; -- diff --git a/installer/data/mysql/nb-NO/1-Obligatorisk/userpermissions.sql b/installer/data/mysql/nb-NO/1-Obligatorisk/userpermissions.sql index a00535f..9a2f021 100644 --- a/installer/data/mysql/nb-NO/1-Obligatorisk/userpermissions.sql +++ b/installer/data/mysql/nb-NO/1-Obligatorisk/userpermissions.sql @@ -36,6 +36,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'budget_modify', 'Endre budsjetter (kan ikke legge til kontolinjer, men endre eksisterende)'), (11, 'planning_manage', 'Administrere budsjettplaner'), (11, 'order_manage', 'Administrere bestillinger og kurver'), + (11, 'order_manage_all', 'Manage all orders & baskets'), (11, 'group_manage', 'Administrere bestillinger og kurv-grupper'), (11, 'order_receive', 'Administrere bestillinger og kurver'), (11, 'budget_add_del', 'Legge til og slette budsjetter (men ikke endre budsjetter)'), diff --git a/installer/data/mysql/pl-PL/mandatory/userpermissions.sql b/installer/data/mysql/pl-PL/mandatory/userpermissions.sql index 1165af1..76f1f5d 100644 --- a/installer/data/mysql/pl-PL/mandatory/userpermissions.sql +++ b/installer/data/mysql/pl-PL/mandatory/userpermissions.sql @@ -15,6 +15,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'budget_modify', 'Modify budget (can''t create lines, but can modify existing ones)'), (11, 'planning_manage', 'Manage budget plannings'), (11, 'order_manage', 'Manage orders & basket'), + (11, 'order_manage_all', 'Manage all orders & baskets'), (11, 'group_manage', 'Manage orders & basketgroups'), (11, 'order_receive', 'Manage orders & basket'), (11, 'budget_add_del', 'Add and delete budgets (but cant modify budgets)'), diff --git a/installer/data/mysql/ru-RU/mandatory/permissions_and_user_flags.sql b/installer/data/mysql/ru-RU/mandatory/permissions_and_user_flags.sql index 00d6538..268e48c 100644 --- a/installer/data/mysql/ru-RU/mandatory/permissions_and_user_flags.sql +++ b/installer/data/mysql/ru-RU/mandatory/permissions_and_user_flags.sql @@ -39,6 +39,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'budget_modify', 'Modify budget (can''t create lines, but can modify existing ones)'), (11, 'planning_manage', 'Manage budget plannings'), (11, 'order_manage', 'Manage orders & basket'), + (11, 'order_manage_all', 'Manage all orders & baskets'), (11, 'group_manage', 'Manage orders & basketgroups'), (11, 'order_receive', 'Manage orders & basket'), (11, 'budget_add_del', 'Add and delete budgets (but cant modify budgets)'), diff --git a/installer/data/mysql/uk-UA/mandatory/permissions_and_user_flags.sql b/installer/data/mysql/uk-UA/mandatory/permissions_and_user_flags.sql index 188a146..ce6a271 100644 --- a/installer/data/mysql/uk-UA/mandatory/permissions_and_user_flags.sql +++ b/installer/data/mysql/uk-UA/mandatory/permissions_and_user_flags.sql @@ -39,6 +39,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'budget_modify', 'Modify budget (can''t create lines, but can modify existing ones)'), (11, 'planning_manage', 'Manage budget plannings'), (11, 'order_manage', 'Manage orders & basket'), + (11, 'order_manage_all', 'Manage all orders & baskets'), (11, 'group_manage', 'Manage orders & basketgroups'), (11, 'order_receive', 'Manage orders & basket'), (11, 'budget_add_del', 'Add and delete budgets (but cant modify budgets)'), diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index 9977f63..566959c 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -5610,6 +5610,40 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) { SetVersion($DBversion); } +$DBversion = "XXX"; +if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { + $dbh->do(" + ALTER TABLE aqbasket ADD branch varchar(10) default NULL + "); + $dbh->do(" + ALTER TABLE aqbasket + ADD CONSTRAINT aqbasket_ibfk_4 FOREIGN KEY (branch) + REFERENCES branches (branchcode) + ON UPDATE CASCADE ON DELETE SET NULL + "); + $dbh->do(" + DROP TABLE IF EXISTS aqbasketusers + "); + $dbh->do(" + CREATE TABLE aqbasketusers ( + basketno int(11) NOT NULL, + borrowernumber int(11) NOT NULL, + PRIMARY KEY (basketno,borrowernumber), + CONSTRAINT aqbasketusers_ibfk_1 FOREIGN KEY (basketno) REFERENCES aqbasket (basketno) ON DELETE CASCADE ON UPDATE CASCADE, + CONSTRAINT aqbasketusers_ibfk_2 FOREIGN KEY (borrowernumber) REFERENCES borrowers (borrowernumber) ON DELETE CASCADE ON UPDATE CASCADE + ) ENGINE=InnoDB DEFAULT CHARSET=utf8; + "); + $dbh->do(" + INSERT INTO permissions (module_bit, code, description) + VALUES (11, 'order_manage_all', 'Manage all orders & baskets') + "); + + print "Upgrade to $DBversion done (Add branch and users list to baskets. " + . "New permission order_manage_all)\n"; + SetVersion($DBversion); +} + + =head1 FUNCTIONS =head2 TableExists($table) 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 new file mode 100644 index 0000000..9b7f52b --- /dev/null +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/aqbasketuser_search.tt @@ -0,0 +1,79 @@ +[% INCLUDE 'doc-head-open.inc' %] +Koha › Basket User Search +[% INCLUDE 'doc-head-close.inc' %] + + + + + + + +
+
+
+ +

Search for Basket User

+
+
+ + + +
+
Only staff with superlibrarian or acquisitions permissions (or order_manage permission if granular permissions are enabled) are returned in the search results
+
+ +[% IF (q) %] +

Searched for [% q %], [% nresults %] patron(s) found.

+[% END %] +[% IF ( users_loop ) %] + + + + + + + + + + + + [% FOREACH user IN users_loop %] + [% IF ( user.toggle ) %] + + [% ELSE %] + + [% END %] + + + + + + + [% END %] +
CardnumberNameBranchCategorycodeSelect?
[% user.cardnumber %][% user.surname %], [% user.firstname %][% user.branchcode %][% user.categorycode %] + Add +
+[% END %] + + +
+
+[% INCLUDE 'intranet-bottom.inc' %] 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 dcd6ccb..c062eeb 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt @@ -79,9 +79,42 @@ "sPaginationType": "four_button" } ) ); } ); + + function basketUserSearchPopup(f) { + window.open( + "/cgi-bin/koha/acqui/aqbasketuser_search.pl", + 'BasketUserSearchPopup', + 'width=740,height=450,toolbar=no,' + ); + } + + function add_basket_user(borrowernumber, borrowername) { + var ids = $("#basketusers_ids").val(); + if(ids.length > 0) { + ids = ids.split(':'); + } else { + ids = new Array; + } + if (ids.indexOf(borrowernumber) < 0) { + ids.push(borrowernumber); + $("#basketusers_ids").val(ids.join(':')); + var li = '
  • '+borrowername + + ' [' + + _('Delete user') + ']
  • '; + $("#basketusers_names").append(li); + return 0; + } + return -1; + } + + function del_basket_user(borrowernumber) { + $("#user_"+borrowernumber).remove(); + var ids = $("#basketusers_ids").val().split(':'); + ids.splice(ids.indexOf(borrowernumber.toString()), 1); + $("#basketusers_ids").val(ids.join(':')); + } //]]> - @@ -97,6 +130,11 @@
    + [% IF (cannot_manage_basket) %] +
    +

    You are not authorised to manage this basket.

    +
    + [% ELSE %]
    [% UNLESS ( confirm_close ) %] [% UNLESS ( selectbasketg ) %] @@ -192,7 +230,45 @@ [% IF ( basketcontractno ) %]
  • Contract name: [% basketcontractname %]
  • [% END %] - [% IF ( authorisedbyname ) %]
  • Managed by: [% authorisedbyname %]
  • [% END %] + [% IF ( authorisedbyname ) %]
  • Created by: [% authorisedbyname %]
  • [% END %] +
  • +
    + Managed by: +
    +
      + [% FOREACH user IN basketusers %] +
    • + [% user.firstname %] [% user.surname %] + [Delete user] +
    • + [% END %] +
    + + + + + +
    +
    +
  • +
  • +
    + Branch: + + + + +
    +
  • [% IF ( creationdate ) %]
  • Opened on: [% creationdate | $KohaDates %]
  • [% END %] [% IF ( closedate ) %]
  • Closed on: [% closedate | $KohaDates %]
  • [% END %] [% IF ( estimateddeliverydate ) %]
  • Estimated delivery date: [% estimateddeliverydate | $KohaDates %]
  • [% END %] @@ -433,6 +509,7 @@
    [% END %]
    +[% END %][%# IF (cannot_manage_basket) %]
    [% INCLUDE 'acquisitions-menu.inc' %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref index ebae997..f28ed6a 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref @@ -22,7 +22,7 @@ Acquisitions: - Show baskets - pref: AcqViewBaskets choices: - user: created by staff member. + user: created or managed by staff member. branch: from staff member's library. all: in system, regardless of owner. - diff --git a/t/Acquisition/CanUserManageBasket.t b/t/Acquisition/CanUserManageBasket.t new file mode 100644 index 0000000..e1855c3 --- /dev/null +++ b/t/Acquisition/CanUserManageBasket.t @@ -0,0 +1,204 @@ +#!/usr/bin/perl + +use Modern::Perl; +use Test::MockModule; +use Test::More tests => 42; + +use C4::Acquisition; + +my $C4_Acquisition_module = new Test::MockModule('C4::Acquisition'); +$C4_Acquisition_module->mock('GetBasketUsers', \&Mock_GetBasketUsers); +my $C4_Context_module = new Test::MockModule('C4::Context'); +$C4_Context_module->mock('preference', \&Mock_preference); + +my $AcqViewBaskets; +my %basketusers = ( + 1 => [], + 2 => [1, 2], + 3 => [], + 4 => [1, 2], +); + +# Borrowers +my $borrower1 = { + borrowernumber => 1, + branchcode => 'B1', +}; + +my $borrower2 = { + borrowernumber => 2, + branchcode => 'B2', +}; + +# Baskets +my $basket1 = { + basketno => 1, + authorisedby => 1 +}; + +my $basket2 = { + basketno => 2, + authorisedby => 2, +}; + +my $basket3 = { + basketno => 3, + authorisedby => 3, + branch => 'B1' +}; + +my $basket4 = { + basketno => 4, + authorisedby => 4, + branch => 'B1' +}; + + +my $flags = { + acquisition => { + order_manage => 1 + } +}; + +################# +# Start testing # +################# + +# ---------------------- +# AcqViewBaskets = 'all' +# ---------------------- + +$AcqViewBaskets = 'all'; + +# Simple cases where user can't manage basket +ok( not CanUserManageBasket($borrower1, $basket1, {}) ); +ok( not CanUserManageBasket($borrower1, $basket1, { + acquisition => 0 +}) ); +ok( not CanUserManageBasket($borrower1, $basket1, { + acquisition => { + order_manage => 0 + } +}) ); + +# Simple cases where user can manage basket +ok( CanUserManageBasket($borrower1, $basket1, { + superlibrarian => 1 +}) ); +ok( CanUserManageBasket($borrower1, $basket1, { + acquisition => 1 +}) ); +ok( CanUserManageBasket($borrower1, $basket1, { + acquisition => { + order_manage_all => 1 + } +}) ); + +ok( CanUserManageBasket($borrower1, $basket1, $flags) ); +ok( CanUserManageBasket($borrower1, $basket2, $flags) ); +ok( CanUserManageBasket($borrower1, $basket3, $flags) ); +ok( CanUserManageBasket($borrower1, $basket4, $flags) ); +ok( CanUserManageBasket($borrower2, $basket1, $flags) ); +ok( CanUserManageBasket($borrower2, $basket2, $flags) ); +ok( CanUserManageBasket($borrower2, $basket3, $flags) ); +ok( CanUserManageBasket($borrower2, $basket4, $flags) ); + +# ------------------------- +# AcqViewBaskets = 'branch' +# ------------------------- + +$AcqViewBaskets = 'branch'; + +# Simple cases where user can't manage basket +ok( not CanUserManageBasket($borrower1, $basket1, {}) ); +ok( not CanUserManageBasket($borrower1, $basket1, { + acquisition => 0 +}) ); +ok( not CanUserManageBasket($borrower1, $basket1, { + acquisition => { + order_manage => 0 + } +}) ); + +# Simple cases where user can manage basket +ok( CanUserManageBasket($borrower1, $basket1, { + superlibrarian => 1 +}) ); +ok( CanUserManageBasket($borrower1, $basket1, { + acquisition => 1 +}) ); +ok( CanUserManageBasket($borrower1, $basket1, { + acquisition => { + order_manage_all => 1 + } +}) ); + +ok( CanUserManageBasket($borrower1, $basket1, $flags) ); +ok( CanUserManageBasket($borrower1, $basket2, $flags) ); +ok( CanUserManageBasket($borrower1, $basket3, $flags) ); +ok( CanUserManageBasket($borrower1, $basket4, $flags) ); +ok( CanUserManageBasket($borrower2, $basket1, $flags) ); +ok( CanUserManageBasket($borrower2, $basket2, $flags) ); +# borrower2 is not on the same branch as basket3 and basket4 +ok( not CanUserManageBasket($borrower2, $basket3, $flags) ); +ok( not CanUserManageBasket($borrower2, $basket4, $flags) ); + +# ----------------------- +# AcqViewBaskets = 'user' +# ----------------------- + +$AcqViewBaskets = 'user'; + +# Simple cases where user can't manage basket +ok( not CanUserManageBasket($borrower1, $basket1, {}) ); +ok( not CanUserManageBasket($borrower1, $basket1, { + acquisition => 0 +}) ); +ok( not CanUserManageBasket($borrower1, $basket1, { + acquisition => { + order_manage => 0 + } +}) ); + +# Simple cases where user can manage basket +ok( CanUserManageBasket($borrower1, $basket1, { + superlibrarian => 1 +}) ); +ok( CanUserManageBasket($borrower1, $basket1, { + acquisition => 1 +}) ); +ok( CanUserManageBasket($borrower1, $basket1, { + acquisition => { + order_manage_all => 1 + } +}) ); + +ok( CanUserManageBasket($borrower1, $basket1, $flags) ); +ok( CanUserManageBasket($borrower1, $basket2, $flags) ); +# basket3 is not managed or created by borrower1 +ok( not CanUserManageBasket($borrower1, $basket3, $flags) ); +ok( CanUserManageBasket($borrower1, $basket4, $flags) ); +# basket 1 is not managed or created by borrower2 +ok( not CanUserManageBasket($borrower2, $basket1, $flags) ); +ok( CanUserManageBasket($borrower2, $basket2, $flags) ); +# basket 3 is not managed or created by borrower2 +ok( not CanUserManageBasket($borrower2, $basket3, $flags) ); +ok( CanUserManageBasket($borrower2, $basket4, $flags) ); + + +# Mocked subs + +# C4::Acquisition::GetBasketUsers +sub Mock_GetBasketUsers { + my ($basketno) = @_; + + return @{ $basketusers{$basketno} }; +} + +# C4::Context->preference +sub Mock_preference { + my ($self, $variable) = @_; + if (lc($variable) eq 'acqviewbaskets') { + return $AcqViewBaskets; + } +} -- 1.7.10.4