From ceda49dd1c376d8d6063d5be20d5bd72947eee35 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 23 Aug 2012 13:53:21 +0200 Subject: [PATCH] [PASSED QA] 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 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 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. --- C4/Serials.pm | 13 +++++++++ .../prog/en/modules/serials/claims.tt | 6 +++- serials/claims.pl | 4 +++ t/db_dependent/Serials_2.t | 32 +++++++++++++++++++++- 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/C4/Serials.pm b/C4/Serials.pm index a157460..1d7c7c0 100644 --- 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 --- 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 @@ [% FOREACH missingissue IN missingissues %] - + + [% UNLESS missingissue.cannot_claim %] + + [% END %] + [% missingissue.name %] [% Branches.GetName( missingissue.branchcode ) %] diff --git a/serials/claims.pl b/serials/claims.pl index ce560ae..6a71cc8 100755 --- 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 --- 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 ); -- 1.9.1