https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39302 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Checkin disappear from |Checkins can disappear from |checkin list if transfer |checkin list if transfer |modal is triggered |modal is triggered -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39302 --- Comment #1 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 179160 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=179160&action=edit Bug 39302: Fix logic flaw in the display count of checkins Checkins can disappear from the checkin list if transfer modal is triggered. There is a flaw in the logic of the checkin code: we keep 20 (default value for numReturnedItemsToShow) checkins in the table, but there can be shift in the "counter" logic when the transfer modal is triggered. This patch is removing the different hashes and use a single @checkins array to pass to the template. When a item is checked in, it is inserted at the beginning of the list (unshift). Koha::Objects are passed for patron, item and biblio to improve the readability (and maintenance cost) of the code. Test plan: Generate several checkouts (see below) Some should generate the transfer modal Check them in => Without this patch notice that at some point there will be a problem in the list, some checkins will be missing. => With this patch applied the checkin list always contain the last 20 checkins (or whatever is the value of numReturnedItemsToShow, or "8" if pref is empty, don't ask why 8!) Code to generate 50 checkouts (all overdue): ``` use C4::Circulation qw( AddIssue ); use Koha::Items; use Koha::Patrons; use t::lib::Mocks; use Koha::DateUtils qw(dt_from_string); my $patron = Koha::Patrons->find(51); t::lib::Mocks::mock_userenv({patron => $patron}); my $items = Koha::Items->search; my $patron_unblessed = $patron->unblessed; my $i; while ( my $item = $items->next ) { my $due = dt_from_string->subtract(days => 45 + rand(45))->set_hour(23)->set_minute(59)->set_second(00); AddIssue($patron, $item->barcode, $due); last if ++$i >= 50; } ``` Then retrieve the barcodes:

SELECT items.barcode FROM issues LEFT JOIN items ON items.itemnumber=issues.itemnumber;

QA: 1. Do not be afraid by the length of this patch, most of the changes are variable renames. 2. return_overdue is now only calculated once per checkin (per page load, could be improved) 3. date formatting is done template-side -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39302 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #179160|0 |1 is obsolete| | --- Comment #2 from Owen Leonard <oleonard@myacpl.org> --- Created attachment 179169 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=179169&action=edit Bug 39302: Fix logic flaw in the display count of checkins Checkins can disappear from the checkin list if transfer modal is triggered. There is a flaw in the logic of the checkin code: we keep 20 (default value for numReturnedItemsToShow) checkins in the table, but there can be shift in the "counter" logic when the transfer modal is triggered. This patch is removing the different hashes and use a single @checkins array to pass to the template. When a item is checked in, it is inserted at the beginning of the list (unshift). Koha::Objects are passed for patron, item and biblio to improve the readability (and maintenance cost) of the code. Test plan: Generate several checkouts (see below) Some should generate the transfer modal Check them in => Without this patch notice that at some point there will be a problem in the list, some checkins will be missing. => With this patch applied the checkin list always contain the last 20 checkins (or whatever is the value of numReturnedItemsToShow, or "8" if pref is empty, don't ask why 8!) Code to generate 50 checkouts (all overdue): ``` use C4::Circulation qw( AddIssue ); use Koha::Items; use Koha::Patrons; use t::lib::Mocks; use Koha::DateUtils qw(dt_from_string); my $patron = Koha::Patrons->find(51); t::lib::Mocks::mock_userenv({patron => $patron}); my $items = Koha::Items->search; my $patron_unblessed = $patron->unblessed; my $i; while ( my $item = $items->next ) { my $due = dt_from_string->subtract(days => 45 + rand(45))->set_hour(23)->set_minute(59)->set_second(00); AddIssue($patron, $item->barcode, $due); last if ++$i >= 50; } ``` Then retrieve the barcodes:

SELECT items.barcode FROM issues LEFT JOIN items ON items.itemnumber=issues.itemnumber;

QA: 1. Do not be afraid by the length of this patch, most of the changes are variable renames. 2. return_overdue is now only calculated once per checkin (per page load, could be improved) 3. date formatting is done template-side Signed-off-by: Owen Leonard <oleonard@myacpl.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39302 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |baptiste.wojtkowski@biblibr | |e.com QA Contact|testopia@bugs.koha-communit |baptiste.wojtkowski@biblibr |y.org |e.com --- Comment #3 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- I'm losing the transfer information I previously had in the array (see attachments). Bug looks fixed otherwise -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39302 --- Comment #5 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Created attachment 179198 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=179198&action=edit Behaviour without patch -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39302 --- Comment #7 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Baptiste Wojtkowski (bwoj) from comment #3)

I'm losing the transfer information I previously had in the array (see attachments). Bug looks fixed otherwise

Oops, good catch. All item's info were actually missing! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39302 Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #179169|0 |1 is obsolete| | --- Comment #8 from Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski@biblibre.com> --- Created attachment 179219 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=179219&action=edit Bug 39302: Fix logic flaw in the display count of checkins Checkins can disappear from the checkin list if transfer modal is triggered. There is a flaw in the logic of the checkin code: we keep 20 (default value for numReturnedItemsToShow) checkins in the table, but there can be shift in the "counter" logic when the transfer modal is triggered. This patch is removing the different hashes and use a single @checkins array to pass to the template. When a item is checked in, it is inserted at the beginning of the list (unshift). Koha::Objects are passed for patron, item and biblio to improve the readability (and maintenance cost) of the code. Test plan: Generate several checkouts (see below) Some should generate the transfer modal Check them in => Without this patch notice that at some point there will be a problem in the list, some checkins will be missing. => With this patch applied the checkin list always contain the last 20 checkins (or whatever is the value of numReturnedItemsToShow, or "8" if pref is empty, don't ask why 8!) Code to generate 50 checkouts (all overdue): ``` use C4::Circulation qw( AddIssue ); use Koha::Items; use Koha::Patrons; use t::lib::Mocks; use Koha::DateUtils qw(dt_from_string); my $patron = Koha::Patrons->find(51); t::lib::Mocks::mock_userenv({patron => $patron}); my $items = Koha::Items->search; my $patron_unblessed = $patron->unblessed; my $i; while ( my $item = $items->next ) { my $due = dt_from_string->subtract(days => 45 + rand(45))->set_hour(23)->set_minute(59)->set_second(00); AddIssue($patron, $item->barcode, $due); last if ++$i >= 50; } ``` Then retrieve the barcodes:

SELECT items.barcode FROM issues LEFT JOIN items ON items.itemnumber=issues.itemnumber;

QA: 1. Do not be afraid by the length of this patch, most of the changes are variable renames. 2. return_overdue is now only calculated once per checkin (per page load, could be improved) 3. date formatting is done template-side Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39302 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |25.05.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=39302 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #11 from Fridolin Somers <fridolin.somers@biblibre.com> --- Hard to apply on 24.11.x Please provide a rebased patch -- You are receiving this mail because: You are watching all bug changes.