From 87ce15db94e971dfa42ac769c47256c05e1b2bb7 Mon Sep 17 00:00:00 2001 From: Aleisha Amohia 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 %] -
- Unable to place a recall. Check your circulation rules. +
+ Cannot place recall: check your circulation rules.
[% END %] [% IF patron_holds_count %] -
+
Patron has placed one or more record-level holds. Convert a hold to a recall.
[% END %] -- 2.39.5