From 3d26fa55e351949491e0f68993dfd53ae2780ee6 Mon Sep 17 00:00:00 2001 From: Julian Maurice Date: Wed, 5 Feb 2014 17:13:31 +0100 Subject: [PATCH 1/3] Bug 7290: More granular permissions for order receiving Add subpermission acquisition => order_receive_all Users with order_receive_all can receive all orders. Users with only order_receive can receive an order if: - basket branch is defined and is the same as the current working branch, or - basket branch is not defined Use dataTables filters possibilities instead of custom JS solution in parcel.pl Unit tests are available in t/Acquisition/CanUserReceiveBasket.t Signed-off-by: Paola Rossi --- C4/Acquisition.pm | 68 ++++++++++++++++++ acqui/finishreceive.pl | 10 ++- acqui/neworderempty.pl | 5 +- acqui/orderreceive.pl | 12 ++++ acqui/parcel.pl | 12 +++- .../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 + .../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 | 20 ++++++ .../prog/en/modules/acqui/orderreceive.tt | 5 ++ .../intranet-tmpl/prog/en/modules/acqui/parcel.tt | 26 +++++-- t/Acquisition/CanUserReceiveBasket.t | 74 ++++++++++++++++++++ 18 files changed, 230 insertions(+), 11 deletions(-) create mode 100644 t/Acquisition/CanUserReceiveBasket.t diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm index 15f69c0..dc9e244 100644 --- a/C4/Acquisition.pm +++ b/C4/Acquisition.pm @@ -52,6 +52,7 @@ BEGIN { &GetBasketUsers &ModBasketUsers &CanUserManageBasket + &CanUserReceiveOrder &ModBasketHeader @@ -818,6 +819,73 @@ sub CanUserManageBasket { return 1; } +=head3 CanUserReceiveOrder + + my $bool = CanUserReceiveOrder($borrower, $order[, $userflags]); + my $bool = CanUserReceiveOrder($borrowernumber, $orderno[, $userflags]); + +Check if a borrower can receive a order, according to user permissions, current +working branch and basket branches. + +First parameter can be either a borrowernumber or a hashref as returned by +C4::Members::GetMember. + +Second parameter can be either a ordernumber or a hashref as returned by +C4::Acquisition::GetOrder. + +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 receive order, returns 1. +Otherwise returns 0. + +=cut + +sub CanUserReceiveOrder { + my ($borrower, $order, $userflags) = @_; + + if (!ref $borrower) { + $borrower = C4::Members::GetMember(borrowernumber => $borrower); + } + if (!ref $order) { + $order = GetOrder($order); + } + + return 0 unless ($borrower and $order); + + if (!defined $userflags) { + my $dbh = C4::Context->dbh; + my $sth = $dbh->prepare("SELECT flags FROM borrowers WHERE borrowernumber = ?"); + $sth->execute($borrower->{borrowernumber}); + my ($flags) = $sth->fetchrow_array; + $sth->finish; + + $userflags = C4::Auth::getuserflags($flags, $borrower->{userid}, $dbh); + } + + my $branch = C4::Context->userenv->{'branch'}; + + unless ($userflags->{superlibrarian} + || (!ref $userflags->{acquisition} && $userflags->{acquisition}) + || (ref $userflags->{acquisition} && $userflags->{acquisition}->{order_receive_all}) ) + { + if (!ref $userflags->{acquisition} && !$userflags->{acquisition}) { + return 0; + } + + if (ref $userflags->{acquisition} && !$userflags->{acquisition}->{order_receive}) { + return 0; + } + + my $basket = GetBasket($order->{basketno}); + if (!$basket || (defined $basket->{branch} && $basket->{branch} ne $branch)) { + return 0; + } + } + + return 1; +} + #------------------------------------------------------------# =head3 GetBasketsByBasketgroup diff --git a/acqui/finishreceive.pl b/acqui/finishreceive.pl index 286474f..cecc9cd 100755 --- a/acqui/finishreceive.pl +++ b/acqui/finishreceive.pl @@ -39,7 +39,8 @@ use List::MoreUtils qw/any/; my $input=new CGI; my $flagsrequired = {acquisition => 'order_receive'}; -checkauth($input, 0, $flagsrequired, 'intranet'); +my ($userid, $cookie, undef, $userflags) + = checkauth($input, 0, $flagsrequired, 'intranet'); my $user = $input->remote_user; my $biblionumber = $input->param('biblionumber'); @@ -61,6 +62,13 @@ my $bookfund = $input->param("bookfund"); my $order = GetOrder($ordernumber); my $new_ordernumber = $ordernumber; +my $borrower = C4::Members::GetMember(userid => $userid); +unless (CanUserReceiveOrder($borrower, $ordernumber, $userflags)) { + my $error = "cannot_receive_order"; + print $input->redirect("/cgi-bin/koha/acqui/parcel.pl?invoiceid=$invoiceid&error=$error"); + exit; +} + #need old recievedate if we update the order, parcel.pl only shows the right parcel this way FIXME if ($quantityrec > $origquantityrec ) { my @received_items = (); diff --git a/acqui/neworderempty.pl b/acqui/neworderempty.pl index 93c07c6..b350802 100755 --- a/acqui/neworderempty.pl +++ b/acqui/neworderempty.pl @@ -66,8 +66,7 @@ the item's id in the breeding reservoir =cut -use warnings; -use strict; +use Modern::Perl; use CGI qw ( -utf8 ); use C4::Context; use C4::Input; @@ -246,7 +245,7 @@ foreach my $thisbranch ( sort {$branches->{$a}->{'branchname'} cmp $branches->{$ value => $thisbranch, branchname => $branches->{$thisbranch}->{'branchname'}, ); - $row{'selected'} = 1 if( $thisbranch && $data->{branchcode} && $thisbranch eq $data->{branchcode}) ; + $row{'selected'} = 1 if( $thisbranch && $data->{branch} && $thisbranch eq $data->{branch}) ; push @branchloop, \%row; } $template->param( branchloop => \@branchloop ); diff --git a/acqui/orderreceive.pl b/acqui/orderreceive.pl index 96a6a02..6b79c64 100755 --- a/acqui/orderreceive.pl +++ b/acqui/orderreceive.pl @@ -115,6 +115,18 @@ unless ( $results and @$results) { # prepare the form for receiving my $order = $results->[0]; +unless (CanUserReceiveOrder($loggedinuser, $order, $userflags)) { + $template->param( + cannot_receive_order => 1, + ordernumber => $ordernumber, + invoice => $invoice, + booksellerid => $booksellerid, + name => $bookseller->{name} + ); + output_html_with_http_headers $input, $cookie, $template->output; + exit; +} + # Check if ACQ framework exists my $acq_fw = GetMarcStructure(1, 'ACQ'); unless($acq_fw) { diff --git a/acqui/parcel.pl b/acqui/parcel.pl index d37c8d9..25e9a9f 100755 --- a/acqui/parcel.pl +++ b/acqui/parcel.pl @@ -54,8 +54,7 @@ To filter the results list on this given date. =cut -use strict; -use warnings; +use Modern::Perl; use C4::Auth; use C4::Acquisition; @@ -75,7 +74,7 @@ use JSON; my $input=new CGI; my $sticky_filters = $input->param('sticky_filters') || 0; -my ($template, $loggedinuser, $cookie) +my ($template, $loggedinuser, $cookie, $userflags) = get_template_and_user({template_name => "acqui/parcel.tt", query => $input, type => "intranet", @@ -267,6 +266,8 @@ unless( defined $invoice->{closedate} ) { $line{holds} = $holds; $line{holds_on_order} = $itemholds?$itemholds:$holds if $line{left_holds_on_order}; + $line{can_receive} = CanUserReceiveOrder($loggedinuser, $pendingorders->[$i], $userflags); + my $budget_name = GetBudgetName( $line{budget_id} ); $line{budget_name} = $budget_name; @@ -278,6 +279,11 @@ unless( defined $invoice->{closedate} ) { ); } +my @errors = $input->param('error'); +foreach my $error (@errors) { + $template->param("error_$error" => 1); +} + $template->param( invoiceid => $invoice->{invoiceid}, invoice => $invoice->{invoicenumber}, diff --git a/installer/data/mysql/de-DE/mandatory/userpermissions.sql b/installer/data/mysql/de-DE/mandatory/userpermissions.sql index 0bfb082..d7239c7 100644 --- a/installer/data/mysql/de-DE/mandatory/userpermissions.sql +++ b/installer/data/mysql/de-DE/mandatory/userpermissions.sql @@ -25,6 +25,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'order_manage_all', 'Bestellungen aller Bibliotheken, unabhängig von Berechtigungen, verwalten'), (11, 'group_manage', 'Bestellgruppen vewalten'), (11, 'order_receive', 'Lieferungen verwalten'), + (11, 'order_receive_all', 'Receive all orders'), (11, 'budget_add_del', 'Konten hinzufügen/ändern, aber bestehende nicht ändern'), (11, 'budget_manage_all', 'Alle Konten verwalten'), (13, 'edit_news', 'Nachrichten für OPAC und Dienstoberfläche verfassen'), diff --git a/installer/data/mysql/en/mandatory/userpermissions.sql b/installer/data/mysql/en/mandatory/userpermissions.sql index 71993dc..72797ed 100644 --- a/installer/data/mysql/en/mandatory/userpermissions.sql +++ b/installer/data/mysql/en/mandatory/userpermissions.sql @@ -25,6 +25,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'order_manage_all', 'Manage all orders and baskets, regardless of restrictions on them'), (11, 'group_manage', 'Manage orders & basketgroups'), (11, 'order_receive', 'Manage orders & basket'), + (11, 'order_receive_all', 'Receive all orders'), (11, 'budget_add_del', 'Add and delete budgets (but can''t modify budgets)'), (11, 'budget_manage_all', 'Manage all budgets'), (13, 'edit_news', 'Write news for the OPAC and staff interfaces'), diff --git a/installer/data/mysql/es-ES/mandatory/userpermissions.sql b/installer/data/mysql/es-ES/mandatory/userpermissions.sql index f830d4e..2373bcb 100644 --- a/installer/data/mysql/es-ES/mandatory/userpermissions.sql +++ b/installer/data/mysql/es-ES/mandatory/userpermissions.sql @@ -25,6 +25,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'order_manage_all', 'Manage all orders and baskets, regardless of restrictions on them'), (11, 'group_manage', 'Manage orders & basketgroups'), (11, 'order_receive', 'Manage orders & basket'), + (11, 'order_receive_all', 'Receive all orders'), (11, 'budget_add_del', 'Add and delete budgets (but cant modify budgets)'), (11, 'budget_manage_all', 'Manage all budgets'), (13, 'edit_news', 'Write news for the OPAC and staff interfaces'), diff --git a/installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql b/installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql index bebf39d..70955b7 100644 --- a/installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql +++ b/installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql @@ -43,6 +43,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'order_manage_all', 'Manage all orders and baskets, regardless of restrictions on them'), (11, 'group_manage', 'Gérer les commandes et les bons de commande'), (11, 'order_receive', 'Gérer les réceptions'), + (11, 'order_receive_all', 'Gérer toutes les réceptions'), (11, 'budget_add_del', 'Ajouter et supprimer les budgets (mais pas modifier)'), (11, 'budget_manage_all', 'Gérer tous les budgets'), (13, 'manage_csv_profiles', 'Gérer les profils d''export CSV'), diff --git a/installer/data/mysql/it-IT/necessari/userpermissions.sql b/installer/data/mysql/it-IT/necessari/userpermissions.sql index 5e3647b..2d0cb53 100644 --- a/installer/data/mysql/it-IT/necessari/userpermissions.sql +++ b/installer/data/mysql/it-IT/necessari/userpermissions.sql @@ -25,6 +25,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'order_manage_all', 'Gestisci ordini e raccoglitori, indipendentemente dalle restrizioni su di loro'), (11, 'group_manage', 'Gestisci ordini e ordini d\'acquisto'), (11, 'order_receive', 'Gestisci arrivi'), + (11, 'order_receive_all', 'Receive all orders'), (11, 'budget_add_del', 'Aggiungi e cancella budgets (senza modificarli)'), (11, 'budget_manage_all', 'Gestisci tutti i budgets'), (13, 'edit_news', 'Scrivi le news per l\'OPAC e per l\'interfaccia staff'), diff --git a/installer/data/mysql/nb-NO/1-Obligatorisk/userpermissions.sql b/installer/data/mysql/nb-NO/1-Obligatorisk/userpermissions.sql index 24b96b9..959ad9a 100644 --- a/installer/data/mysql/nb-NO/1-Obligatorisk/userpermissions.sql +++ b/installer/data/mysql/nb-NO/1-Obligatorisk/userpermissions.sql @@ -45,6 +45,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'order_manage_all', 'Manage all orders and baskets, regardless of restrictions on them'), (11, 'group_manage', 'Administrere bestillinger og kurv-grupper'), (11, 'order_receive', 'Administrere bestillinger og kurver'), + (11, 'order_receive_all', 'Receive all orders'), (11, 'budget_add_del', 'Legge til og slette budsjetter (men ikke endre budsjetter)'), (11, 'budget_manage_all', 'Administrere alle budsjetter'), (13, 'edit_news', 'Legge ut nyhter i OPACen og det interne grensesnittet'), diff --git a/installer/data/mysql/pl-PL/mandatory/userpermissions.sql b/installer/data/mysql/pl-PL/mandatory/userpermissions.sql index a92709c..99a3898 100644 --- a/installer/data/mysql/pl-PL/mandatory/userpermissions.sql +++ b/installer/data/mysql/pl-PL/mandatory/userpermissions.sql @@ -25,6 +25,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'order_manage_all', 'Zarządzanie zamówieniami i koszykami, niezależnie od ich ustawień'), (11, 'group_manage', 'Zarządzanie zamówieniami i grupami koszyków'), (11, 'order_receive', 'Zarządzanie zamówieniami i koszykami'), + (11, 'order_receive_all', 'Receive all orders'), (11, 'budget_add_del', 'Dodawanie i usuwanie budżetów (bez modyfikacji)'), (11, 'budget_manage_all', 'Zarządzanie wszystkimi budżetami'), (13, 'edit_news', 'Tworzenie i publikowanie wiadomości w interfejsie bibliotekarza i OPAC'), 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 b746d1c..11ca45a 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 @@ -52,6 +52,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'order_manage_all', 'Manage all orders and baskets, regardless of restrictions on them'), (11, 'group_manage', 'Manage orders & basketgroups'), (11, 'order_receive', 'Manage orders & basket'), + (11, 'order_receive_all', 'Receive all orders'), (11, 'budget_add_del', 'Add and delete budgets (but cant modify budgets)'), (11, 'budget_manage_all', 'Manage all budgets'), (13, 'edit_news', 'Написание новостей для электронного каталога и интерфейса библиотекарей'), 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 37ca4f6..54b8576 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 @@ -52,6 +52,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (11, 'order_manage_all', 'Manage all orders and baskets, regardless of restrictions on them'), (11, 'group_manage', 'Manage orders & basketgroups'), (11, 'order_receive', 'Manage orders & basket'), + (11, 'order_receive_all', 'Receive all orders'), (11, 'budget_add_del', 'Add and delete budgets (but cant modify budgets)'), (11, 'budget_manage_all', 'Manage all budgets'), (13, 'edit_news', 'Написання новин для електронного каталогу та інтерфейсу бібліотекарів'), diff --git a/installer/data/mysql/updatedatabase.pl b/installer/data/mysql/updatedatabase.pl index ad8ec97..a1c5b37 100755 --- a/installer/data/mysql/updatedatabase.pl +++ b/installer/data/mysql/updatedatabase.pl @@ -9773,6 +9773,26 @@ if ( CheckVersion($DBversion) ) { SetVersion ($DBversion); } +$DBversion = "3.19.00.012"; +if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) { + $dbh->do(" + INSERT INTO permissions (module_bit, code, description) + VALUES (11, 'order_receive_all', 'Receive all orders') + "); + + # Add this new permission to all users that have 'order_receive' permission + $dbh->do(q{ + INSERT INTO user_permissions (borrowernumber, module_bit, code) + SELECT borrowernumber, 11, 'order_receive_all' + FROM user_permissions + WHERE code = 'order_receive' + GROUP BY borrowernumber + }); + + print "Upgrade to $DBversion done (Bug 7290: Add permission order_receive_all)\n"; + SetVersion($DBversion); +} + =head1 FUNCTIONS =head2 TableExists($table) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt index 833f0e6..7631387 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/orderreceive.tt @@ -129,6 +129,10 @@

Receive items from : [% name %] [% IF ( invoice ) %][[% invoice %]] [% END %] (order #[% ordernumber %])

+[% IF (cannot_receive_order) %] +

You are not authorised to receive this order.

+[% ELSE %] + [% IF ( count ) %]
@@ -345,6 +349,7 @@ This ordernumber does not exist. [% END %] +[% END %][%# IF (cannot_receive_order) %]
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt index a244f5f..2260046 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/parcel.tt @@ -151,11 +151,19 @@
[% IF ( receive_error ) %]
-

Error adding items:

+

Error receiving items:

    - [% FOREACH error_loo IN error_loop %] -
  • [% error_loo.error_param %][% IF ( error_loo.error_duplicate_barcode ) %]Duplicate Barcode[% END %]
  • - [% END %] + [% FOREACH error IN error_loop %] +
  • + [% error.error_param %] + [% IF ( error.error_duplicate_barcode ) %] + Duplicate Barcode + [% ELSIF (error.error_cannot_receive_order) %] + You were not authorised to receive this order + [% END %] + +
  • + [% END %]
[% END %] @@ -202,6 +210,12 @@
[% END %] + [% IF error_cannot_receive_order %] +
+ You cannot receive this order. +
+ [% END %] + [% UNLESS no_orders_to_display %]

Invoice number: [% invoice %] Received by: [% loggedinusername %] On: [% formatteddatereceived %]

@@ -280,7 +294,11 @@ [% loop_order.total | $Price %] [% loop_order.budget_name %] + [% IF (loop_order.can_receive) %] Receive + [% ELSE %] + Can't receive order + [% END %]
Transfer diff --git a/t/Acquisition/CanUserReceiveBasket.t b/t/Acquisition/CanUserReceiveBasket.t new file mode 100644 index 0000000..f0a7453 --- /dev/null +++ b/t/Acquisition/CanUserReceiveBasket.t @@ -0,0 +1,74 @@ +#!/usr/bin/perl + +use Modern::Perl; +use Test::MockModule; +use Test::More tests => 18; + +use C4::Acquisition; + +my $C4_Acquisition_module = new Test::MockModule('C4::Acquisition'); +$C4_Acquisition_module->mock('GetBasket', \&Mock_GetBasket); +my $C4_Context_module = new Test::MockModule('C4::Context'); +$C4_Context_module->mock('userenv', \&Mock_userenv); + +my $baskets = { + 1 => {}, + 2 => { + branch => 'B1' + }, + 3 => { + branch => 'B2' + } +}; + +my $userenv = {}; + +my $borrower = {}; + +my $order1 = { + basketno => 1 +}; +my $order2 = { + basketno => 2 +}; +my $order3 = { + basketno => 3 +}; + +$userenv->{branch} = 'B1'; + +ok( CanUserReceiveOrder($borrower, $order1, {superlibrarian => 1}) ); +ok( CanUserReceiveOrder($borrower, $order1, {acquisition => 1}) ); +ok( CanUserReceiveOrder($borrower, $order1, {acquisition => { order_receive_all => 1 }}) ); + +ok( not CanUserReceiveOrder($borrower, $order1, {}) ); +ok( not CanUserReceiveOrder($borrower, $order1, {acquisition => 0}) ); +ok( not CanUserReceiveOrder($borrower, $order1, {acquisition => { order_receive => 0 }}) ); + +ok( CanUserReceiveOrder($borrower, $order1, {acquisition => { order_receive => 1 }}) ); +ok( CanUserReceiveOrder($borrower, $order2, {acquisition => { order_receive => 1 }}) ); +ok( not CanUserReceiveOrder($borrower, $order3, {acquisition => { order_receive => 1 }}) ); + +$userenv->{branch} = 'B2'; + +ok( CanUserReceiveOrder($borrower, $order1, {superlibrarian => 1}) ); +ok( CanUserReceiveOrder($borrower, $order1, {acquisition => 1}) ); +ok( CanUserReceiveOrder($borrower, $order1, {acquisition => { order_receive_all => 1 }}) ); + +ok( not CanUserReceiveOrder($borrower, $order1, {}) ); +ok( not CanUserReceiveOrder($borrower, $order1, {acquisition => 0}) ); +ok( not CanUserReceiveOrder($borrower, $order1, {acquisition => { order_receive => 0 }}) ); + +ok( CanUserReceiveOrder($borrower, $order1, {acquisition => { order_receive => 1 }}) ); +ok( not CanUserReceiveOrder($borrower, $order2, {acquisition => { order_receive => 1 }}) ); +ok( CanUserReceiveOrder($borrower, $order3, {acquisition => { order_receive => 1 }}) ); + +sub Mock_GetBasket { + my ($basketno) = @_; + + return $baskets->{$basketno}; +} + +sub Mock_userenv { + return $userenv; +} -- 1.7.10.4