From df3ad7a1460206d93e1107dbac762c4c267d2ac0 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Wed, 16 Apr 2014 16:58:36 +0200 Subject: [PATCH] [PASSED QA] Bug 12098: Refactor can_*_subscription in C4::Serials http://bugs.koha-community.org/show_bug.cgi?id=12098 Signed-off-by: Kyle M Hall Signed-off-by: Katrin Fischer Tested on top of patches for 12048 and 12080. Subscription search - superlibrarian, IndyBranches on/off - always sees all subscriptions - superserials, IndyBranches on/off - always sees all subscriptions - no superserials, IndyBranches on - only sees own subscriptions Note: Subscriptions without branches will only show, when all subscriptions are visible. In a future enh it might be good to enforce setting a branch, when IndyBranches is used. - no superserials, IndyBranches off - always sees all subscriptions Subscription editing - superlibrarian, IndyBranches on/off - can edit all subscriptions - superserials, IndyBranches on/off - can edit all subscriptions - no superserials, IndyBranches on - can only edit own subscriptons and subscriptions without branch NOTE: it would make sense to also allow Edit > Edit as new (duplicate) here, so one can copy the subscription from another branch to modify it for the own branch. Passes tests in t, xt and QA script, also newly provided unit tests. --- C4/Serials.pm | 49 +++++++++++++++++-------------------------------- 1 file changed, 17 insertions(+), 32 deletions(-) diff --git a/C4/Serials.pm b/C4/Serials.pm index 892996f..98d52f0 100644 --- a/C4/Serials.pm +++ b/C4/Serials.pm @@ -2840,45 +2840,30 @@ sub subscriptionCurrentlyOnOrder { $can = can_edit_subscription( $subscriptionid[, $userid] ); -Return 1 if the subscription is editable by the current logged user (or a given $userid), else 0. +Return 1 if the subscription can be edited by the current logged user (or a given $userid), else 0. =cut sub can_edit_subscription { my ( $subscription, $userid ) = @_; - return 0 unless C4::Context->userenv; - my $flags = C4::Context->userenv->{flags}; - $userid ||= C4::Context->userenv->{'id'}; - - if ( C4::Context->preference('IndependentBranches') ) { - return 1 - if C4::Context->IsSuperLibrarian() - or - C4::Auth::haspermission( $userid, { serials => 'superserials' } ) - or ( - C4::Auth::haspermission( $userid, - { serials => 'edit_subscription' } ) - and ( not defined $subscription->{branchcode} - or $subscription->{branchcode} eq '' - or $subscription->{branchcode} eq - C4::Context->userenv->{'branch'} ) - ); - } - else { - return 1 - if C4::Context->IsSuperLibrarian() - or - C4::Auth::haspermission( $userid, { serials => 'superserials' } ) - or C4::Auth::haspermission( - $userid, { serials => 'edit_subscription' } - ), - ; - } - return 0; + return _can_do_on_subscription( $subscription, $userid, 'edit_subscription' ); } +=head2 can_show_subscription + + $can = can_show_subscription( $subscriptionid[, $userid] ); + +Return 1 if the subscription can be shown by the current logged user (or a given $userid), else 0. + +=cut + sub can_show_subscription { my ( $subscription, $userid ) = @_; + return _can_do_on_subscription( $subscription, $userid, '*' ); +} + +sub _can_do_on_subscription { + my ( $subscription, $userid, $permission ) = @_; return 0 unless C4::Context->userenv; my $flags = C4::Context->userenv->{flags}; $userid ||= C4::Context->userenv->{'id'}; @@ -2890,7 +2875,7 @@ sub can_show_subscription { C4::Auth::haspermission( $userid, { serials => 'superserials' } ) or ( C4::Auth::haspermission( $userid, - { serials => '*' } ) + { serials => $permission } ) and ( not defined $subscription->{branchcode} or $subscription->{branchcode} eq '' or $subscription->{branchcode} eq @@ -2903,7 +2888,7 @@ sub can_show_subscription { or C4::Auth::haspermission( $userid, { serials => 'superserials' } ) or C4::Auth::haspermission( - $userid, { serials => '*' } + $userid, { serials => $permission } ), ; } -- 1.8.3.2