From 4c3032a22c9e9f713b95df38e990c451d7cf5506 Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Thu, 13 Jun 2013 08:53:49 -0700 Subject: [PATCH] Bug 10258: fix permissions check for setting basket group for order basket Improve the code that displays and allows staff to set the basket group from the basket details page for a closed basket. Prior to this patch, a staff member who did not have the group_manage acquisition permission would still see a control to change the group that the basket belongs to; attempting to change the group would present with with a login page. This patch also does some tidying of how basket group details are passed to the template. To test: [1] Create an order basket and close it. Do not assign it to a basket group. [2] View the basket details while logged in as a staff user who has the order_manage acquisitions permission but not the group_manage. The displayed basket group should be "No group". [3] Switch to a staff user who also has the group_manage permission, then view the basket details again. The basket group field should now be a select input that allows you to change the basket group. [4] Change the basket group. Verify that the basket group you selected is now displayed as the current group for that order basket. The basket group delivery and billing place fields should also now be displayed. [5] Close the basket group set in the previous step, then view the basket details again. This time, the basket group name should be displayed with a suffix of " (closed)", and no input to change the group should be displayed. [6] Swith to a staff user who does not have the group_manage permission, view the basket details, and verify that the basket name is displayed with a suffix of " (closed)". Signed-off-by: Galen Charlton Signed-off-by: Marcel de Rooy Signed-off-by: Galen Charlton (cherry picked from commit 44364db8d53bf5e3135ae2de6270a920e5c053c1) Signed-off-by: Tomas Cohen Arazi (cherry picked from commit dfbb9890d970f3f70cb6eb02275679d65101489a) Signed-off-by: Bernardo Gonzalez Kriegel (cherry picked from commit dfbb9890d970f3f70cb6eb02275679d65101489a) --- acqui/basket.pl | 14 +++++--------- .../prog/en/modules/acqui/basket.tt | 16 ++++++++++++---- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/acqui/basket.pl b/acqui/basket.pl index f203af2517..9dad355301 100755 --- a/acqui/basket.pl +++ b/acqui/basket.pl @@ -207,8 +207,8 @@ if ( $op eq 'delete_confirm' ) { } #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); - if ($basket->{closedate} && haspermission({ acquisition => 'group_manage'} )) { + my $staffuser = GetMember(borrowernumber => $loggedinuser); + if ($basket->{closedate} && haspermission($staffuser->{userid}, { acquisition => 'group_manage'} )) { $basketgroups = GetBasketgroups($basket->{booksellerid}); for my $bg ( @{$basketgroups} ) { if ($basket->{basketgroupid} && $basket->{basketgroupid} == $bg->{id}){ @@ -287,13 +287,9 @@ if ( $op eq 'delete_confirm' ) { my @orders = GetOrders($basketno); if ($basket->{basketgroupid}){ - my $basketgroup = GetBasketgroup($basket->{basketgroupid}); - for my $key (keys %$basketgroup ){ - $basketgroup->{"basketgroup$key"} = delete $basketgroup->{$key}; - } - $basketgroup->{basketgroupdeliveryplace} = C4::Branch::GetBranchName( $basketgroup->{basketgroupdeliveryplace} ); - $basketgroup->{basketgroupbillingplace} = C4::Branch::GetBranchName( $basketgroup->{basketgroupbillingplace} ); - $template->param(%$basketgroup); + $basketgroup = GetBasketgroup($basket->{basketgroupid}); + $basketgroup->{deliveryplacename} = C4::Branch::GetBranchName( $basketgroup->{deliveryplace} ); + $basketgroup->{billingplacename} = C4::Branch::GetBranchName( $basketgroup->{billingplace} ); } my $borrower= GetMember('borrowernumber' => $loggedinuser); my $budgets = GetBudgetHierarchy; 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 cd4321d0c9..e3eceffa61 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt @@ -231,15 +231,21 @@ [% IF ( closedate ) %]
+ [% IF ( CAN_user_acquisition_group_manage ) %]
+ [% END %]
  1. Basket group: [% IF basketgroup.closed %] [% basketgroup.name %] (closed) + [% ELSIF ( ! CAN_user_acquisition_group_manage ) %] + [%- IF basketgroup.name -%] + [% basketgroup.name %] + [%- ELSE -%] + No group + [%- END -%] [% ELSE %] -
    2. -
  2. [% END %]
    3. - [% IF ( basketgroupdeliveryplace ) %]
  3. Basket group delivery place: [% basketgroupdeliveryplace %]
    4. [% END %] - [% IF ( basketgroupbillingplace ) %]
  4. Basket group billing place: [% basketgroupbillingplace %]
    5. [% END %] + [% IF ( basketgroup.deliveryplacename ) %]
  5. Basket group delivery placename: [% basketgroup.deliveryplacename %]
    6. [% END %] + [% IF ( basketgroup.billingplacename ) %]
  6. Basket group billing place: [% basketgroup.billingplacename %]
    7. [% END %]
+ [% IF ( CAN_user_acquisition_group_manage ) %]
+ [% END %]
[% END %] -- 2.39.5