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 <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

diff --git a/Koha/Item.pm b/Koha/Item.pm
index b9c591b..a0a18e5 100644 (file)
--- 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 9282b38..d3ec915 100644 (file)
--- 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<Koha::Patron> 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<Koha::Patron> 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<Koha::Patron> 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 a723099..b3e4181 100644 (file)
--- 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 905120e..48bd799 100755 (executable)
--- 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 90c41a3..787bac2 100755 (executable)
--- 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 586f89c..bbf801b 100755 (executable)
--- 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 9d47e19..b306384 100644 (file)
--- 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 ~%]
-    <div class="btn-group dropup"><button type="button" class="btn btn-xs btn-default dropdown-toggle" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false"> <i class="fa fa-pencil"></i> Edit <span class="caret"></span> </button> <ul class="dropdown-menu pull-right"> [% IF user.can_edit_item( item ) %]<li><a href="/cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=[% item.biblionumber | uri %]&itemnumber=[% item.itemnumber | uri %]">Edit item</a></li>[% END %] <li><a href="/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=[% item.biblionumber | html %]">Edit record</a></li> </ul> </div>
+    <div class="btn-group dropup"><button type="button" class="btn btn-xs btn-default dropdown-toggle" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false"> <i class="fa fa-pencil"></i> Edit <span class="caret"></span> </button> <ul class="dropdown-menu pull-right"> [% IF user.can_edit_items_from( item.homebranch ) %]<li><a href="/cgi-bin/koha/cataloguing/additem.pl?op=edititem&biblionumber=[% item.biblionumber | uri %]&itemnumber=[% item.itemnumber | uri %]">Edit item</a></li>[% END %] <li><a href="/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=[% item.biblionumber | html %]">Edit record</a></li> </ul> </div>
   [%~ 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 623974a..cb10993 100644 (file)
--- 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 %]
                                         <td class="actions">
-                                            [% 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 ) %]
                                                 <a class="btn btn-default btn-xs" href="add_items.pl?course_id=[% course.course_id | html %]&amp;itemnumber=[% cr.item.itemnumber | html %]&amp;biblionumber=[% cr.biblio.biblionumber | html %]&amp;action=lookup&amp;return=[% course.course_id | html %]&amp;is_edit=1"><i class="fa fa-pencil"></i> Edit</a>
                                             [% END %]
 

diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t
index 8684c8a..12f00d0 100755 (executable)
--- 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" );
     };
 };