https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 --- Comment #1 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Created attachment 180239 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180239&action=edit Bug 38356: Check patrons current checkouts separately If syspref CheckPrevCheckout is enabled, Koha checks if patron has previously checked out item within timeperiod set in syspref CheckPrevCheckoutDelay. This however doesn't take into account cases where item is currently checked out for patron and they are trying to check out another item from same record (can be done if syspref AllowMultipleIssuesOnABiblio is enabled). We should always check if item is currently checked out for the patron if CheckPrevCheckout is enabled. To test: 1. Enable syspref CheckPrevCheckout and set CheckPrevCheckoutDelay as e.g. 10. Also enable syspref AllowMultipleIssuesOnABiblio. 2. Find a record that has been checked out for patron over 10 days ago. 3. Attempt to check out item from same record for same patron. => Note that you have to confirm if you want to check out item for the patron. 4. Find record that is currently checked out for a patron. 5. Attempt to check out item from same record for same patron. => Note that item is checked out for patron without confirm message. 6. Apply this patch and restart services if needed. 7. Repeat steps 2. and 3. => Confirm that you still have to confirm if you want to check out item for the patron. 8. Repeat steps 4. and 5. => New confirm message "Patron has this title currently checked out:..." should be displayed and you have to confirm if you want to check out item for the patron. Also prove t/db_dependent/Patron/Borrower_PrevCheckout.t Sponsored-by: Koha-Suomi Oy -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 Felicie <felicie.thiery@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180239|0 |1 is obsolete| | --- Comment #2 from Felicie <felicie.thiery@biblibre.com> --- Created attachment 180305 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=180305&action=edit Bug 38356: Check patrons current checkouts separately If syspref CheckPrevCheckout is enabled, Koha checks if patron has previously checked out item within timeperiod set in syspref CheckPrevCheckoutDelay. This however doesn't take into account cases where item is currently checked out for patron and they are trying to check out another item from same record (can be done if syspref AllowMultipleIssuesOnABiblio is enabled). We should always check if item is currently checked out for the patron if CheckPrevCheckout is enabled. To test: 1. Enable syspref CheckPrevCheckout and set CheckPrevCheckoutDelay as e.g. 10. Also enable syspref AllowMultipleIssuesOnABiblio. 2. Find a record that has been checked out for patron over 10 days ago. 3. Attempt to check out item from same record for same patron. => Note that you have to confirm if you want to check out item for the patron. 4. Find record that is currently checked out for a patron. 5. Attempt to check out item from same record for same patron. => Note that item is checked out for patron without confirm message. 6. Apply this patch and restart services if needed. 7. Repeat steps 2. and 3. => Confirm that you still have to confirm if you want to check out item for the patron. 8. Repeat steps 4. and 5. => New confirm message "Patron has this title currently checked out:..." should be displayed and you have to confirm if you want to check out item for the patron. Also prove t/db_dependent/Patron/Borrower_PrevCheckout.t Sponsored-by: Koha-Suomi Oy Signed-off-by: Felicie <felicie.thiery@biblibre.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |lucas@bywatersolutions.com |y.org | CC| |lucas@bywatersolutions.com Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #180305|0 |1 is obsolete| | --- Comment #3 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Created attachment 181883 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=181883&action=edit Bug 38356: Check patrons current checkouts separately If syspref CheckPrevCheckout is enabled, Koha checks if patron has previously checked out item within timeperiod set in syspref CheckPrevCheckoutDelay. This however doesn't take into account cases where item is currently checked out for patron and they are trying to check out another item from same record (can be done if syspref AllowMultipleIssuesOnABiblio is enabled). We should always check if item is currently checked out for the patron if CheckPrevCheckout is enabled. To test: 1. Enable syspref CheckPrevCheckout and set CheckPrevCheckoutDelay as e.g. 10. Also enable syspref AllowMultipleIssuesOnABiblio. 2. Find a record that has been checked out for patron over 10 days ago. 3. Attempt to check out item from same record for same patron. => Note that you have to confirm if you want to check out item for the patron. 4. Find record that is currently checked out for a patron. 5. Attempt to check out item from same record for same patron. => Note that item is checked out for patron without confirm message. 6. Apply this patch and restart services if needed. 7. Repeat steps 2. and 3. => Confirm that you still have to confirm if you want to check out item for the patron. 8. Repeat steps 4. and 5. => New confirm message "Patron has this title currently checked out:..." should be displayed and you have to confirm if you want to check out item for the patron. Also prove t/db_dependent/Patron/Borrower_PrevCheckout.t Sponsored-by: Koha-Suomi Oy Signed-off-by: Felicie <felicie.thiery@biblibre.com> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 25.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 --- Comment #7 from Jonathan Druart <jonathan.druart@gmail.com> --- Also t/db_dependent/Circulation.t .. 68/76 # No tests run! t/db_dependent/Circulation.t .. 70/76 # Failed test 'No tests run for subtest "CanBookBeIssued + needsconfirmation message"' # at t/db_dependent/Circulation.t line 7094. DBIC result _type isn't of the _type BiblioMetadata at /kohadevbox/koha/Koha/Patron.pm line 854. # Looks like your test exited with 11 just after 71. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 --- Comment #9 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- (In reply to Jonathan Druart from comment #7)

Also t/db_dependent/Circulation.t .. 68/76 # No tests run! t/db_dependent/Circulation.t .. 70/76 # Failed test 'No tests run for subtest "CanBookBeIssued + needsconfirmation message"' # at t/db_dependent/Circulation.t line 7094. DBIC result _type isn't of the _type BiblioMetadata at /kohadevbox/koha/Koha/Patron.pm line 854. # Looks like your test exited with 11 just after 71.

As far as I can tell t/db_dependent/Circulation.t tests are now failing randomly. They passed for me 3 times out of 5. But if I backtrack to the previous patch added to the main before these patches (bug 39775: Remove no-sort def), tests seem to pass every time. Also the line where they fail points to $biblio->is_serial which tries to check biblios metadata and is not related to the changes made here. Could it be possible that build_object sometimes fails to create metadata for the biblio object? If that's the case here it's weird that moving code around in the do_check_for_previous_checkout method has this kind of effect to building the test objects. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 --- Comment #11 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- (In reply to Katrin Fischer from comment #10)

(In reply to Emmi Takkinen from comment #9)

...

(In reply to Jonathan Druart from comment #7)

...

Also t/db_dependent/Circulation.t .. 68/76 # No tests run! t/db_dependent/Circulation.t .. 70/76 # Failed test 'No tests run for subtest "CanBookBeIssued + needsconfirmation message"' # at t/db_dependent/Circulation.t line 7094. DBIC result _type isn't of the _type BiblioMetadata at /kohadevbox/koha/Koha/Patron.pm line 854. # Looks like your test exited with 11 just after 71.

As far as I can tell t/db_dependent/Circulation.t tests are now failing randomly. They passed for me 3 times out of 5. But if I backtrack to the previous patch added to the main before these patches (bug 39775: Remove no-sort def), tests seem to pass every time. Also the line where they fail points to $biblio->is_serial which tries to check biblios metadata and is not related to the changes made here. Could it be possible that build_object sometimes fails to create metadata for the biblio object? If that's the case here it's weird that moving code around in the do_check_for_previous_checkout method has this kind of effect to building the test objects.

Hi Emmi, thanks! I wonder if is_serial breaks because of Bug 39835 - Tidy is_boolean / tinyint?

My guess is that probably not. I changed the way test item is build in "CanBookBeIssued + needsconfirmation message" tests and now tests pass every time I run them. Will provide patch shortly so someone else can also confirm this. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 --- Comment #13 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Created attachment 182064 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=182064&action=edit (follow-up) Bug 38356: Fix failing Circulation.t tests After pushing bug 38356 to main, Circulation.t tests started to fail. Fail happened randomly and was related to sub is_serial trying to fetch biblios metadata. This patch changes the way test item is build in "CanBookBeIssued + needsconfirmation message" tests. It seems more reliable to use build_sample_item rather than building item using biblio and biblioitem data. To test prove t/db_dependent/Circulation.t. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 --- Comment #15 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- (In reply to Katrin Fischer from comment #14)

Sorry, hope not bothering you: patch looks good to me. But I'd like to push together with the fixes for SIP if possible to save some Jenkins time. Are you working on these as well?

Ah, forgot to say that I'm now checking those SIP tests. I'll add separate patch when I'm done. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #182064|0 |1 is obsolete| | --- Comment #17 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Created attachment 182065 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=182065&action=edit Bug 38356: (follow-up) Fix failing Circulation.t tests After pushing bug 38356 to main, Circulation.t tests started to fail. Fail happened randomly and was related to sub is_serial trying to fetch biblios metadata. This patch changes the way test item is build in "CanBookBeIssued + needsconfirmation message" tests. It seems more reliable to use build_sample_item rather than building item using biblio and biblioitem data. To test prove t/db_dependent/Circulation.t. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 --- Comment #19 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Both tests should now pass. Had to modify Circulation.t patches title so that QA tool is happy with it. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 --- Comment #21 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- (In reply to Katrin Fischer from comment #20)

Hi Emmi,

thanks a lot!

Please check you are running latest ktd and maybe "nuke" your node_modules directory once to avoid these:

OK C4/SIP/ILS/Transaction/Checkout.pm

FAIL t/db_dependent/Circulation.t FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl t/db_dependent/Circulation.t`

I've tidied the patch before pushing, but it should happen automatically on commit with the hooks now.

Follow-ups pushed to main.

Thanks for pointing this out. I'm not using ktd but my qa tool was indeed pointing out those messy lines. Thank you for fixing them! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |If release notes| |AllowMultipleIssuesOnABibli | |o is enabled patrons can | |checkout multiple items | |from same record but these | |check outs were not checked | |by Koha. This patch changes | |CheckPrevCheckout | |systempreferences | |functionality so that it | |also checks patrons current | |check outs and displays new | |confirmation message | |"Patron has this title | |currently checked out:..." | |so that librarians now have | |to confirm if item will be | |check out to patron. --- Comment #22 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Added release notes. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|If |If release notes|AllowMultipleIssuesOnABibli |`AllowMultipleIssuesOnABibl |o is enabled patrons can |io` is enabled patrons can |checkout multiple items |checkout multiple items |from same record but these |from same record but these |check outs were not checked |check outs were not checked |by Koha. This patch changes |by Koha. This patch changes |CheckPrevCheckout |`CheckPrevCheckout` system |systempreferences |preference functionality so |functionality so that it |that it also checks patrons |also checks patrons current |current check outs and |check outs and displays new |displays new confirmation |confirmation message |message "Patron has this |"Patron has this title |title currently checked |currently checked out:..." |out:..." so that librarians |so that librarians now have |now have to confirm if item |to confirm if item will be |will be check out to |check out to patron. |patron. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38356 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Needs documenting CC| |fridolin.somers@biblibre.co | |m --- Comment #23 from Fridolin Somers <fridolin.somers@biblibre.com> --- Enhancement not pushed to 24.11.x -- You are receiving this mail because: You are watching all bug changes.