From 8c599edd6d7e5e8b03b05b7a0fcad02ccdf0c6d1 Mon Sep 17 00:00:00 2001 From: Kyle Hall Date: Thu, 23 Feb 2023 07:52:02 -0500 Subject: [PATCH] Bug 33070: Remove use of can_edit_items Test Plan: 1) Apply this patch 2) prove t/db_dependent/Koha/Acquisition/Order.t 3) git grep "can_edit_item(" should return no results Signed-off-by: Tomas Cohen Arazi --- Koha/Item.pm | 2 +- Koha/Patron.pm | 42 ++++--------------- Koha/UI/Table/Builder/Items.pm | 2 +- catalogue/detail.pl | 2 +- catalogue/moredetail.pl | 2 +- cataloguing/additem.pl | 4 +- .../catalogue/itemsearch_item.json.inc | 2 +- .../modules/course_reserves/course-details.tt | 2 +- t/db_dependent/Koha/Patrons.t | 16 +++---- 9 files changed, 25 insertions(+), 49 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index 89d1a948c45..16e32e91553 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -296,7 +296,7 @@ sub safe_to_delete { $error //= "not_same_branch" if defined C4::Context->userenv and defined C4::Context->userenv->{number} - and !Koha::Patrons->find( C4::Context->userenv->{number} )->can_edit_item( $self ); + and !Koha::Patrons->find( C4::Context->userenv->{number} )->can_edit_items_from( $self->homebranch ); # check it doesn't have a waiting reserve $error //= "book_reserved" diff --git a/Koha/Patron.pm b/Koha/Patron.pm index 9282b38307f..d3ec9154d8c 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -1510,51 +1510,24 @@ sub can_see_patrons_from { ); } -=head3 can_edit_item - -my $can_edit = $patron->can_edit_item( $item ); +=head3 can_edit_items_from -Return true if the patron (usually the logged in user) can edit the given item + my $can_edit = $patron->can_edit_items_from( $branchcode ); -The parameter can be a Koha::Item, an item hashref, or a branchcode. +Return true if the I can edit items from the given branchcode =cut -sub can_edit_item { - my ( $self, $item ) = @_; - - my $userenv = C4::Context->userenv(); - - my $ref = ref($item); - - my $branchcode = - $ref eq 'Koha::Item' ? $item->homebranch - : $ref eq 'HASH' ? $item->{homebranch} - : $ref eq q{} ? $item - : undef; - - return unless $branchcode; +sub can_edit_items_from { + my ( $self, $branchcode ) = @_; return 1 if C4::Context->IsSuperLibrarian(); + my $userenv = C4::Context->userenv(); if ( $userenv && C4::Context->preference('IndependentBranches') ) { return $userenv->{branch} eq $branchcode; } - return $self->can_edit_items_from($branchcode); -} - -=head3 can_edit_items_from - - my $can_edit = $patron->can_edit_items_from( $branchcode ); - -Return true if the I can edit items from the given branchcode - -=cut - -sub can_edit_items_from { - my ( $self, $branchcode ) = @_; - return $self->can_see_things_from( { branchcode => $branchcode, @@ -1622,10 +1595,13 @@ Return true if the I can perform some action on the given thing sub can_see_things_from { my ( $self, $params ) = @_; + my $branchcode = $params->{branchcode}; my $permission = $params->{permission}; my $subpermission = $params->{subpermission}; + return 1 if C4::Context->IsSuperLibrarian(); + my $can = 0; if ( $self->branchcode eq $branchcode ) { $can = 1; diff --git a/Koha/UI/Table/Builder/Items.pm b/Koha/UI/Table/Builder/Items.pm index a7230997f8f..b3e4181e297 100644 --- a/Koha/UI/Table/Builder/Items.pm +++ b/Koha/UI/Table/Builder/Items.pm @@ -89,7 +89,7 @@ sub build_table { holds => $item->biblio->holds->count, item_holds => $item->holds->count, is_checked_out => $item->checkout ? 1 : 0, - nomod => $patron ? !$patron->can_edit_item($item) : 0, + nomod => $patron ? !$patron->can_edit_items_from($item->homebranch) : 0, }; push @items, $item_info; } diff --git a/catalogue/detail.pl b/catalogue/detail.pl index 905120ec9d4..48bd7998796 100755 --- a/catalogue/detail.pl +++ b/catalogue/detail.pl @@ -442,7 +442,7 @@ foreach my $item (@items) { $item_info->{'course_reserves'} = GetItemCourseReservesInfo( itemnumber => $item->itemnumber ); } - $item_info->{can_be_edited} = $patron->can_edit_item( $item ); + $item_info->{can_be_edited} = $patron->can_edit_items_from( $item->homebranch ); if ( C4::Context->preference("LocalCoverImages") == 1 ) { $item_info->{cover_images} = $item->cover_images; diff --git a/catalogue/moredetail.pl b/catalogue/moredetail.pl index 46ac7f750a2..5dfcb5d3283 100755 --- a/catalogue/moredetail.pl +++ b/catalogue/moredetail.pl @@ -247,7 +247,7 @@ foreach my $item (@items){ } ); - $item_info->{nomod} = !$patron->can_edit_item( $item ); + $item_info->{nomod} = !$patron->can_edit_items_from( $item->homebranch ); push @item_data, $item_info; } diff --git a/cataloguing/additem.pl b/cataloguing/additem.pl index 586f89cd629..bbf801bdac0 100755 --- a/cataloguing/additem.pl +++ b/cataloguing/additem.pl @@ -161,7 +161,7 @@ my ($template, $loggedinuser, $cookie) my $patron = Koha::Patrons->find( $loggedinuser ); my $item = $itemnumber ? Koha::Items->find( $itemnumber ) : undef; -if ( $item && !$patron->can_edit_item( $item ) ) { +if ( $item && !$patron->can_edit_items_from( $item->homebranch ) ) { print $input->redirect("/cgi-bin/koha/catalogue/detail.pl?biblionumber=$biblionumber"); exit; } @@ -635,7 +635,7 @@ if ($op) { my @items; for my $item ( $biblio->items->as_list, $biblio->host_items->as_list ) { my $i = $item->columns_to_str; - $i->{nomod} = 1 unless $patron->can_edit_item($item); + $i->{nomod} = 1 unless $patron->can_edit_items_from($item->homebranch); push @items, $i; } diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/catalogue/itemsearch_item.json.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/catalogue/itemsearch_item.json.inc index 9d47e19193d..b306384ed1f 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/catalogue/itemsearch_item.json.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/catalogue/itemsearch_item.json.inc @@ -32,6 +32,6 @@ "[% (item.issues || 0) | html %]", "[% IF item.checkout %][% item.checkout.date_due | $KohaDates %][% END %]", "[% FILTER escape_quotes ~%] -
+
[%~ END %]" ] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt index 623974a4f39..cb109938a3b 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course-details.tt @@ -294,7 +294,7 @@ [% IF CAN_user_coursereserves_add_reserves || CAN_user_coursereserves_delete_reserves %] - [% IF CAN_user_coursereserves_add_reserves && user.can_edit_item( cr.item ) %] + [% IF CAN_user_coursereserves_add_reserves && user.can_edit_items_from( cr.item.homebranch ) %] Edit [% END %] diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t index 8684c8aa15e..12f00d02751 100755 --- a/t/db_dependent/Koha/Patrons.t +++ b/t/db_dependent/Koha/Patrons.t @@ -2460,7 +2460,7 @@ subtest 'filter_by_amount_owed' => sub { ); }; -subtest 'libraries_where_can_edit_items + can_edit_item' => sub { +subtest 'libraries_where_can_edit_items() and can_edit_items_from() tests' => sub { plan tests => 2; $schema->storage->txn_begin; @@ -2520,18 +2520,18 @@ subtest 'libraries_where_can_edit_items + can_edit_item' => sub { is_deeply( \@branchcodes, [$library_2A->branchcode], "patron_2A doesn't have edit_any_item => Can only see patron's from its group" ); }; - subtest 'can_edit_item' => sub { + subtest 'can_edit_items_from' => sub { plan tests => 6; t::lib::Mocks::mock_userenv({ patron => $patron_1A_1 }); - is( $patron_1A_1->can_edit_item( $library_1A->id ), 1, "patron_1A_1 can see patron_1A_2, from its library" ); - is( $patron_1A_1->can_edit_item( $library_1B->id ), 1, "patron_1A_1 can see patron_1B, from its group" ); - is( $patron_1A_1->can_edit_item( $library_2A->id ), 1, "patron_1A_1 can see patron_1A_2, from another group" ); + is( $patron_1A_1->can_edit_items_from( $library_1A->id ), 1, "patron_1A_1 can see patron_1A_2, from its library" ); + is( $patron_1A_1->can_edit_items_from( $library_1B->id ), 1, "patron_1A_1 can see patron_1B, from its group" ); + is( $patron_1A_1->can_edit_items_from( $library_2A->id ), 1, "patron_1A_1 can see patron_1A_2, from another group" ); t::lib::Mocks::mock_userenv({ patron => $patron_1A_2 }); - is( $patron_1A_2->can_edit_item( $library_1A->id ), 1, "patron_1A_2 can see patron_1A_1, from its library" ); - is( $patron_1A_2->can_edit_item( $library_1B->id ), 1, "patron_1A_2 can see patron_1B, from its group" ); - is( $patron_1A_2->can_edit_item( $library_2A->id ), 0, "patron_1A_2 can NOT see patron_2A, from another group" ); + is( $patron_1A_2->can_edit_items_from( $library_1A->id ), 1, "patron_1A_2 can see patron_1A_1, from its library" ); + is( $patron_1A_2->can_edit_items_from( $library_1B->id ), 1, "patron_1A_2 can see patron_1B, from its group" ); + is( $patron_1A_2->can_edit_items_from( $library_2A->id ), 0, "patron_1A_2 can NOT see patron_2A, from another group" ); }; }; -- 2.34.1