Bugzilla – Attachment 172175 Details for
Bug 31391
Staff-side recalls
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 31391: [WIP] Better recalls error messages
Bug-31391-WIP-Better-recalls-error-messages.patch (text/plain), 11.83 KB, created by
Aleisha Amohia
on 2024-09-27 20:10:18 UTC
(
hide
)
Description:
Bug 31391: [WIP] Better recalls error messages
Filename:
MIME Type:
Creator:
Aleisha Amohia
Created:
2024-09-27 20:10:18 UTC
Size:
11.83 KB
patch
obsolete
>From 87ce15db94e971dfa42ac769c47256c05e1b2bb7 Mon Sep 17 00:00:00 2001 >From: Aleisha Amohia <aleisha@catalyst.net.nz> >Date: Fri, 27 Sep 2024 20:08:47 +0000 >Subject: [PATCH] Bug 31391: [WIP] Better recalls error messages > >--- > Koha/Biblio.pm | 24 +++++----- > Koha/Item.pm | 48 +++++++++---------- > .../prog/en/modules/recalls/request.tt | 6 +-- > 3 files changed, 39 insertions(+), 39 deletions(-) > >diff --git a/Koha/Biblio.pm b/Koha/Biblio.pm >index 183b0ae7608..84c36711779 100644 >--- a/Koha/Biblio.pm >+++ b/Koha/Biblio.pm >@@ -1977,9 +1977,9 @@ sub recalls { > > =head3 can_be_recalled > >- my @items_for_recall = $biblio->can_be_recalled({ patron => $patron_object }); >+ my $recall_status = $biblio->can_be_recalled({ patron => $patron_object }); > >-Does biblio-level checks and returns the items attached to this biblio that are available for recall >+Does biblio-level checks and returns a success or error message. > > if hold_convert $param is included, this is to say that this a check to convert a hold to a recall, so we should not check for an existing hold. > >@@ -1988,7 +1988,7 @@ if hold_convert $param is included, this is to say that this a check to convert > sub can_be_recalled { > my ( $self, $params ) = @_; > >- return 0 if ( C4::Context->preference('UseRecalls') eq "off" ); >+ return ( 0, 'recalls_disabled' ) if ( C4::Context->preference('UseRecalls') eq "off" ); > > my $patron = $params->{patron}; > >@@ -2000,7 +2000,7 @@ sub can_be_recalled { > my @all_items = Koha::Items->search({ biblionumber => $self->biblionumber })->as_list; > > # if there are no available items at all, no recall can be placed >- return 0 if ( scalar @all_items == 0 ); >+ return ( 0, 'no_available_items' ) if ( scalar @all_items == 0 ); > > my @itemtypes; > my @itemnumbers; >@@ -2016,7 +2016,7 @@ sub can_be_recalled { > } > > # if there are no recallable items, no recall can be placed >- return 0 if ( scalar @items == 0 ); >+ return ( 0, 'no_available_items' ) if ( scalar @items == 0 ); > > # Check the circulation rule for each relevant itemtype for this biblio > my ( @recalls_allowed, @recalls_per_record, @on_shelf_recalls ); >@@ -2044,17 +2044,17 @@ sub can_be_recalled { > my $on_shelf_recalls = (sort {$on_shelf_recalls_count{$b} <=> $on_shelf_recalls_count{$a}} @on_shelf_recalls)[0]; # take most common > > # check recalls allowed has been set and is not zero >- return 0 if ( !defined($recalls_allowed) || $recalls_allowed == 0 ); >+ return ( 0, 'not_allowed' ) if ( !defined($recalls_allowed) || $recalls_allowed == 0 ); > > if ( $patron ) { > # check borrower has not reached open recalls allowed limit >- return 0 if ( $patron->recalls->filter_by_current->count >= $recalls_allowed ); >+ return ( 0, 'reached_max_total_recalls' ) if ( $patron->recalls->filter_by_current->count >= $recalls_allowed ); > > # check borrower has not reached open recalls allowed per record limit >- return 0 if ( $patron->recalls->filter_by_current->search({ biblio_id => $self->biblionumber })->count >= $recalls_per_record ); >+ return ( 0, 'reached_max_record_recalls' ) if ( $patron->recalls->filter_by_current->search({ biblio_id => $self->biblionumber })->count >= $recalls_per_record ); > > # check if any of the items under this biblio are already checked out by this borrower >- return 0 if ( Koha::Checkouts->search({ itemnumber => [ @all_itemnumbers ], borrowernumber => $patron->borrowernumber })->count > 0 ); >+ return ( 0, 'patron_checkouts' ) if ( Koha::Checkouts->search({ itemnumber => [ @all_itemnumbers ], borrowernumber => $patron->borrowernumber })->count > 0 ); > } > > # check item availability >@@ -2064,13 +2064,13 @@ sub can_be_recalled { > } > > # can't recall if on shelf recalls only allowed when all unavailable, but items are still available for checkout >- return 0 if ( $on_shelf_recalls eq 'all' && $checked_out_count < scalar @items ); >+ return ( 0, 'on_shelf_not_allowed' ) if ( $on_shelf_recalls eq 'all' && $checked_out_count < scalar @items ); > > # can't recall if no items have been checked out >- return 0 if ( $checked_out_count == 0 ); >+ return ( 0, 'no_checkouts' ) if ( $checked_out_count == 0 ); > > # can recall >- return @items; >+ return ( 1, 'OK' ); > } > > =head3 ratings >diff --git a/Koha/Item.pm b/Koha/Item.pm >index 1b68b441c22..f62c837cff9 100644 >--- a/Koha/Item.pm >+++ b/Koha/Item.pm >@@ -2265,9 +2265,9 @@ sub recall { > > =head3 can_be_recalled > >- if ( $item->can_be_recalled({ patron => $patron_object }) ) # do recall >+ if ( $item->can_be_recalled({ patron => $patron_object }) eq 'OK' ) # do recall > >-Does item-level checks and returns if items can be recalled by this borrower >+Does item-level checks and returns an 'OK' message if item can be recalled by this borrower, otherwise returns an error reason. > > if hold_convert $param is included, this is to say that this a check to convert a hold to a recall, so we should not check for an existing hold. > >@@ -2276,15 +2276,15 @@ if hold_convert $param is included, this is to say that this a check to convert > sub can_be_recalled { > my ( $self, $params ) = @_; > >- return 0 if ( C4::Context->preference('UseRecalls') eq "off" ); >+ return ( 0, 'recalls_disabled' ) if ( C4::Context->preference('UseRecalls') eq "off" ); > > # check if this item is not for loan, withdrawn or lost >- return 0 if ( $self->notforloan != 0 ); >- return 0 if ( $self->itemlost != 0 ); >- return 0 if ( $self->withdrawn != 0 ); >+ return ( 0, 'item_unavailable' ) if ( $self->notforloan != 0 ); >+ return ( 0, 'item_unavailable' ) if ( $self->itemlost != 0 ); >+ return ( 0, 'item_unavailable' ) if ( $self->withdrawn != 0 ); > > # check if this item is not checked out - if not checked out, can't be recalled >- return 0 if ( !defined( $self->checkout ) ); >+ return ( 0, 'no_checkouts' ) if ( !defined( $self->checkout ) ); > > my $patron = $params->{patron}; > >@@ -2306,24 +2306,24 @@ sub can_be_recalled { > }); > > # check recalls allowed has been set and is not zero >- return 0 if ( !defined($rule->{recalls_allowed}) || $rule->{recalls_allowed} == 0 ); >+ return ( 0, 'not_allowed' ) if ( !defined($rule->{recalls_allowed}) || $rule->{recalls_allowed} == 0 ); > > if ( $patron ) { > # check borrower has not reached open recalls allowed limit >- return 0 if ( $patron->recalls->filter_by_current->count >= $rule->{recalls_allowed} ); >+ return ( 0, 'reached_max_total_recalls' ) if ( $patron->recalls->filter_by_current->count >= $rule->{recalls_allowed} ); > > # check borrower has not reach open recalls allowed per record limit >- return 0 if ( $patron->recalls->filter_by_current->search({ biblio_id => $self->biblionumber })->count >= $rule->{recalls_per_record} ); >+ return ( 0, 'reached_max_record_recalls' ) if ( $patron->recalls->filter_by_current->search({ biblio_id => $self->biblionumber })->count >= $rule->{recalls_per_record} ); > > # check if this patron has already recalled this item >- return 0 if ( Koha::Recalls->search({ item_id => $self->itemnumber, patron_id => $patron->borrowernumber })->filter_by_current->count > 0 ); >+ return ( 0, 'patron_recalled' ) if ( Koha::Recalls->search({ item_id => $self->itemnumber, patron_id => $patron->borrowernumber })->filter_by_current->count > 0 ); > > # check if this patron has already checked out this item >- return 0 if ( Koha::Checkouts->search({ itemnumber => $self->itemnumber, borrowernumber => $patron->borrowernumber })->count > 0 ); >+ return ( 0, 'patron_checkouts' ) if ( Koha::Checkouts->search({ itemnumber => $self->itemnumber, borrowernumber => $patron->borrowernumber })->count > 0 ); > > # check if this patron has already reserved this item > unless ( $params->{hold_convert} ) { >- return 0 if ( Koha::Holds->search({ itemnumber => $self->itemnumber, borrowernumber => $patron->borrowernumber })->count > 0 ); >+ return ( 0, 'patron_host' ) if ( Koha::Holds->search({ itemnumber => $self->itemnumber, borrowernumber => $patron->borrowernumber })->count > 0 ); > } > } > >@@ -2332,7 +2332,7 @@ sub can_be_recalled { > my @items = Koha::Items->search({ biblionumber => $self->biblionumber, itemlost => 0, withdrawn => 0, notforloan => 0 })->as_list; > > # if there are no available items at all, no recall can be placed >- return 0 if ( scalar @items == 0 ); >+ return ( 0, 'no_available_items' ) if ( scalar @items == 0 ); > > my $checked_out_count = 0; > foreach (@items) { >@@ -2340,18 +2340,18 @@ sub can_be_recalled { > } > > # can't recall if on shelf recalls only allowed when all unavailable, but items are still available for checkout >- return 0 if ( $rule->{on_shelf_recalls} eq 'all' && $checked_out_count < scalar @items ); >+ return ( 0, 'on_shelf_not_allowed' ) if ( $rule->{on_shelf_recalls} eq 'all' && $checked_out_count < scalar @items ); > > # can't recall if no items have been checked out >- return 0 if ( $checked_out_count == 0 ); >+ return ( 0, 'no_checkouts' ) if ( $checked_out_count == 0 ); > > # can recall >- return 1; >+ return ( 1, 'OK' ); > } > > =head3 can_be_waiting_recall > >- if ( $item->can_be_waiting_recall ) { # allocate item as waiting for recall >+ if ( $item->can_be_waiting_recall eq 'OK' ) { # allocate item as waiting for recall > > Checks item type and branch of circ rules to return whether this item can be used to fill a recall. > At this point the item has already been recalled. We are now at the checkin and set waiting stage. >@@ -2361,12 +2361,12 @@ At this point the item has already been recalled. We are now at the checkin and > sub can_be_waiting_recall { > my ( $self ) = @_; > >- return 0 if ( C4::Context->preference('UseRecalls') eq "off" ); >+ return ( 0, 'recalls_disabled' ) if ( C4::Context->preference('UseRecalls') eq "off" ); > > # check if this item is not for loan, withdrawn or lost >- return 0 if ( $self->notforloan != 0 ); >- return 0 if ( $self->itemlost != 0 ); >- return 0 if ( $self->withdrawn != 0 ); >+ return ( 0, 'item_unavailable' ) if ( $self->notforloan != 0 ); >+ return ( 0, 'item_unavailable' ) if ( $self->itemlost != 0 ); >+ return ( 0, 'item_unavailable' ) if ( $self->withdrawn != 0 ); > > my $branchcode = $self->holdingbranch; > if ( C4::Context->preference('CircControl') eq 'PickupLibrary' and C4::Context->userenv and C4::Context->userenv->{'branch'} ) { >@@ -2387,10 +2387,10 @@ sub can_be_waiting_recall { > ); > > # check recalls allowed has been set and is not zero >- return 0 if ( !defined($rule->{recalls_allowed}) || $rule->{recalls_allowed} == 0 ); >+ return ( 0, 'not_allowed' ) if ( !defined($rule->{recalls_allowed}) || $rule->{recalls_allowed} == 0 ); > > # can recall >- return 1; >+ return ( 1, 'OK' ); > } > > =head3 check_recalls >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/recalls/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/recalls/request.tt >index 709dc57197f..e95202a0b6f 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/recalls/request.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/recalls/request.tt >@@ -53,13 +53,13 @@ > [% IF patron %] > > [% IF error %] >- <div class="dialog alert"> >- Unable to place a recall. Check your circulation rules. >+ <div class="alert alert-warning"> >+ <strong>Cannot place recall:</strong> check your circulation rules. > </div> > [% END %] > > [% IF patron_holds_count %] >- <div class="dialog alert alert-info"> >+ <div class="alert alert-info"> > Patron has placed one or more record-level holds. <a href="/cgi-bin/koha/reserve/request.pl?biblionumber=[% biblio.biblionumber | uri %]">Convert a hold to a recall</a>. > </div> > [% END %] >-- >2.39.5
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=172175">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 31391
:
139442
|
139443
|
139444
|
139446
|
139642
|
139643
|
139644
|
139645
|
141474
|
142594
|
143155
|
144104
|
144105
|
144131
|
144132
|
144133
|
144134
|
144135
|
144136
|
147581
|
147582
|
150485
|
150486
|
150487
|
150488
|
150489
|
150490
|
150491
|
156995
|
156996
|
156997
|
156998
|
156999
|
157000
|
157001
|
158691
|
158692
|
158693
|
158694
|
158695
|
158825
|
158826
|
158827
|
158828
|
158829
|
158830
|
158831
|
158832
|
172166
|
172167
|
172168
|
172169
|
172170
|
172171
|
172172
|
172173
|
172174
|
172175
|
173839
|
173840
|
173841
|
173842
|
173843
|
173844
|
173845
|
173846
|
173847
|
174396
|
174822
|
174823
|
174824
|
174825
|
174826
|
174827
|
174828
|
174829
|
174830
|
174831