From 6608598576ec6a222442efec1b33226f95408f95 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 | 53 ++++- 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 | 33 ++++ .../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, 699 insertions(+), 29 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 4a9eae6..cc4ec66 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 @@ -156,8 +159,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=? @@ -603,6 +605,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 f203af2..ef4295b 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{}; @@ -190,6 +203,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") ) { @@ -205,6 +233,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 ($basketgroup, $basketgroups); my $member = GetMember(borrowernumber => $loggedinuser); @@ -241,6 +280,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(); @@ -320,9 +366,12 @@ if ( $op eq 'delete_confirm' ) { 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, deliveryplace => C4::Branch::GetBranchName( $basket->{deliveryplace} ), diff --git a/acqui/booksellers.pl b/acqui/booksellers.pl index f6c7b2e..b61a3fd 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', @@ -97,11 +97,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 = []; @@ -111,21 +106,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 f203a8d..0f47c6b 100644 --- a/installer/data/mysql/de-DE/mandatory/userpermissions.sql +++ b/installer/data/mysql/de-DE/mandatory/userpermissions.sql @@ -16,6 +16,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 8577ef2..00096d3 100644 --- a/installer/data/mysql/en/mandatory/userpermissions.sql +++ b/installer/data/mysql/en/mandatory/userpermissions.sql @@ -16,6 +16,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 8577ef2..00096d3 100644 --- a/installer/data/mysql/es-ES/mandatory/userpermissions.sql +++ b/installer/data/mysql/es-ES/mandatory/userpermissions.sql @@ -16,6 +16,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 3c57a81..01ea1bd 100644 --- a/installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql +++ b/installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql @@ -33,6 +33,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 31e6e18..cebb5f2 100644 --- a/installer/data/mysql/it-IT/necessari/userpermissions.sql +++ b/installer/data/mysql/it-IT/necessari/userpermissions.sql @@ -18,6 +18,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 221c4e4..e12d7e0 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -2601,6 +2601,7 @@ CREATE TABLE `aqbasket` ( -- stores data about baskets in acquisitions `basketgroupid` int(11), -- links this basket to its group (aqbasketgroups.id) `deliveryplace` varchar(10) default NULL, -- basket delivery place `billingplace` varchar(10) default NULL, -- basket billing place + branch varchar(10) default NULL, -- basket branch PRIMARY KEY (`basketno`), KEY `booksellerid` (`booksellerid`), KEY `basketgroupid` (`basketgroupid`), @@ -2608,6 +2609,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 7977865..c4dcd0b 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 02b2551..ce763d6 100644 --- a/installer/data/mysql/pl-PL/mandatory/userpermissions.sql +++ b/installer/data/mysql/pl-PL/mandatory/userpermissions.sql @@ -16,6 +16,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 868c3c0..4dd32da 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 @@ -40,6 +40,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 8d0126b..b508acf 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 @@ -40,6 +40,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 f0a9269..d12dad3 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -6532,6 +6532,39 @@ if (C4::Context->preference("Version") < TransformToNum($DBversion)) { SetVersion ($DBversion); } +$DBversion = "3.11.00.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 386d699..50c6351 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt @@ -105,9 +105,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(':')); + } //]]> - @@ -123,6 +156,11 @@
    + [% IF (cannot_manage_basket) %] +
    +

    You are not authorised to manage this basket.

    +
    + [% ELSE %]
    [% UNLESS ( confirm_close ) %] [% UNLESS ( selectbasketg ) %] @@ -222,7 +260,45 @@ [% END %] [% IF ( deliveryplace ) %]
  • Delivery place: [% deliveryplace %]
  • [% END %] [% IF ( billingplace ) %]
  • Billing place: [% billingplace %]
  • [% 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 %] @@ -494,6 +570,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 1ec49ff..105193f 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