From 5350a9276eaee6d35e73c942e910b38f2e68244d Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 23 Apr 2025 14:17:28 +0100 Subject: [PATCH] Bug 9762: (Alternative) Log circulation overrides MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An alternative approach to the original submission for bug 9762. Instead of recording a distinct action log line for a set of override messages in addition to the standard issue record, this patch updates the existing issue log to include any confirmation or impossible responses from the preceeding CanBookBeIssued call. We continue to record these as a JSON structure as the prior patch did, but instead of human messages we use the predefined codes as returned from the existing CanBookBeIssued response. To test: After each step, a single action log message should be displayed under `Tools > Log viewer`. The message itself will now consist of a JSON string with an array of 'confirmations' and 'forced' strings representing the overriden feedback from CanBookBeIssued. Circulation -- Checking out 1) Check out an item that is age-restricted (AgeRestrictionOverride syspref) 2) Check out an item that has a “not for loan” status (AllowNotForLoanOverride syspref) 3) Check out an item that has a “lost” status (IssueLostItem syspref) 4) Check out an item to a patron who has reached the checkout limit (AllowTooManyOverride syspref) 5) Check out an item to a patron who is not allowed to borrow this item type 6) Check out an item to a patron who has unpaid fines (AllFinesNeedOverride and/or AllowFineOverride + noissuescharge sysprefs) 7) Check out an item on hold for someone else 8) Check out an item on hold ready for pickup by someone else 9) Check out an item already checked out to someone else (AutoReturnCheckedOutItems syspref) 10) Check out to a patron who has restrictions Circulation -- Checking in 11) Ignore a hold upon check in 12) Ignore a hold and transfer upon check in Circulation – Renewing 13) Renew an item on hold for someone else (AllowRenewalOnHoldOverride syspref) 14) Renew an item that has reached the maximum number of renewals (AllowRenewalLimitOverride syspref) Holds 15) Place a hold for a patron who is not allowed to place this item type on hold (AllowHoldPolicyOverride syspref) 16) Place a hold for a patron who has reached the maximum number of holds 17) Place an item-level hold when rules dictate that this is not allowed Sponsored-by: OpenFifth Signed-off-by: Martin Renvoize Signed-off-by: Lucas Gass --- C4/Circulation.pm | 28 +++++++-- C4/SIP/ILS/Transaction/Checkout.pm | 11 +++- Koha/CurbsidePickup.pm | 10 +++- Koha/REST/V1/Checkouts.pm | 10 +++- circ/circulation.pl | 8 ++- .../prog/en/includes/action-logs.inc | 59 +++++++++++++++++++ .../prog/en/modules/tools/viewlog.tt | 16 +++++ opac/sco/sco-main.pl | 8 ++- tools/viewlog.pl | 34 ++++++++++- 9 files changed, 170 insertions(+), 14 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 8fd794eb11..fc0ec5eb18 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1674,6 +1674,8 @@ sub AddIssue { my $auto_renew = $params && $params->{auto_renew}; my $cancel_recall = $params && $params->{cancel_recall}; my $recall_id = $params && $params->{recall_id}; + my $confirmations = $params->{confirmations}; + my $forced = $params->{forced}; my $dbh = C4::Context->dbh; my $barcodecheck = CheckValidBarcode($barcode); @@ -2011,11 +2013,27 @@ sub AddIssue { } ); } - logaction( - "CIRCULATION", "ISSUE", - $patron->borrowernumber, - $item_object->itemnumber, - ) if C4::Context->preference("IssueLog"); + + # Log the checkout + if ( C4::Context->preference('IssueLog') ) { + my $info = $item_object->itemnumber; + if ( defined($confirmations) || defined($forced) ) { + $info = to_json( + { + issue => $issue->issue_id, + itemnumber => $item_object->itemnumber, + confirmations => $confirmations, + forced => $forced + }, + { pretty => 1, canonical => 1 } + ); + } + logaction( + "CIRCULATION", "ISSUE", + $patron->borrowernumber, + $info, + ); + } Koha::Plugins->call( 'after_circ_action', diff --git a/C4/SIP/ILS/Transaction/Checkout.pm b/C4/SIP/ILS/Transaction/Checkout.pm index 4909400493..39ffff173a 100644 --- a/C4/SIP/ILS/Transaction/Checkout.pm +++ b/C4/SIP/ILS/Transaction/Checkout.pm @@ -174,7 +174,16 @@ sub do_checkout { $recall_id = $messages->{RECALLED}; } } - my $issue = AddIssue( $patron, $barcode, $overridden_duedate, 0, undef, undef, { recall_id => $recall_id } ); + my $issue = AddIssue( + $patron, $barcode, + $overridden_duedate, + 0, undef, undef, + { + recall_id => $recall_id, + confirmations => [ grep { /^[A-Z_]+$/ } keys %{$needsconfirmation} ], + forced => [ keys %{$issuingimpossible} ] + } + ); $self->{due} = $self->duedatefromissue( $issue, $itemnumber ); } diff --git a/Koha/CurbsidePickup.pm b/Koha/CurbsidePickup.pm index 6c75a8525b..59b35dd8e5 100644 --- a/Koha/CurbsidePickup.pm +++ b/Koha/CurbsidePickup.pm @@ -252,7 +252,15 @@ sub mark_as_delivered { C4::Circulation::CanBookBeIssued( $patron, $hold->item->barcode ); unless ( keys %$issuingimpossible ) { - my $issue = C4::Circulation::AddIssue( $patron, $hold->item->barcode ); + my $issue = C4::Circulation::AddIssue( + $patron, + $hold->item->barcode, + undef, undef, undef, undef, + { + confirmations => [ grep { /^[A-Z_]+$/ } keys %{$needsconfirmation} ], + forced => [ keys %{$issuingimpossible} ] + } + ); if ($issue) { Koha::CurbsidePickupIssue->new( { diff --git a/Koha/REST/V1/Checkouts.pm b/Koha/REST/V1/Checkouts.pm index 8ff774edc2..578a093cfe 100644 --- a/Koha/REST/V1/Checkouts.pm +++ b/Koha/REST/V1/Checkouts.pm @@ -299,7 +299,15 @@ sub add { } # Call 'AddIssue' - my $checkout = AddIssue( $patron, $item->barcode ); + my $checkout = AddIssue( + $patron, + $item->barcode, + undef, undef, undef, undef, + { + confirmations => [ grep { /^[A-Z_]+$/ } keys %{$confirmation} ], + forced => [ keys %{$impossible} ] + } + ); if ($checkout) { $c->res->headers->location( $c->req->url->to_string . '/' . $checkout->id ); return $c->render( diff --git a/circ/circulation.pl b/circ/circulation.pl index e598c56a0e..c0b8ec8b65 100755 --- a/circ/circulation.pl +++ b/circ/circulation.pl @@ -552,9 +552,13 @@ if ( @$barcodes && $op eq 'cud-checkout' ) { $cancelreserve, undef, undef, { - onsite_checkout => $onsite_checkout, auto_renew => $session->param('auto_renew'), - switch_onsite_checkout => $switch_onsite_checkout, cancel_recall => $cancel_recall, + onsite_checkout => $onsite_checkout, + auto_renew => $session->param('auto_renew'), + switch_onsite_checkout => $switch_onsite_checkout, + cancel_recall => $cancel_recall, recall_id => $recall_id, + confirmations => [ grep { /^[A-Z_]+$/ } keys %{$needsconfirmation} ], + forced => [ keys %{$issuingimpossible} ] } ); $template_params->{issue} = $issue; diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc index 09455971a2..03724bc859 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc @@ -176,3 +176,62 @@ [% log_interface | html %] [% END %] [% END %] + +[% BLOCK translate_log_override %] + [% SWITCH override %] + [% CASE 'INVALID_DATE' %] + Invalid date confirmed + [% CASE 'DEBT_GUARANTEES' %] + Guarantees had debts + [% CASE 'DEBT_GUARANTORS' %] + Guarantors had debts + [% CASE 'DEBT' %] + Patron had debt + [% CASE 'USERBLOCKEDOVERDUE' %] + Patron had overdues + [% CASE 'ADDITIONAL_MATERIALS' %] + Additional materials were confirmed + [% CASE 'RENEW_ISSUE' %] + Renewal + [% CASE 'ISSUED_TO_ANOTHER' %] + Item was issued to another patron prior to this checkout + [% CASE 'PATRON_CANT' %] + Max renewals reached + [% CASE 'TOO_MANY' %] + Max renewals reached + [% CASE 'PREVISSUE' %] + Patron was notified of previous issues + [% CASE 'NOT_FOR_LOAN' %] + Item was not for loan + [% CASE 'NOT_FOR_LOAN_FORCING' %] + Item was not for loan + [% CASE 'ITEM_LOST' %] + Item was reported lost + [% CASE 'BORRNOTSAMEBRANCH' %] + Patron does not belong to this library + [% CASE 'RENTALCHARGE' %] + Item had a rental charge + [% CASE 'RECALLED' %] + Item was recalled + [% CASE 'RESERVE_WAITING' %] + Item had a reserve waiting + [% CASE 'RESERVED' %] + Item was reserved + [% CASE 'TRANSFERRED' %] + Item was transferred + [% CASE 'PROCESSING' %] + Item was in processing + [% CASE 'BOOKED_EARLY' %] + Item was booked and checked out early + [% CASE 'BOOKED_TO_ANOTHER' %] + Item was booked to another patron + [% CASE 'AGE_RESTRICTION' %] + Item has an eage restriction + [% CASE 'HIGHHOLDS' %] + Item had high holds + [% CASE 'BIBLIO_ALREADY_ISSUED' %] + Biblio was already issued + [% CASE %] + [% override | html %] + [% END %] +[% END %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt index 5df1493c8b..a53132ea91 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt @@ -333,6 +333,22 @@ title="Display detail for this item" >Item [% loopro.barcode | html %] + [% ELSIF ( loopro.module == 'CIRCULATION' && loopro.json_found ) %] +
+ + [% FOREACH force IN loopro.json.forces %] +
[% PROCESS translare_log_override override=force %]
+ [% END %] + [% FOREACH confirmation IN loopro.json.confirmations %] +
[% PROCESS translate_log_override override=confirmation %]
+ [% END %] +
[% ELSIF loopro.module == "SYSTEMPREFERENCE" || loopro.module == "REPORTS" || loopro.module == "NEWS" %]
[% loopro.info | trim | html %]
diff --git a/opac/sco/sco-main.pl b/opac/sco/sco-main.pl index 9ecae7c76c..fa198ab886 100755 --- a/opac/sco/sco-main.pl +++ b/opac/sco/sco-main.pl @@ -281,7 +281,13 @@ if ( $patron && $op eq "cud-returnbook" && $allowselfcheckreturns ) { )->count; } - my $new_issue = AddIssue( $patron, $barcode ); + my $new_issue = AddIssue( + $patron, $barcode, undef, undef, undef, undef, + { + confirmations => [ grep { /^[A-Z_]+$/ } keys %{$needconfirm} ], + forced => [ keys %{$impossible} ] + } + ); $template->param( issued => 1, new_issue => $new_issue ); push @newissueslist, $barcode unless ( grep /^$barcode$/, @newissueslist ); diff --git a/tools/viewlog.pl b/tools/viewlog.pl index 21157e94f3..5d5341168f 100755 --- a/tools/viewlog.pl +++ b/tools/viewlog.pl @@ -22,6 +22,7 @@ use Modern::Perl; use C4::Auth qw( get_template_and_user ); use CGI qw ( -utf8 ); +use JSON; use Text::CSV::Encoded; use C4::Context; use C4::Output qw( output_html_with_http_headers ); @@ -152,12 +153,11 @@ if ($do_it) { $result->{'biblioitemnumber'} = q{}; $result->{'barcode'} = q{}; - if ( substr( $log->info, 0, 4 ) eq 'item' || $log->module eq "CIRCULATION" ) { + if ( substr( $log->info, 0, 4 ) eq 'item' ) { # get item information so we can create a working link my $itemnumber = $log->object; - $itemnumber = $log->info if ( $log->module eq "CIRCULATION" ); - my $item = Koha::Items->find($itemnumber); + my $item = Koha::Items->find($itemnumber); if ($item) { $result->{'object_found'} = 1; $result->{'biblionumber'} = $item->biblionumber; @@ -166,6 +166,34 @@ if ($do_it) { } } + if ( $log->module eq "CIRCULATION" ) { + my $info = $log->info; + my $decoded; + my $is_json = eval { + $decoded = decode_json($info); + 1; + }; + + if ( $is_json && ref($decoded) ) { + $result->{'json_found'} = 1; + my $item = Koha::Items->find( $decoded->{itemnumber} ); + if ($item) { + $decoded->{biblionumber} = $item->biblionumber; + $decoded->{biblioitemnumber} = $item->biblionumber; + $decoded->{barcode} = $item->barcode; + } + $result->{'json'} = $decoded; + } else { + my $item = Koha::Items->find($info); + if ($item) { + $result->{'object_found'} = 1; + $result->{'biblionumber'} = $item->biblionumber; + $result->{'biblioitemnumber'} = $item->biblionumber; + $result->{'barcode'} = $item->barcode; + } + } + } + #always add firstname and surname for librarian/user if ( $log->user ) { my $patron = Koha::Patrons->find( $log->user );