Bugzilla – Attachment 187707 Details for
Bug 40866
Corrections to override logging
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40866: [25.05.x] Remove original implementation of Bug 9762 and implement alternative squashed patchset for 25.05
Bug-40866-2505x-Remove-original-implementation-of-.patch (text/plain), 36.37 KB, created by
Lucas Gass (lukeg)
on 2025-10-09 21:21:26 UTC
(
hide
)
Description:
Bug 40866: [25.05.x] Remove original implementation of Bug 9762 and implement alternative squashed patchset for 25.05
Filename:
MIME Type:
Creator:
Lucas Gass (lukeg)
Created:
2025-10-09 21:21:26 UTC
Size:
36.37 KB
patch
obsolete
>From d5aba6860f67007c888deabbcf1705881573b0c1 Mon Sep 17 00:00:00 2001 >From: Hammat Wele <hammat.wele@inlibro.com> >Date: Tue, 30 Sep 2025 16:37:45 +0000 >Subject: [PATCH] Bug 40866: [25.05.x] Remove original implementation of Bug > 9762 and implement alternative squashed patchset for 25.05 > >Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> >Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> >--- > C4/Circulation.pm | 76 +-------------- > C4/Reserves.pm | 24 ++++- > Koha/REST/V1/Holds.pm | 7 ++ > circ/circulation.pl | 33 ------- > circ/renew.pl | 49 +--------- > circ/returns.pl | 27 ------ > .../prog/en/includes/action-logs.inc | 10 ++ > .../prog/en/modules/circ/circulation.tt | 5 - > .../prog/en/modules/tools/viewlog.tt | 22 +++++ > reserve/request.pl | 28 ------ > svc/renew | 52 +---------- > t/db_dependent/Circulation.t | 21 ++++- > t/db_dependent/Reserves.t | 92 ++++++++++++++++++- > t/db_dependent/api/v1/holds.t | 72 ++++++++++++++- > 14 files changed, 248 insertions(+), 270 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 454b1109bc3..1dbc4cf0046 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -825,15 +825,11 @@ sub CanBookBeIssued { > my $circ_library = Koha::Libraries->find( _GetCircControlBranch( $item_object, $patron ) ); > > my $now = dt_from_string(); >- my @message_log; > > $duedate ||= CalcDateDue( $now, $effective_itemtype, $circ_library->branchcode, $patron ); > > if ( DateTime->compare( $duedate, $now ) == -1 ) { # duedate cannot be before now > $needsconfirmation{INVALID_DATE} = $duedate; >- if ($issueconfirmed) { >- push( @message_log, "sticky due date is invalid or due date in the past" ); >- } > } > > my $fees = Koha::Charges::Fees->new( >@@ -879,9 +875,6 @@ sub CanBookBeIssued { > } > if ( $patron->is_debarred ) { > $issuingimpossible{DEBARRED} = 1; >- if ($issueconfirmed) { >- push( @message_log, "borrower is restricted" ); >- } > } > > if ( $patron->is_expired ) { >@@ -954,16 +947,10 @@ sub CanBookBeIssued { > } else { > if ( $patron_borrowing_status->{noissuescharge}->{overlimit} && $allowfineoverride ) { > $needsconfirmation{DEBT} = $patron_borrowing_status->{noissuescharge}->{charge}; >- if ($issueconfirmed) { >- push( @message_log, "borrower had amend" ); >- } > } elsif ( $patron_borrowing_status->{noissuescharge}->{overlimit} && !$allowfineoverride ) { > $issuingimpossible{DEBT} = $patron_borrowing_status->{noissuescharge}->{charge}; > } elsif ( $patron_borrowing_status->{noissuescharge}->{charge} > 0 && $allfinesneedoverride ) { > $needsconfirmation{DEBT} = $patron_borrowing_status->{noissuescharge}->{charge}; >- if ($issueconfirmed) { >- push( @message_log, "borrower had amend" ); >- } > } > } > >@@ -1044,9 +1031,6 @@ sub CanBookBeIssued { > $needsconfirmation{issued_surname} = $patron->surname; > $needsconfirmation{issued_cardnumber} = $patron->cardnumber; > $needsconfirmation{issued_borrowernumber} = $patron->borrowernumber; >- if ($issueconfirmed) { >- push( @message_log, "item is checked out for someone else" ); >- } > } > } > } >@@ -1074,9 +1058,6 @@ sub CanBookBeIssued { > $needsconfirmation{current_loan_count} = $toomany->{count}; > $needsconfirmation{max_loans_allowed} = $toomany->{max_allowed}; > $needsconfirmation{circulation_rule_TOO_MANY} = $toomany->{circulation_rule}; >- if ($issueconfirmed) { >- push( @message_log, "too many checkout" ); >- } > } else { > $issuingimpossible{TOO_MANY} = $toomany->{reason}; > $issuingimpossible{current_loan_count} = $toomany->{count}; >@@ -1109,9 +1090,6 @@ sub CanBookBeIssued { > } else { > $needsconfirmation{NOT_FOR_LOAN_FORCING} = 1; > $needsconfirmation{item_notforloan} = $item_object->notforloan; >- if ($issueconfirmed) { >- push( @message_log, "item not for loan" ); >- } > } > } else { > >@@ -1157,9 +1135,6 @@ sub CanBookBeIssued { > my $code = $av->count ? $av->next->lib : ''; > $needsconfirmation{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'confirm' ); > $alerts{ITEM_LOST} = $code if ( C4::Context->preference("IssueLostItem") eq 'alert' ); >- if ($issueconfirmed) { >- push( @message_log, "item lost" ); >- } > } > if ( C4::Context->preference("IndependentBranches") ) { > my $userenv = C4::Context->userenv; >@@ -1260,13 +1235,6 @@ sub CanBookBeIssued { > $needsconfirmation{'resbranchcode'} = $res->{branchcode}; > $needsconfirmation{'reswaitingdate'} = $res->{'waitingdate'}; > $needsconfirmation{'reserve_id'} = $res->{reserve_id}; >- >- if ($issueconfirmed) { >- push( >- @message_log, >- "item is on reserve and waiting, but has been reserved by some other patron" >- ); >- } > } elsif ( $restype eq "Reserved" ) { > > # The item is on reserve for someone else. >@@ -1278,10 +1246,6 @@ sub CanBookBeIssued { > $needsconfirmation{'resbranchcode'} = $res->{branchcode}; > $needsconfirmation{'resreservedate'} = $res->{reservedate}; > $needsconfirmation{'reserve_id'} = $res->{reserve_id}; >- >- if ($issueconfirmed) { >- push( @message_log, "item is on reserve for someone else" ); >- } > } elsif ( $restype eq "Transferred" ) { > > # The item is determined hold being transferred for someone else. >@@ -1293,10 +1257,6 @@ sub CanBookBeIssued { > $needsconfirmation{'resbranchcode'} = $res->{branchcode}; > $needsconfirmation{'resreservedate'} = $res->{reservedate}; > $needsconfirmation{'reserve_id'} = $res->{reserve_id}; >- >- if ($issueconfirmed) { >- push( @message_log, "item is determined hold being transferred for someone else" ); >- } > } elsif ( $restype eq "Processing" ) { > > # The item is determined hold being processed for someone else. >@@ -1308,10 +1268,6 @@ sub CanBookBeIssued { > $needsconfirmation{'resbranchcode'} = $res->{branchcode}; > $needsconfirmation{'resreservedate'} = $res->{reservedate}; > $needsconfirmation{'reserve_id'} = $res->{reserve_id}; >- >- if ($issueconfirmed) { >- push( @message_log, "item is determined hold being processed for someone else" ); >- } > } > } > } >@@ -1355,9 +1311,6 @@ sub CanBookBeIssued { > if ( $restriction_age && $patron->dateofbirth && $restriction_age > $patron->get_age() ) { > if ( C4::Context->preference('AgeRestrictionOverride') ) { > $needsconfirmation{AGE_RESTRICTION} = "$agerestriction"; >- if ($issueconfirmed) { >- push( @message_log, "age restriction" ); >- } > } else { > $issuingimpossible{AGE_RESTRICTION} = "$agerestriction"; > } >@@ -1416,34 +1369,7 @@ sub CanBookBeIssued { > } > } > >- my $borrower = $patron; >- my $user = C4::Context->userenv->{number}; >- my $branchcode = C4::Context->userenv->{branch}; >- >- # action, cardnumber, barcode, date, heure, user, branche >- if ($issueconfirmed) { >- >- my $infos = ( >- { >- message => \@message_log, >- borrowernumber => $borrower->borrowernumber, >- barcode => $barcode, >- manager_id => $user, >- branchcode => $branchcode, >- } >- ); >- >- my $json_infos = JSON->new->utf8->pretty->encode($infos); >- $json_infos =~ s/"/'/g; >- >- logaction( >- "CIRCULATION", "ISSUE", >- $borrower->{'borrowernumber'}, >- $json_infos, >- ) if C4::Context->preference("IssueLog"); >- } >- >- return ( \%issuingimpossible, \%needsconfirmation, \%alerts, \%messages, \@message_log ); >+ return ( \%issuingimpossible, \%needsconfirmation, \%alerts, \%messages ); > } > > =head2 CanBookBeReturned >diff --git a/C4/Reserves.pm b/C4/Reserves.pm >index 04b43af2507..4ace8d99075 100644 >--- a/C4/Reserves.pm >+++ b/C4/Reserves.pm >@@ -22,6 +22,8 @@ package C4::Reserves; > > use Modern::Perl; > >+use JSON qw( to_json ); >+ > use C4::Accounts; > use C4::Biblio qw( GetMarcFromKohaField ); > use C4::Circulation qw( CheckIfIssuedToPatron GetAgeRestriction GetBranchItemRule ); >@@ -192,6 +194,8 @@ sub AddReserve { > my $itemtype = $params->{itemtype}; > my $non_priority = $params->{non_priority}; > my $item_group_id = $params->{item_group_id}; >+ my $confirmations = $params->{confirmations}; >+ my $forced = $params->{forced}; > > $resdate ||= dt_from_string; > >@@ -261,8 +265,24 @@ sub AddReserve { > # record patron activity > $hold->patron->update_lastseen('hold'); > >- logaction( 'HOLDS', 'CREATE', $hold->id, $hold ) >- if C4::Context->preference('HoldsLog'); >+ # Log the hold creation >+ if ( C4::Context->preference('HoldsLog') ) { >+ my $info = $hold->id; >+ if ( defined($confirmations) || defined($forced) ) { >+ $info = to_json( >+ { >+ hold => $hold->id, >+ branchcode => $hold->branchcode, >+ biblionumber => $hold->biblionumber, >+ itemnumber => $hold->itemnumber, >+ confirmations => $confirmations, >+ forced => $forced >+ }, >+ { pretty => 1, canonical => 1 } >+ ); >+ } >+ logaction( 'HOLDS', 'CREATE', $hold->id, $info ); >+ } > > my $reserve_id = $hold->id(); > >diff --git a/Koha/REST/V1/Holds.pm b/Koha/REST/V1/Holds.pm >index 53abf9c2cdf..f4f74d655a4 100644 >--- a/Koha/REST/V1/Holds.pm >+++ b/Koha/REST/V1/Holds.pm >@@ -191,6 +191,12 @@ sub add { > > my $priority = C4::Reserves::CalculatePriority($biblio_id); > >+ # Build confirmations array for logging overrides >+ my $confirmations = []; >+ if ($override_all) { >+ push @{$confirmations}, 'HOLD_POLICY_OVERRIDE'; >+ } >+ > my $hold_id = C4::Reserves::AddReserve( > { > branchcode => $pickup_library_id, >@@ -206,6 +212,7 @@ sub add { > itemtype => $item_type, > non_priority => $non_priority, > item_group_id => $item_group_id, >+ confirmations => $confirmations, > } > ); > >diff --git a/circ/circulation.pl b/circ/circulation.pl >index d85defd0ad1..bc268f499b0 100755 >--- a/circ/circulation.pl >+++ b/circ/circulation.pl >@@ -39,7 +39,6 @@ use C4::Members; > use C4::Biblio qw( TransformMarcToKoha ); > use C4::Search qw( new_record_from_zebra ); > use C4::Reserves qw( ModReserveAffect ); >-use C4::Log qw( logaction ); > use Koha::Holds; > use C4::Context; > use CGI::Session; >@@ -326,9 +325,6 @@ if ($patron) { > # STEP 3 : ISSUING > # > # >-my $message; >-my @message; >- > if ( @$barcodes && $op eq 'cud-checkout' ) { > my $checkout_infos; > for my $barcode (@$barcodes) { >@@ -564,34 +560,6 @@ if ( @$barcodes && $op eq 'cud-checkout' ) { > } > ); > $template_params->{issue} = $issue; >- >- my $borrower = $patron; >- my $borrowernumber = $patron->borrowernumber; >- my $user = C4::Context->userenv->{number}; >- my $branchcode = C4::Context->userenv->{branch}; >- $message = "Restriction overridden temporarily"; >- @message = ("Restriction overridden temporarily"); >- >- if ($issueconfirmed) { >- my $infos = ( >- { >- message => \@message, >- borrowernumber => $borrowernumber, >- barcode => $barcode, >- manager_id => $user, >- branchcode => $branchcode, >- } >- ); >- >- my $json_infos = JSON->new->utf8->pretty->encode($infos); >- $json_infos =~ s/"/'/g; >- >- logaction( >- "CIRCULATION", "ISSUE", >- $borrower->{'borrowernumber'}, >- $json_infos, >- ) if C4::Context->preference("IssueLog"); >- } > $session->clear('auto_renew'); > $inprocess = 1; > } >@@ -810,7 +778,6 @@ $template->param( > stickyduedate => $stickyduedate, > duedatespec => $duedatespec, > restoreduedatespec => $restoreduedatespec, >- message => $message, > totaldue => sprintf( '%.2f', $balance ), # FIXME not used in template? > inprocess => $inprocess, > $view => 1, >diff --git a/circ/renew.pl b/circ/renew.pl >index 069312f85b1..fcc9860a9a9 100755 >--- a/circ/renew.pl >+++ b/circ/renew.pl >@@ -24,7 +24,6 @@ use C4::Context; > use C4::Auth qw( get_template_and_user ); > use C4::Output qw( output_html_with_http_headers ); > use C4::Circulation qw( barcodedecode CanBookBeRenewed GetLatestAutoRenewDate AddRenewal ); >-use C4::Log qw( logaction ); > use Koha::DateUtils qw( dt_from_string ); > use Koha::Database; > use Koha::BiblioFrameworks; >@@ -66,42 +65,19 @@ if ( $op eq 'cud-renew' && $barcode ) { > > $patron = $checkout->patron; > my $borrowernumber = $patron->borrowernumber; >- my $user = C4::Context->userenv->{number}; >- my $branchcode = C4::Context->userenv->{branch}; >- my @message; > > if ( ( $patron->is_debarred || q{} ) lt dt_from_string()->ymd() ) { > my $confirmations; > my $can_renew; > my $info; > ( $can_renew, $error, $info ) = CanBookBeRenewed( $patron, $checkout, $override_limit ); >- push @{$confirmations}, $override_limit if $override_limit; >+ push @{$confirmations}, 'RENEWAL_LIMIT' if $override_limit; > > if ( $error && ( $error eq 'on_reserve' ) ) { > if ($override_holds) { > $can_renew = 1; > $error = undef; >- >- @message = ("Override Renew hold for another"); >- >- my $infos = ( >- { >- message => \@message, >- borrowernumber => $borrowernumber, >- barcode => $barcode, >- manager_id => $user, >- branchcode => $branchcode, >- } >- ); >- >- my $json_infos = JSON->new->utf8->pretty->encode($infos); >- $json_infos =~ s/"/'/g; >- >- logaction( >- "CIRCULATION", "RENEWAL", >- $patron->borrowernumber, >- $json_infos, >- ) if C4::Context->preference("RenewalLog"); >+ push @{$confirmations}, 'ON_RESERVE'; > } else { > $can_renew = 0; > } >@@ -133,27 +109,6 @@ if ( $op eq 'cud-renew' && $barcode ) { > } > ); > $template->param( date_due => $date_due ); >- >- @message = ("Override limit Renew"); >- >- my $infos = ( >- { >- message => \@message, >- borrowernumber => $borrowernumber, >- barcode => $barcode, >- manager_id => $user, >- branchcode => $branchcode, >- } >- ); >- >- my $json_infos = JSON->new->utf8->pretty->encode($infos); >- $json_infos =~ s/"/'/g; >- >- logaction( >- "CIRCULATION", "RENEWAL", >- $patron->borrowernumber, >- $json_infos, >- ) if C4::Context->preference("RenewalLog"); > } > } else { > $error = "patron_restricted"; >diff --git a/circ/returns.pl b/circ/returns.pl >index 1721092cf5b..f4871fa9d2e 100755 >--- a/circ/returns.pl >+++ b/circ/returns.pl >@@ -42,7 +42,6 @@ use C4::Members; > use C4::Output qw( output_html_with_http_headers ); > use C4::Reserves qw( ModReserve ModReserveAffect CheckReserves ); > use C4::RotatingCollections; >-use C4::Log qw( logaction ); > use Koha::AuthorisedValues; > use Koha::BiblioFrameworks; > use Koha::Calendar; >@@ -159,32 +158,6 @@ if ( $query->param('reserve_id') && $op eq 'cud-affect_reserve' ) { > $transfer->transit; > } > } >-} else { >- my $message = "return an element that is reserved"; >- my @message = ("return an element that is reserved"); >- my $user = C4::Context->userenv->{number}; >- my $branchcode = C4::Context->userenv->{branch}; >- my $barcode = $query->param('barcode'); >- >- my $infos = ( >- { >- message => \@message, >- >- #'card number' => $cardnumber, >- barcode => $barcode, >- manager_id => $user, >- branchcode => $branchcode, >- } >- ); >- >- my $json_infos = JSON->new->utf8->pretty->encode($infos); >- $json_infos =~ s/"/'/g; >- >- logaction( >- "CIRCULATION", "RETURN", >- $session, >- $json_infos, >- ) if C4::Context->preference("ReturnLog"); > } > > if ( $query->param('recall_id') && $op eq 'cud-affect_recall' ) { >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 6b0deca6366..262ecd79718 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/action-logs.inc >@@ -179,6 +179,8 @@ > > [% BLOCK translate_log_override %] > [% SWITCH override %] >+ [% CASE 'HOLD_POLICY_OVERRIDE' %] >+ <span>Hold policy override</span> > [% CASE 'INVALID_DATE' %] > <span>Invalid date confirmed</span> > [% CASE 'DEBT_GUARANTEES' %] >@@ -189,6 +191,8 @@ > <span>Patron had debt</span> > [% CASE 'USERBLOCKEDOVERDUE' %] > <span>Patron had overdues</span> >+ [% CASE 'USERBLOCKEDNOENDDATE' %] >+ <span>Patron had an indefinite restriction</span> > [% CASE 'ADDITIONAL_MATERIALS' %] > <span>Additional materials were confirmed</span> > [% CASE 'RENEW_ISSUE' %] >@@ -247,6 +251,12 @@ > <span>Override Renew hold for another</span> > [% CASE 'TOO_MUCH' %] > <span>Override auto-renew too much owing restriction</span> >+ [% CASE 'CURRENTISSUE' %] >+ <span>Item is currently checked out to this patron</span> >+ [% CASE 'UNSEEN' %] >+ <span>Unseen renewal</span> >+ [% CASE 'SEEN' %] >+ <span>Seen renewal</span> > [% CASE %] > <span>[% override | html %]</span> > [% END %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt >index 29aa6532b97..d774abc194c 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt >@@ -788,11 +788,6 @@ > <p>Item checked out</p> > [% END %] > >- [% IF ( message ) %] >- [% INCLUDE 'patron-toolbar.inc' %] >- <h4>No patron matched <span class="ex">[% message | html %]</span></h4> >- [% END %] >- > <!-- BARCODE ENTRY --> > > [% IF patron %] >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 44e6e4b20e2..03efca0532b 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/viewlog.tt >@@ -349,6 +349,28 @@ > <div class="confirmed"> [% PROCESS translate_log_override override=confirmation %] </div> > [% END %] > </div> >+ [% ELSIF ( loopro.module == 'HOLDS' && loopro.json_found ) %] >+ <div class="loginfo" id="loginfo[% loopro.action_id | html %]"> >+ <div> >+ <a href="/cgi-bin/koha/reserve/request.pl?biblionumber=[% loopro.json.biblionumber | uri %]" title="Display holds for this record" >+ >Hold [% loopro.json.hold | html %] on biblio [% loopro.json.biblionumber | html %]</a >+ > >+ [% IF loopro.json.itemnumber %] >+ - >+ <a >+ href="/cgi-bin/koha/catalogue/moredetail.pl?itemnumber=[% loopro.json.itemnumber | uri %]&biblionumber=[% loopro.json.biblionumber | uri %]#item[% loopro.json.itemnumber | uri %]" >+ title="Display detail for this item" >+ >Item [% loopro.json.itemnumber | html %]</a >+ > >+ [% END %] >+ </div> >+ [% FOREACH force IN loopro.json.forced %] >+ <div class="forced"> [% PROCESS translate_log_override override=force %] </div> >+ [% END %] >+ [% FOREACH confirmation IN loopro.json.confirmations %] >+ <div class="confirmed"> [% PROCESS translate_log_override override=confirmation %] </div> >+ [% END %] >+ </div> > [% ELSIF loopro.module == "SYSTEMPREFERENCE" || loopro.module == "REPORTS" || loopro.module == "NEWS" %] > <div class="loginfo" id="loginfo[% loopro.action_id | html %]">[% loopro.info | trim | html %]</div> > <div class="compare_info" id="compare_info[% loopro.action_id | html %]"> >diff --git a/reserve/request.pl b/reserve/request.pl >index dd2023c6b51..a553070e4b3 100755 >--- a/reserve/request.pl >+++ b/reserve/request.pl >@@ -40,7 +40,6 @@ use C4::Serials qw( CountSubscriptionFromBiblionumber ); > use C4::Circulation qw( _GetCircControlBranch GetBranchItemRule ); > use Koha::DateUtils qw( dt_from_string ); > use C4::Search qw( enabled_staff_search_views ); >-use C4::Log qw( logaction ); > > use Koha::Biblios; > use Koha::Checkouts; >@@ -570,33 +569,6 @@ if ( ( $findborrower && $borrowernumber_hold || $findclub && $club_hold ) > $num_alreadyheld++; > } > >- my $borrower = $patron; >- my $borrowernumber = $patron->borrowernumber; >- my $user = C4::Context->userenv->{number}; >- my $branchcode = C4::Context->userenv->{branch}; >- my $message = "Override limitation of hold"; >- my @message = ("Override limitation of hold"); >- my $barcode = $item->{barcode}; >- >- my $infos = ( >- { >- message => \@message, >- borrowernumber => $borrowernumber, >- barcode => $barcode, >- manager_id => $user, >- branchcode => $branchcode, >- } >- ); >- >- my $json_infos = JSON->new->utf8->pretty->encode($infos); >- $json_infos =~ s/"/'/g; >- >- logaction( >- "HOLD", "ISSUE", >- $borrower->{'borrowernumber'}, >- $json_infos, >- ) if C4::Context->preference("IssueLog"); >- > push( @available_itemtypes, $item->{itype} ); > } else { > >diff --git a/svc/renew b/svc/renew >index a2877d5f7fa..c3b8547a06a 100755 >--- a/svc/renew >+++ b/svc/renew >@@ -26,7 +26,6 @@ use Try::Tiny; > use C4::Circulation qw( AddRenewal CanBookBeRenewed ); > use C4::Context; > use C4::Auth qw(check_cookie_auth); >-use C4::Log qw( logaction ); > > use Koha::DateUtils qw(output_pref dt_from_string); > >@@ -54,10 +53,6 @@ my $date_due; > if ( $input->param('date_due') ) { > $date_due = dt_from_string( scalar $input->param('date_due') ); > } >-my $borrower = Koha::Patrons->find($borrowernumber); >-my $user = C4::Context->userenv->{'number'}; >-my $message; >-my @message; > > my $data; > $data->{itemnumber} = $itemnumber; >@@ -75,56 +70,13 @@ my $confirmations; > if ( $data->{error} && $data->{error} eq 'on_reserve' && C4::Context->preference('AllowRenewalOnHoldOverride') ) { > $data->{renew_okay} = 1; > $data->{error} = undef; >- >- push @{$confirmations}, 'ON_RESERVE' if $override_limit; >- $message = "Override Renew hold for another"; >- @message = ("Override Renew hold for another"); >- >- my $infos = ( >- { >- message => \@message, >- borrowernumber => $borrowernumber, >- barcode => $item->barcode, >- manager_id => $user, >- branchcode => $branchcode, >- } >- ); >- >- my $json_infos = JSON->new->utf8->pretty->encode($infos); >- $json_infos =~ s/"/'/g; >- >- logaction( >- "CIRCULATION", "RENEWAL", >- $borrower->{'borrowernumber'}, >- $json_infos, >- ) if C4::Context->preference("IssueLog"); >+ push @{$confirmations}, 'ON_RESERVE'; > } > > if ( $data->{error} && $data->{error} eq 'too_many' && C4::Context->preference('AllowRenewalLimitOverride') ) { > $data->{renew_okay} = 1; > $data->{error} = undef; >- >- $message = "Override limit Renew"; >- @message = ("Override limit Renew"); >- >- my $infos = ( >- { >- message => \@message, >- borrowernumber => $borrowernumber, >- barcode => $item->barcode, >- manager_id => $user, >- branchcode => $branchcode, >- } >- ); >- >- my $json_infos = JSON->new->utf8->pretty->encode($infos); >- $json_infos =~ s/"/'/g; >- >- logaction( >- "CIRCULATION", "RENEWAL", >- $borrower->{'borrowernumber'}, >- $json_infos, >- ) if C4::Context->preference("IssueLog"); >+ push @{$confirmations}, 'RENEWAL_LIMIT' if $override_limit; > } > > if ( $data->{renew_okay} || ( $seen && $data->{error} eq 'too_unseen' ) ) { >diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t >index aa5821daf0b..ab52b04cb52 100755 >--- a/t/db_dependent/Circulation.t >+++ b/t/db_dependent/Circulation.t >@@ -19,7 +19,7 @@ use Modern::Perl; > use utf8; > > use Test::NoWarnings; >-use Test::More tests => 80; >+use Test::More tests => 81; > use Test::Exception; > use Test::MockModule; > use Test::Deep qw( cmp_deeply ); >@@ -7907,6 +7907,25 @@ subtest 'Bug 9762: AddRenewal override JSON logging' => sub { > is_deeply( $log_data->{forced}, ['TOO_MUCH'], 'Forced overrides logged correctly' ); > }; > >+subtest 'Bug 40866: CanBookBeIssued returns correct number of values' => sub { >+ plan tests => 2; >+ >+ my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $patron = >+ $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $library->branchcode } } ); >+ my $item = $builder->build_sample_item( { library => $library->branchcode } ); >+ >+ t::lib::Mocks::mock_userenv( { patron => $patron, branchcode => $library->branchcode } ); >+ >+ # CanBookBeIssued should return exactly 4 values after Bug 40866 cleanup >+ # (issuingimpossible, needsconfirmation, alerts, messages) >+ # The 5th return value (@message_log) from the original Bug 9762 implementation was removed >+ my @result = C4::Circulation::CanBookBeIssued( $patron, $item->barcode ); >+ >+ is( scalar @result, 4, 'CanBookBeIssued returns exactly 4 values' ); >+ ok( ref( $result[0] ) eq 'HASH', 'First return value is issuingimpossible hashref' ); >+}; >+ > $schema->storage->txn_rollback; > C4::Context->clear_syspref_cache(); > $branches = Koha::Libraries->search(); >diff --git a/t/db_dependent/Reserves.t b/t/db_dependent/Reserves.t >index 79d6796f82e..345a485a2fd 100755 >--- a/t/db_dependent/Reserves.t >+++ b/t/db_dependent/Reserves.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 70; >+use Test::More tests => 71; > use Test::NoWarnings; > use Test::MockModule; > use Test::Warn; >@@ -25,6 +25,7 @@ use Test::Warn; > use t::lib::Mocks; > use t::lib::TestBuilder; > >+use JSON qw( from_json ); > use MARC::Record; > use DateTime::Duration; > >@@ -2004,3 +2005,92 @@ subtest 'CheckReserves() item type tests' => sub { > > $schema->storage->txn_rollback; > }; >+ >+subtest 'Bug 40866: AddReserve override JSON logging' => sub { >+ plan tests => 8; >+ >+ $schema->storage->txn_begin; >+ >+ my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $patron = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { branchcode => $library->branchcode } >+ } >+ ); >+ my $biblio = $builder->build_sample_biblio(); >+ my $item = $builder->build_sample_item( >+ { >+ library => $library->branchcode, >+ biblionumber => $biblio->biblionumber >+ } >+ ); >+ >+ t::lib::Mocks::mock_userenv( { patron => $patron, branchcode => $library->branchcode } ); >+ t::lib::Mocks::mock_preference( 'HoldsLog', 1 ); >+ >+ # Clear any existing logs >+ Koha::ActionLogs->search( { module => 'HOLDS', action => 'CREATE' } )->delete; >+ >+ # Test 1: Normal hold without overrides - should log hold ID only >+ my $hold_id = C4::Reserves::AddReserve( >+ { >+ branchcode => $library->branchcode, >+ borrowernumber => $patron->borrowernumber, >+ biblionumber => $biblio->biblionumber, >+ priority => 1, >+ } >+ ); >+ ok( $hold_id, 'Hold created successfully' ); >+ >+ my $logs = Koha::ActionLogs->search( >+ { >+ module => 'HOLDS', >+ action => 'CREATE', >+ object => $hold_id >+ }, >+ { order_by => { -desc => 'timestamp' } } >+ ); >+ is( $logs->count, 1, 'One log entry created for normal hold' ); >+ my $log = $logs->next; >+ is( $log->info, $hold_id, 'Normal hold logs hold ID only' ); >+ >+ # Cancel the hold for next test >+ my $hold = Koha::Holds->find($hold_id); >+ $hold->cancel; >+ >+ # Test 2: Hold with override confirmation - should log JSON with confirmations >+ my $hold_id2 = C4::Reserves::AddReserve( >+ { >+ branchcode => $library->branchcode, >+ borrowernumber => $patron->borrowernumber, >+ biblionumber => $biblio->biblionumber, >+ priority => 1, >+ confirmations => ['HOLD_POLICY_OVERRIDE'], >+ forced => [] >+ } >+ ); >+ ok( $hold_id2, 'Hold with override created successfully' ); >+ >+ $logs = Koha::ActionLogs->search( >+ { >+ module => 'HOLDS', >+ action => 'CREATE', >+ object => $hold_id2 >+ }, >+ { order_by => { -desc => 'timestamp' } } >+ ); >+ is( $logs->count, 1, 'One log entry created for override hold' ); >+ $log = $logs->next; >+ >+ my $log_data = eval { from_json( $log->info ) }; >+ ok( !$@, 'Log info is valid JSON' ); >+ ok( exists $log_data->{confirmations}, 'JSON contains confirmations array' ); >+ is_deeply( >+ $log_data->{confirmations}, >+ ['HOLD_POLICY_OVERRIDE'], >+ 'Confirmations logged correctly' >+ ); >+ >+ $schema->storage->txn_rollback; >+}; >diff --git a/t/db_dependent/api/v1/holds.t b/t/db_dependent/api/v1/holds.t >index e98d8b125e0..f0a55194ef7 100755 >--- a/t/db_dependent/api/v1/holds.t >+++ b/t/db_dependent/api/v1/holds.t >@@ -17,7 +17,7 @@ > > use Modern::Perl; > >-use Test::More tests => 18; >+use Test::More tests => 19; > use Test::NoWarnings; > use Test::MockModule; > use Test::Mojo; >@@ -25,6 +25,7 @@ use t::lib::TestBuilder; > use t::lib::Mocks; > > use DateTime; >+use JSON qw( from_json ); > use Mojo::JSON qw(encode_json); > > use C4::Context; >@@ -32,6 +33,7 @@ use Koha::Patrons; > use C4::Reserves qw( AddReserve CanItemBeReserved CanBookBeReserved ); > use C4::Items; > >+use Koha::ActionLogs; > use Koha::Database; > use Koha::DateUtils qw( dt_from_string output_pref ); > use Koha::Biblios; >@@ -1885,3 +1887,71 @@ subtest 'PUT /holds/{hold_id}/lowest_priority tests' => sub { > > $schema->storage->txn_rollback; > }; >+ >+subtest 'Bug 40866: API hold creation with override logs confirmations' => sub { >+ plan tests => 6; >+ >+ $schema->storage->txn_begin; >+ >+ my $librarian = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { flags => 1 } >+ } >+ ); >+ my $password = 'thePassword123'; >+ $librarian->set_password( { password => $password, skip_validation => 1 } ); >+ my $userid = $librarian->userid; >+ >+ my $patron = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ value => { flags => 0 } >+ } >+ ); >+ >+ my $library = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $biblio = $builder->build_sample_biblio; >+ >+ t::lib::Mocks::mock_preference( 'AllowHoldPolicyOverride', 1 ); >+ t::lib::Mocks::mock_preference( 'HoldsLog', 1 ); >+ >+ # Clear any existing logs >+ Koha::ActionLogs->search( { module => 'HOLDS', action => 'CREATE' } )->delete; >+ >+ my $post_data = { >+ patron_id => $patron->id, >+ biblio_id => $biblio->id, >+ pickup_library_id => $library->branchcode, >+ }; >+ >+ # Create hold with override >+ my $hold_id = >+ $t->post_ok( "//$userid:$password@/api/v1/holds" => { 'x-koha-override' => 'any' } => json => $post_data ) >+ ->status_is(201)->tx->res->json->{hold_id}; >+ >+ ok( $hold_id, 'Hold created via API with override' ); >+ >+ # Verify the hold was logged with override information >+ my $logs = Koha::ActionLogs->search( >+ { >+ module => 'HOLDS', >+ action => 'CREATE', >+ object => $hold_id >+ }, >+ { order_by => { -desc => 'timestamp' } } >+ ); >+ >+ is( $logs->count, 1, 'One log entry created for API hold with override' ); >+ >+ my $log = $logs->next; >+ my $log_data = eval { from_json( $log->info ) }; >+ ok( !$@, 'Log info is valid JSON' ); >+ is_deeply( >+ $log_data->{confirmations}, >+ ['HOLD_POLICY_OVERRIDE'], >+ 'HOLD_POLICY_OVERRIDE logged in confirmations array' >+ ); >+ >+ $schema->storage->txn_rollback; >+}; >-- >2.39.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40866
:
186985
|
186986
|
187063
|
187116
|
187627
|
187628
|
187629
|
187630
|
187631
|
187632
|
187633
|
187675
|
187676
|
187677
|
187678
|
187679
|
187680
|
187681
|
187682
|
187683
|
187684
|
187685
|
187686
|
187687
|
187688
|
187689
|
187690
|
187691
|
187692
|
187693
|
187694
| 187707 |
187725