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 <gmc@esilibrary.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Galen Charlton <gmc@esilibrary.com>

diff --git a/acqui/basket.pl b/acqui/basket.pl
index 7eff6e4..06d4d1a 100755 (executable)
--- 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 96d2dee..7f9ee31 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
@@ -203,15 +203,21 @@
                 </div>
                 [% IF ( closedate ) %]
                 <div class="yui-u">
+                    [% IF ( CAN_user_acquisition_group_manage ) %]
                     <form action="/cgi-bin/koha/acqui/basketgroup.pl" method="post">
+                    [% END %]
                         <ol>
                         <li>
                           <span class="label">Basket group:</span>
                           [% IF basketgroup.closed %]
                             [% basketgroup.name %] (closed)
+                          [% ELSIF ( ! CAN_user_acquisition_group_manage ) %]
+                            [%- IF basketgroup.name -%]
+                                [% basketgroup.name %]
+                            [%- ELSE -%]
+                                No group
+                            [%- END -%]
                           [% ELSE %]
-                            </li>
-                            <li>
                             <select id="basketgroupid" name="basketgroupid">
                               <option value="new">Add new group</option>
                               [% FOREACH bg IN basketgroups %]
@@ -232,10 +238,12 @@
                             <input type="submit" value="Change basket group" />
                           [% END %]
                         </li>
-                    [% IF ( basketgroupdeliveryplace ) %]<li><span class="label">Basket group delivery place:</span> [% basketgroupdeliveryplace %]</li>[% END %]
-                    [% IF ( basketgroupbillingplace ) %]<li><span class="label">Basket group billing place:</span> [% basketgroupbillingplace %]</li>[% END %]
+                    [% IF ( basketgroup.deliveryplacename ) %]<li><span class="label">Basket group delivery placename:</span> [% basketgroup.deliveryplacename %]</li>[% END %]
+                    [% IF ( basketgroup.billingplacename ) %]<li><span class="label">Basket group billing place:</span> [% basketgroup.billingplacename %]</li>[% END %]
                         </ol>
+                    [% IF ( CAN_user_acquisition_group_manage ) %]
                     </form>
+                    [% END %]
                 </div>
                 [% END %]
             </div>