Bugzilla – Attachment 147397 Details for
Bug 33070
Get rid of Koha::Patron->can_edit_item and use can_edit_item_from instead
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 33070: Remove use of can_edit_items
Bug-33070-Remove-use-of-canedititems.patch (text/plain), 11.39 KB, created by
Tomás Cohen Arazi (tcohen)
on 2023-02-24 19:37:04 UTC
(
hide
)
Description:
Bug 33070: Remove use of can_edit_items
Filename:
MIME Type:
Creator:
Tomás Cohen Arazi (tcohen)
Created:
2023-02-24 19:37:04 UTC
Size:
11.39 KB
patch
obsolete
>From 8c599edd6d7e5e8b03b05b7a0fcad02ccdf0c6d1 Mon Sep 17 00:00:00 2001 >From: Kyle Hall <kyle@bywatersolutions.com> >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 <tomascohen@theke.io> >--- > 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<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 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 ~%] >- <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 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 %] > <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 %]&itemnumber=[% cr.item.itemnumber | html %]&biblionumber=[% cr.biblio.biblionumber | html %]&action=lookup&return=[% course.course_id | html %]&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 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
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=147397">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 33070
:
147397
|
148325