Attachment 18944 Details for Bug 10258 – Bug 10258: fix permissions check for setting basket group for order basket

[patch] Bug 10258: fix permissions check for setting basket group for order basket

Bug-10258-fix-permissions-check-for-setting-basket.patch (text/plain), 6.36 KB, created by Galen Charlton on 2013-06-13 16:16:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Galen Charlton

Created: 2013-06-13 16:16:34 UTC

Size: 6.36 KB

patch

obsolete

>From d6242e2c607206efc7a7abd650cc669f05c6585e Mon Sep 17 00:00:00 2001
>From: Galen Charlton <gmc@esilibrary.com>
>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 <gmc@esilibrary.com>
>---
> acqui/basket.pl                                    |   14 +++++---------
> .../intranet-tmpl/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 7eff6e4..06d4d1a 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 96d2dee..7f9ee31 100644
>--- 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>
>-- 
>1.7.2.5

Actions: View | Diff | Splinter Review

Attachments on bug 10258: 18166 | 18592 | 18736 | 18853 | 18944 | 19039 | 19053 | 19054 | 19648 | 19649