Bug 8438: Users can only claim for serials related to their branch.

Only superlibrarian users and users with superserials permission
can override this limitation.

This patch adds a new subroutine C4::Serials::can_claim_subscription.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

The upgrading of the DB was as required:
for each [PT/S] patron with the permission "claim_serials" ON,
the permission "superserials" became[/was kept to] ON.

Then, after having checked the DB upgrading, to test the currently
adding limitation:
> Users can only claim for serials related to their branch
, I reset 2 PT/S patrons-users to:
permission claim_serials: YES
permission superserials: NO
and I set the syspref "IndependendeBranches" to "Prevent".

For:
> Only superlibrarian users can override this limitation.
the S patron-user could list AND claim:
A) subscriptions of his own branch,
B) subscriptions of other branch,
C) subscriptions without branch.

For:
> Only users with superserials permission can override this limitation.
the PT patron-user could list:
A) subscriptions of his own branch,
B) subscriptions of other branch,
C) subscriptions without branch,
and could claim only:
A) subscriptions of his own branch,
C) subscriptions without branch.

NB: a subscription is selected to be claimed.

Then I set the syspref "IndependendeBranches" to "Don't prevent".
The behaviour was [exactly the same as in master] without the added
limitation.

On [S/PT] patrons, if permission claim_serials was NO, no Claims
link was available on Serials' page, either under "Prevent" or under
"Don't prevent".

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Added the results of Paola's testing to the commit message
as test plan.

Note to RM: Maybe we could add a note to the release notes, that
Koha now enforces superserials with independent branches better,
so people might have to adapt permissions in order to claim
for other branches.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

diff --git a/C4/Serials.pm b/C4/Serials.pm
index a157460..1d7c7c0 100644 (file)
--- a/C4/Serials.pm
+++ b/C4/Serials.pm
@@ -2718,6 +2718,19 @@ sub subscriptionCurrentlyOnOrder {
     return $sth->fetchrow_array;
 }
 
+=head2 can_claim_subscription
+
+    $can = can_claim_subscription( $subscriptionid[, $userid] );
+
+Return 1 if the subscription can be claimed by the current logged user (or a given $userid), else 0.
+
+=cut
+
+sub can_claim_subscription {
+    my ( $subscription, $userid ) = @_;
+    return _can_do_on_subscription( $subscription, $userid, 'claim_serials' );
+}
+
 =head2 can_edit_subscription
 
     $can = can_edit_subscription( $subscriptionid[, $userid] );

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt
index 7f90e4c..dd77a73 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/serials/claims.tt
@@ -230,7 +230,11 @@
                 </tfoot>
                 <tbody>[% FOREACH missingissue IN missingissues %]
                     <tr>
-                        <td><input type="checkbox" name="serialid" value="[% missingissue.serialid %]" /></td>
+                        <td>
+                          [% UNLESS missingissue.cannot_claim %]
+                            <input type="checkbox" name="serialid" value="[% missingissue.serialid %]" />
+                          [% END %]
+                        </td>
                         <td>[% missingissue.name %]</td>
                         <td>
                             <span class="branch-[% missingissue.branchcode %]">[% Branches.GetName( missingissue.branchcode ) %]</span>

diff --git a/serials/claims.pl b/serials/claims.pl
index ce560ae..6a71cc8 100755 (executable)
--- a/serials/claims.pl
+++ b/serials/claims.pl
@@ -94,6 +94,10 @@ my $letters = GetLetters({ module => 'claimissues' });
 my @missingissues;
 if ($supplierid) {
     @missingissues = GetLateOrMissingIssues($supplierid);
+    foreach my $issue (@missingissues) {
+        $issue->{cannot_claim} = 1
+          unless C4::Serials::can_claim_subscription($issue);
+    }
 }
 
 $template->param(

diff --git a/t/db_dependent/Serials_2.t b/t/db_dependent/Serials_2.t
index 0a233fc..1f192ed 100644 (file)
--- a/t/db_dependent/Serials_2.t
+++ b/t/db_dependent/Serials_2.t
@@ -1,7 +1,7 @@
 #!/usr/bin/perl
 use Modern::Perl;
 
-use Test::More tests => 36;
+use Test::More tests => 46;
 
 use MARC::Record;
 
@@ -65,6 +65,7 @@ my $subscriptionid_from_another_branch = NewSubscription(
 
 my $subscription_from_my_branch = GetSubscription( $subscriptionid_from_my_branch );
 is( C4::Serials::can_edit_subscription($subscription_from_my_branch), 0, "cannot edit a subscription without userenv set");
+is( C4::Serials::can_claim_subscription($subscription_from_my_branch), 0, "cannot edit a subscription without userenv set");
 
 my $userid = 'my_userid';
 my $borrowernumber = C4::Members::AddMember(
@@ -80,6 +81,7 @@ $userenv = { flags => 1, id => $borrowernumber, branch => '' };
 # Can edit a subscription
 
 is( C4::Serials::can_edit_subscription($subscription_from_my_branch), 1, "User can edit a subscription with an empty branchcode");
+is( C4::Serials::can_claim_subscription($subscription_from_my_branch), 1, "User can edit a subscription with an empty branchcode");
 
 my $subscription_from_another_branch = GetSubscription( $subscriptionid_from_another_branch );
 
@@ -101,6 +103,13 @@ is( C4::Serials::can_show_subscription($subscription_from_my_branch), 1,
 is( C4::Serials::can_show_subscription($subscription_from_another_branch), 1,
 "With IndependentBranches, superlibrarian can show a subscription from another branch"
 );
+is( C4::Serials::can_claim_subscription($subscription_from_my_branch), 1,
+"With IndependentBranches, superlibrarian can claim a subscription from his branch"
+);
+is( C4::Serials::can_claim_subscription($subscription_from_another_branch), 1,
+"With IndependentBranches, superlibrarian can claim a subscription from another branch"
+);
+
 
 set_flags( 'superserials', $borrowernumber );
 is( C4::Serials::can_edit_subscription($subscription_from_my_branch), 1,
@@ -115,6 +124,13 @@ is( C4::Serials::can_show_subscription($subscription_from_my_branch), 1,
 is( C4::Serials::can_show_subscription($subscription_from_another_branch), 1,
 "With IndependentBranches, superserials can show a subscription from another branch"
 );
+is( C4::Serials::can_claim_subscription($subscription_from_my_branch), 1,
+"With IndependentBranches, superserials can claim a subscription from his branch"
+);
+is( C4::Serials::can_claim_subscription($subscription_from_another_branch), 1,
+"With IndependentBranches, superserials can claim a subscription from another branch"
+);
+
 
 
 set_flags( 'edit_subscription', $borrowernumber );
@@ -130,6 +146,13 @@ is( C4::Serials::can_show_subscription($subscription_from_my_branch), 1,
 is( C4::Serials::can_show_subscription($subscription_from_another_branch), 0,
 "With IndependentBranches, show_subscription cannot show a subscription from another branch"
 );
+is( C4::Serials::can_claim_subscription($subscription_from_my_branch), 0,
+"With IndependentBranches, claim_subscription cannot claim a subscription from his branch with the edit_subscription permission"
+);
+is( C4::Serials::can_claim_subscription($subscription_from_another_branch), 0,
+"With IndependentBranches, claim_subscription cannot claim a subscription from another branch"
+);
+
 
 set_flags( 'renew_subscription', $borrowernumber );
 is( C4::Serials::can_edit_subscription($subscription_from_my_branch), 0,
@@ -145,6 +168,13 @@ is( C4::Serials::can_show_subscription($subscription_from_another_branch), 0,
 "With IndependentBranches, renew_subscription cannot show a subscription from another branch"
 );
 
+set_flags( 'claim_serials', $borrowernumber );
+is( C4::Serials::can_claim_subscription($subscription_from_my_branch), 1,
+"With IndependentBranches, claim_subscription can claim a subscription from his branch with the edit_subscription permission"
+);
+is( C4::Serials::can_claim_subscription($subscription_from_another_branch), 0,
+"With IndependentBranches, claim_subscription cannot claim a subscription from another branch"
+);
 
 # Branches are not independent
 t::lib::Mocks::mock_preference( "IndependentBranches", 0 );