https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42643 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42643 [Bug 42643] [OMNIBUS] Assorted performance and stability work -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Initiative type|--- |Epic Target Milestone|--- |26.11 Strategic theme|--- |Modernisation -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Vue-based interface for |Migrate check-in to Vue |check-in | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43017 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43017 [Bug 43017] [OMNIBUS] Interface optimization -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |blawlor@clamsnet.org -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |43454 Depends on|43454 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43454 [Bug 43454] Add REST endpoint to cancel a transfer (POST /transfers/{transfer_id}/cancellation) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|24401 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24401 [Bug 24401] Add POST /checkins endpoint with availability check and confirmation flow -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- This is submitted to a branch, because of the many deps and constant required rebases it would pollute the report. Once we get some eyes on it and the dependencies move, I can submit here. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Benjamin Daeuber <bdaeuber@cityoffargo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bdaeuber@cityoffargo.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #4 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Hi, this is fully implemented as I mentioned on my previous comment. A few more fixes were added in the meantime, mostly cosmetic. I'll try to upload a full test plan. What I did was launching the current version and the new ones in parallel using KTD, and replicating the same scenarios covering all UC from the UC file. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Lisette Scheer <lisette@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |zoho.roboto@bywatersolution | |s.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Alex Carver [Acerock7] <alex@rcls.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |alex@rcls.org --- Comment #6 from Alex Carver [Acerock7] <alex@rcls.org> --- Hiya Tomás-- I'm working through the test plan and wanted to share some things I've found. I'll map them to the same sections in the plan: 1. Core check-in flow I don't see the sidenav when in checkin.pl at all Non-public notes on items do not appear in check-in table, though there is a note column. Public notes also do not appear in my testing. On main, they appear in red in this column. 2. Check in Options "Effective Checkin Date" message does not appear when clicking book drop mode. Book drop mode sets return date as today's date. Double checked that library calendar was set correctly for the logged in library. While using "Specify return date" and using a date earlier than the checkout date, the book stays on the patron record. "Forgive overdue charges" option not available 3. Item Status Alerts NotForLoan status is not updated on Checkin. Tested with _ALL_ 2:0 setting in the syspref -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #8 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Hi folks. I'll be travelling tomorrow for KohaCon but I'll try to re-run my side-by-side check and try to address your findings. I didn't know about bug 42344 so, oh well. Thanks for pointing that out! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Brendan Lawlor <blawlor@clamsnet.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #10 from Brendan Lawlor <blawlor@clamsnet.org> --- I used Claude Opus 5.5 to test this with ktd proxy running current main and the bug's branch side by side. Most of the things that it caught I was able to confirm. Here's Claude's findings in order of severity with some notes from me: Tested branch bug_42383_vuejs_checkin on KTD against the full test plan, using real data and a headless browser. Most of the plan passes; the problems are below. FAILURES 1. HoldsAutoFill is ignored (test plan 4.6) With HoldsAutoFill on, the hold found modal still appears and the hold is not filled. Nothing in the new code reads the preference. Legacy returns.pl called ModReserveAffect and requested a transfer when needed. - Confirmed with manual human testing 2. Hold cancellation reason is never saved (4.4) HoldCancellationModal.vue binds r.authorised_value, but /authorised_value_categories/.../authorised_values returns "value". The options have no real value, so no reason is sent to hold_cancellation and the cancelled hold has cancellation_reason NULL. - Confirmed with manual human testing using ktd --dbshell after cancelling a hold 3. Wrong transfers are not repaired (5.4) Item in transit MPL -> FPL, checked in at CPL: the "Wrong transfer detected" modal is correct, but the transfer is left as MPL -> FPL. Legacy returns.pl replaced it (request_transfer with replace => 'WrongTransfer', ignore_limits => 1). "Yes, transfer" can't fix it either: the store only calls transfer_confirmation when entry.transfer_id is set, and it is null here. - Confirmed with Manual testing and verifying in ktd --dbshell. My understanding of the legacy checkin is that whe a wrong transfer is detected the original row in branchtransfers should get a datecancelled and cancellation_reason = 'WrongTransfer'. A new branch transfer should be created for the new transfer with the datesent. 4. Bundle "Confirm checkin and mark missing items as lost" does nothing (9.2) verified_bundle_barcodes is in the API spec (paths/checkins.yaml) but no Perl code reads it. With 2 of 3 bundle items scanned, the missing item was not set to BundleLostValue. Legacy did this in returns.pl (and removed unexpected items from the bundle). The Title and Author columns in the verification table are also empty: the component reads biblio_title / biblio_author, which the items response doesn't provide. - Confirmed with manual human testing. Confirm chekcin and mark missing items as lost does not update the item's lost status. Title and Author don't display in the confirm bundle contents table, and 5. "Remove from bundle" does nothing (3.5) The button in the in_bundle alert (Main.vue) has no click handler; onMessagesClick only handles .resolve-claim-btn. "View host item" also links to the checked-in item's own biblio (latest.item.biblio.biblio_id) rather than the host's. - Confirmed with Manual testing, Remove from bundle button does not work and View host item links to the bundled item instead of host 6. "Print checkin slip" opens the quick slip (10.5) slip-printer.js opens circulation.pl?borrowernumber=X&print=qslip. Legacy opened members/printslip.pl?borrowernumber=X&print=checkinslip. - Confirmed with manual human testing. Also this just opened the route in a new windo, not the actual print dialog 7. Patrons with privacy "Never" (10.5) With AnonymousPatron set, the row shows the anonymous patron's name and offers "Print checkin slip" for the anonymous patron. The embedded checkout is already anonymised (privacy 1), so the privacy check never hides the button. - Confirmed with manual human testing. I guess the Anonymous patron name and print checkin slip button should not be displayed in this case 8. Checkout note (issue.note) not shown on a normal return (8.3) The note is in the API response but is only rendered inside ConfirmationModal, so it appears only when a hold/transfer/recall modal opens, and the date is shown as a raw ISO timestamp. Legacy showed the "Patron note" box, with a formatted date, on every return that had one. - Confirmed with manual human testing. It's an ISO timestamp instead of a formatted date, so it's a very minor issue. 9. Circulation sidebar never shows (1.5) checkin.tt still uses Koha.Preference('CircSidebar'), which was removed by bug 42344. All other circ templates now use [% WRAPPER 'main-container.inc' aside='circ-nav' %]. - Confirmed by manual human testing. SMALLER ISSUES - Every normal return gets an "Already returned" badge. It comes from the WasReturned message, which just means the checkout was returned; legacy never displayed it. - Confirmed by manual human testing. It should just say 'Returned' - Hold for another library (4.3): the modal says "Hold at <library>" / "Confirm hold (Y)" rather than "Transfer to" / "Confirm hold and transfer", because the response has no needs_transfer message. The transfer itself is created correctly on confirm. - Confirmed by manual human testing - Ignore (I) on a hold modal marks the row "Not checked in" although the item was checked in (dismissPending is used for post-checkin actions as well). -Confirmed by manual human testing - Re-checking in an item already waiting for a hold (4.5): the modal doesn't say the hold is already waiting (legacy: "Hold found (item is already waiting)"). - Confirmed by manual human testing. No item is already waiting message' - "No, don't transfer (N)" cancels the transfer, but the row still shows "Transferred" and "Transfer to <library>". - Confirmed by manual human testing - Invalid barcode shows "Item not found"; legacy showed "No item with barcode: X". - Confirmed with manual human testing - UpdateNotForLoanStatusOnCheckin: shown only as a raw "not for loan status updated" badge, without the from/to values. - Patron column: the name is not a link and there is no checkouts count (checkouts_count isn't embedded). - Confirmed by manual human testing - When a lost item is returned and a new overdue fine is charged, both "refund applied" and "refund applied and new overdue charge calculated" are shown; legacy showed one (IF/ELSIF). - "Forgive fees for manually expired holds" is emitted by CheckinOptions but never sent by the store. - Koha::Module::Policy::Checkin sets recalls_enabled from preference('UseRecalls') ? 1 : 0, which is true for 'off'. It should compare with ne 'off'. Nothing in the Vue code reads it yet. PASSED - Core flow: scan, barcode clears and refocuses, fast scans processed independently (newest first), not checked out, RecordLocalUseOnReturn. - Options: book drop (warning highlight, fine uses the drop-box date), forgive overdue charges, return date override, "Remember return date", book drop and forgive combined; exempt_fine hidden without updatecharges: writeoff, and a forced request gets a 403 with X-Koha-Module-Policy (payload exempt_fine = 0). - BlockReturnOfWithdrawnItems / BlockReturnOfLostItems blocking modals and the withdrawn warning; UpdateNotForLoanStatusOnCheckin changes the status. - Hold found modal details (patron, category, address, phone, email, SMS, restricted, address in doubt, notification methods, main contact method), confirm (waiting), confirm for another library (transfer created), Print slip and confirm, WaitingNotifyAtCheckin. - Transfers: blocking and non-blocking modals, scanning while a modal is open, print transfer slip, AutomaticConfirmTransfer, transfer arrived, AllowReturnToBranch=homebranch blocking. - Recalls: modal details, confirm (waiting), confirm with transfer (in_transit). - Lost fee refunded / remains / charged, processing fee message, FineNotifyAtCheckin with payment link. - Restriction messages (new, previously restricted, indefinite), return claim warning, resolve claim modal (PUT /return_claims/{id}/resolve), AutoClaimReturnStatusOnCheckin. - CircConfirmItemParts confirm and deny; bundle verification modal UI, progress counter, confirm without verifying, cancel. - Table: TablesSettings column hiding, numReturnedItemsToShow cap, overdue flag, Blocked / Not checked in indicators. - Network error message and retry; the page and the API are refused without circulate_remaining_permissions. - API: GET /checkins/availability (blockers/confirms/warnings, no side effects), POST by item_id and external_id, 412 with confirmation token then 200, 400 without an identifier, 404 for an unknown item; hold_confirmation, hold_cancellation, transfer_confirmation, transfer_cancellation, recall_confirmation. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #11 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Rechecked all of Alex's feedback in Comment 9. All issues addressed successfully except: "With `TransfersBlockCirc` off, and 'AutomaticConfirmTransfer` on, modal appears briefly but disappears before user interacts with it. Unsure if that's intended behavior of these two sysprefs together." On current main, if TransfersBlockCirc is off the modal is dismissed when one clicks anywhere outside the modal. With these patches, the modal will not go away until one interacts with it. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #13 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Andrew Fuerste-Henry from comment #12)

(In reply to Andrew Fuerste-Henry from comment #11)

...

Rechecked all of Alex's feedback in Comment 9. All issues addressed successfully except:

"With `TransfersBlockCirc` off, and 'AutomaticConfirmTransfer` on, modal appears briefly but disappears before user interacts with it. Unsure if that's intended behavior of these two sysprefs together."

On current main, if TransfersBlockCirc is off the modal is dismissed when one clicks anywhere outside the modal. With these patches, the modal will not go away until one interacts with it.

Correcting myself slightly -- the modal isn't dismissed by clicking elsewhere in the window, but it is dismissed by checkin in another item. I don't feel this is a blocker.

I'm starting to think my 'cool idea' of keeping the focus on the input and allowing super quick scanning and async processing was not the best idea, as y'all can see some of this changes as regressions... If you feel I should make it more strictly similar to the old behavior, I can do it tomorrow morning. I really like the current implementation and would like to keep it, though. Mentioning in case it is a strong blocker for y'all. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #15 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- (In reply to Brendan Lawlor from comment #10) Checked the issues Brendan found previously, 3 are still problematic:

- UpdateNotForLoanStatusOnCheckin: shown only as a raw "not for loan status updated" badge, without the from/to values.

When updating an item to a not for loan value of 0, the interface just shows 0. Previously, it said "Available for loan"

- When a lost item is returned and a new overdue fine is charged, both "refund applied" and "refund applied and new overdue charge calculated" are shown; legacy showed one (IF/ELSIF).

I didn't get a message about my new overdue fine being calculated. Steps to reproduce: - set a circ rule with an overdue fine - set lost item refund policy to "refund lost item charge and charge new overdue fine" - have an item with a replacement cost - check the item out with a due date in the past - mark the item lost - confirm your patron has a lost fee but no overdue fine - check the item in - there is no message about new overdue charge - confirm your patron's lost fee for refunded and a new overdue charge was created

- "Forgive fees for manually expired holds" is emitted by CheckinOptions but never sent by the store.

When I check the "Forgive fees for manually expired holds" checkbox, my hold would not cancel manually. Steps to recreate: - set a value in ExpireReservesMaxPickUpDelayCharge - place a hold, capture item for hold - from the db, update the hold expirationdate to a date in the past - go to checkin page, check box for "Forgive fees for manually expired holds" - check item in, get "item is already waiting" modal, click Cancel Hold - get hold cancellation reason modal, click Cancel Hold - get "item is already waiting" modal again, repeat until bored -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #18 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Rebased on top of main now that bug 42418 is pushed. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #20 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209553 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209553&action=edit Bug 42383: Add Koha::REST::Plugin::Policy This patch adds a Mojolicious plugin that provides the attach_module_policy helper to any API controller. Usage: $c->attach_module_policy('Checkin'); $c->attach_module_policy('Checkin', { library => $library_id }); The helper dynamically loads the appropriate Koha::Module::Policy subclass, generates the JWT, and sets the X-Koha-Module-Policy response header. This avoids duplicating policy attachment logic across controllers. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/checkins.t => SUCCESS: Tests pass 3. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #22 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209555 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209555&action=edit Bug 42383: Add Koha::Checkin->cancel_hold This patch adds a cancel_hold() domain method to Koha::Checkin that cancels the trapped hold and clears the hold_id on the checkin record. Accepts an optional hashref with a 'reason' key for the cancellation reason (authorised value from HOLD_CANCELLATION). Throws Koha::Exceptions::MissingParameter if the checkin has no hold_id. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Checkin.t => SUCCESS: Tests pass 3. Tests cover: - cancel_hold without reason (no notification) - cancel_hold with reason (letter warning expected, reason stored) - cancel_hold without hold_id (throws MissingParameter) 4. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #24 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209557 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209557&action=edit Bug 42383: Add Koha::Checkin->cancel_transfer This patch adds a cancel_transfer() domain method to Koha::Checkin that cancels the associated transfer and clears the transfer_id. If UseRecalls is enabled and the item has a recall in transit, the recall transfer is also reverted. Throws Koha::Exceptions::MissingParameter if the checkin has no transfer_id. No fuzzy fallbacks. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Checkin.t => SUCCESS: Tests pass 3. Tests cover: - cancel_transfer cancels the transfer (datecancelled set) - transfer_id cleared on the checkin - cancel_transfer without transfer_id (throws MissingParameter) 4. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #26 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209559 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209559&action=edit Bug 42383: Add checkin sub-resource confirmation/cancellation endpoints Adds the hold/transfer/recall confirmation and cancellation sub-resource endpoints on top of the checkin REST API: * POST /checkins/{checkin_id}/hold_confirmation * POST /checkins/{checkin_id}/hold_cancellation * POST /checkins/{checkin_id}/transfer_confirmation * POST /checkins/{checkin_id}/transfer_cancellation * POST /checkins/{checkin_id}/recall_confirmation Each delegates to the matching Koha::Checkin method and renders a shared 201 response via _render_checkin_response (Location header included). Module policy (X-Koha-Module-Policy) is attached only on the permission error paths (exempt_fine, forgive_hold_fees, return_date), not on success responses: the Vue app is hydrated with the policy at template load time, so recomputing and signing the policy JWT on every successful checkin would waste CPU. The checkin definition gains the full `messages` documentation and the availability definition its confirms/item_parts additions. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove xt/api.t t/db_dependent/api/v1/checkins.t => SUCCESS: Tests pass! 3. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #28 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209561 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209561&action=edit Bug 42383: Add Vue.js checkin page Adds the Vue-based checkin (returns) page that consumes the checkin REST API and its sub-resource confirmation/cancellation endpoints: * circ/checkin.pl + checkin.tt entry point * Vue module, store and components (BarcodeInput, CheckedInItems, CheckinOptions, ConfirmationModal, ResolveClaimModal, BundleVerificationModal, Main) under vue/components/Circ/Checkin/ * checkin-api-client fetch client, registered in the Vue api-client * rspack: new `checkin` entry building to js/vue/dist/checkin.js * http-client: honour return_response on non-ok responses so callers can inspect the error body/headers * Koha::Checkin->verify_bundle for the bundle verification modal * docs/checkin-vue-usecases.md The Vue app reads its module policy from the page hydration at load time (X-Koha-Module-Policy is not fetched per request). Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ yarn build => SUCCESS: rspack compiles, js/vue/dist/checkin.js is produced 3. Enable the page, go to Circulation -> Check in (checkin.pl), scan a barcode => SUCCESS: item is checked in, confirmations/messages render 4. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #30 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209563 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209563&action=edit Bug 42383: Add tests for checkin sub-resource endpoints and messages Adds API test coverage for the Bug 42383 additions: * sub-resource confirmation endpoints (hold/transfer confirmation and cancellation): not-found, no-associated-resource (400), and the happy paths (201 + Location, hold set waiting, transfer set in transit / cancelled) * add - post-checkin messages: was_lost message serialization; the not-issued case keeps the 24401 confirmation flow (412 confirmation_required) * add - return_date and dropbox_mode coverage * X-Koha-Module-Policy header on error paths: the module policy JWT is attached on the exempt_fine permission error (403), not on success responses (the Vue app is hydrated with the policy at template load) Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/checkins.t => SUCCESS: Tests pass! 3. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #32 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209565 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209565&action=edit Bug 42383: (follow-up) Match the legacy hold/transfer checkin modals Bring the Vue checkin confirmation modal in line with the legacy returns page for the hold and transfer cases: - Show the item as a "Title (barcode)" heading, linked to moredetail.pl - Show the patron note recorded on the checkout - Show library names and the "Hold at" / "Checked in at" headings - Match the legacy hold modal title and "Hold for" layout - Match the legacy transfer modal behaviour - Offer hold cancellation from the hold modal via a secondary (nested) modal rather than an inline action Test plan: 1. Exercise hold-found and transfer checkins on the Vue checkin page => SUCCESS: the confirmation modal matches the legacy returns page 2. From a hold-found checkin, cancel the hold => SUCCESS: cancellation is confirmed through a secondary modal 3. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #34 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209567 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209567&action=edit Bug 42383: (follow-up) Flesh out the recall found checkin modal The recall modal was a stub (title + an empty "Recall found" line). Bring it in line with the legacy returns.pl recall dialog: * "Recall placed by:" bulleted patron block (name link, address, phone, email) from the embedded recall.patron. * Recall notes, when present. * "Transfer required for recall" hint when the recall needs a transfer. * "Recall at <library name>" heading from the recall pickup library. * Add the "Ignore (I)" button alongside Confirm / Print and confirm. Test plan: 1. Apply patch, yarn build 2. Recall an item (UseRecalls) checked out to another patron, then check it in => SUCCESS: the modal shows who placed the recall, the pickup library, and Confirm / Print and confirm / Ignore 3. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #36 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209569 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209569&action=edit Bug 42383: (follow-up) Match legacy behaviour for blocked, bundle, waiting-hold, claim and local-use checkins Round out the Vue checkin page so the remaining returns.pl use cases match the legacy behaviour: - Blocked check-ins (e.g. wrong branch) open a "Cannot check in" modal with the destination library and accompanying-materials note - The bundle verification status column is consistent (Verified / Not verified / Lost) - The waiting-holds alert names the patron (via the checkout.patron embed) and is shown first - The return-claim modal shows the lost/fee check-in messages inline and uses the standard button palette and "Ignore" idiom - Not-issued items are checked in directly via x-koha-override, recording local use when RecordLocalUseOnReturn is set - "Print slip and confirm" for holds and recalls ports bug 42505: an empty popup is opened during the click (so it is not blocked) and the hold slip is only loaded into it after the confirm_hold/confirm_recall call succeeds, so waitingdate/expirationdate are current on the first print. The popup is closed if the confirmation fails. Test plan: 1. Exercise each of the above checkin scenarios on the Vue checkin page => SUCCESS: behaviour and layout match the legacy returns page 2. Set HoldsAutoFill to "Don't", add Waiting since: [% hold.waitingdate | $KohaDates %] to the HOLD_SLIP notice, check in an item with a hold for the check-in library and click "Print slip and confirm" => SUCCESS: The popup is not blocked and the slip shows today's waiting date on the first print 3. prove t/db_dependent/api/v1/checkins.t => SUCCESS 4. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) Assisted-by: Opus 5.5 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #38 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209571 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209571&action=edit Bug 42383: (QA follow-up) Address check-in page QA findings This patch addresses the check-in page findings from QA (comments 9 and 10), using the API changes from the previous patch: - Sidebar: checkin.tt used the CircSidebar preference, removed by bug 42344, so the circulation sidebar never showed. - Notes column: read the API keys (staff_notes, public_notes, internal_notes) so patron and item notes are displayed. - Patron column: link the patron and show the checkouts count (checkout.patron.checkouts+count). Anonymized checkouts (privacy "Never") no longer show the AnonymousPatron or a checkin slip button. - Print checkin slip opens members/printslip.pl?print=checkinslip. - Book drop mode shows the effective checkin date. - HoldsAutoFill: auto-filled holds don't open the hold modal; a "Hold filled for" box with a Print button is shown, and the slip is printed when HoldsAutoFillPrintSlip is set. - AutomaticConfirmTransfer: the transfer modal no longer confirms itself after a second. As on the legacy page, a non-blocking transfer is confirmed when staff move on (scan the next item). - Hold cancellation: the reason is now sent (wrong authorised value key) together with "Forgive fees for manually expired holds". - "Ignore" on hold, recall and claim modals keeps the row as checked in instead of marking it "Not checked in". - "No, don't transfer" clears the transfer from the row. - Hold modal: "Transfer to" / "Confirm hold and transfer" for holds at another library, and "Hold found (item is already waiting)". - Bundles: the verification table shows title and author, all scanned barcodes are sent (unexpected items are removed from their bundle), missing and unexpected items are reported, "Remove from bundle" works and links to the host record. - Messages: no "Already returned" badge on every return, a single lost item fee outcome (as the legacy IF/ELSIF), not for loan and shelving location changes show old and new values, the checkout note is shown on every return with a formatted date, and an unknown barcode shows "No item with barcode: X". - Values interpolated into the message boxes are HTML-escaped. Test plan: 1. Apply this patch and rebuild: $ ktd --shell k$ yarn js:build k$ restart_all 2. Go to Circulation -> Check in and repeat the scenarios from the QA comments 9 and 10, comparing with the legacy page on main => SUCCESS: Behaviour matches the legacy page 3. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #40 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209573 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209573&action=edit Bug 42383: (QA follow-up) Ask to transfer when AutomaticItemReturn is disabled When an item checked in needs to go back to another library and AutomaticItemReturn is disabled, AddReturn does not create a transfer and only reports NeedsTransfer. The legacy page asked "Transfer now?" and created the transfer on "Yes". The new page showed no modal at all, so there was no way to start the transfer from the check-in page. This patch: - Makes Koha::Checkin->confirm_transfer accept to_library_id and reason. If the checkin has no transfer yet, it requests one to that library (reason defaults to Manual) and sets it in transit. - Makes POST /checkins/{checkin_id}/transfer_confirmation accept an optional body with to_library_id and reason. It returns 400 if there is no transfer and no valid library, and 409 if the transfer is not allowed by the branch transfer limits or one is already pending. - Makes the check-in page show the transfer modal for needs_transfer, with "Transfer now?" and a "No, don't transfer" button. "Yes" requests the transfer using the destination and reason the checkin reported. Test plan: 1. Set AutomaticItemReturn to "Don't automatically transfer" 2. Check out an item whose home library is not your logged in library 3. Go to Circulation > Check in and check the item in => FAIL: No modal is shown, the transfer cannot be started 4. Apply this patch, rebuild and restart: $ ktd --shell k$ yarn api:bundle && yarn js:build && restart_all 5. Repeat steps 2 and 3 => SUCCESS: The modal asks "Transfer now?" 6. Click "Yes, transfer" => SUCCESS: The row shows the transfer, the item is in transit to its home library (check the item details page) 7. Repeat steps 2 and 3, click "No, don't transfer" => SUCCESS: No transfer is created, the row shows no transfer 8. Run: k$ prove t/db_dependent/Koha/Checkin.t \ t/db_dependent/api/v1/checkins.t \ xt/api.t => SUCCESS: Tests pass! 9. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #42 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209575 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209575&action=edit Bug 42383: Do not set a hold aside for an item checked out in the meantime Bug 43803 made ModReserveAffect return 0 and leave the hold untouched when the item has been checked out again while the hold confirmation was pending (e.g. from another workstation). The legacy returns.pl displayed a warning and skipped the transfer in that case, but Koha::Checkin->confirm_hold ignored the return value: it created and sent a transfer to the pickup library for an item that is on loan, and the hold_confirmation endpoint reported success. This patch makes Koha::Checkin->confirm_hold throw the new Koha::Exceptions::Checkin::ItemCheckedOut exception (before any transfer is created) when ModReserveAffect refuses the hold, and makes POST /checkins/{checkin_id}/hold_confirmation return 409 with the 'item_checked_out' error code. The check-in page shows the error and keeps the hold pending. Test plan: 1. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Checkin.t \ t/db_dependent/api/v1/checkins.t => SUCCESS: Tests pass! 2. Make sure HoldsAutoFill is disabled 3. Place an item-level hold on an item checked out to patron A (hold for patron B, pickup at a different library) 4. Go to Circulation > Check in, check in the item => SUCCESS: The "Hold found" modal appears. Leave it open 5. From another tab, check out the item to patron C, confirming the checkout without cancelling the hold 6. Back on the first tab, click "Confirm hold" => SUCCESS: An error is displayed: "The item has been checked out in the meantime: the hold has not been set aside and remains pending" => SUCCESS: The hold is still pending and no transfer was created 7. Check the item in again and confirm the hold => SUCCESS: The hold is set in transit to the pickup library 8. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #44 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209577 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209577&action=edit Bug 42383: (QA follow-up) Show 'Available for loan' for a not for loan value of 0 When UpdateNotForLoanStatusOnCheckin sets an item's not for loan status to 0, the check-in page displayed the raw '0' as the new value. The avDescription helper only treated undef and the empty string as 'Available for loan', while the legacy returns.tt treated any false value (including 0) that way. This patch adds notForLoanDescription, which shows 'Available for loan' for any value that is not a non-zero number, and locationDescription for the UpdateItemLocationOnCheckin message. The latter also fixes an empty shelving location being displayed as 'Available for loan': it now shows 'Blank', as the legacy page did. Test plan: 1. Set UpdateNotForLoanStatusOnCheckin to: BK: 1: 0 2. Check out a BK item, then set its not for loan status to 1 3. Check the item in on the check-in page => FAIL: The message reads 'Not for loan status updated. Old value: ... New value: 0.' 4. Apply this patch and rebuild the JS: $ ktd --shell k$ yarn build 5. Repeat steps 2 and 3 => SUCCESS: The new value reads 'Available for loan' 6. Set UpdateItemLocationOnCheckin to: _BLANK_: CART and check in an item with no shelving location => SUCCESS: The old value reads 'Blank' 7. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #46 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209579 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209579&action=edit Bug 42383: (QA follow-up) Add tests for forgive_hold_fees on hold_cancellation This patch adds tests for POST /checkins/{checkin_id}/hold_cancellation with forgive_hold_fees. They highlight that the endpoint rejects the parameter: the request body schema does not allow it, so the request fails with a 400 and the hold is never cancelled. The tests cover: - 403 (no_permission_for_forgive_hold_fees) without updatecharges.writeoff - No ExpireReservesMaxPickUpDelayCharge fee when the fees are forgiven - The fee being charged when they are not Test plan: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/checkins.t => FAIL: Tests fail! The forgive_hold_fees requests return 400 3. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #48 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209581 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209581&action=edit Bug 42383: (QA follow-up) Keep the ONLYMESSAGE keyword out of check-in messages UpdateNotForLoanStatusOnCheckin accepts ONLYMESSAGE as a rule value, meaning 'display the not for loan status, do not change it'. _attach_messages_to_checkin passed the AddReturn NotForLoanStatusUpdated hash through as the not_for_loan_status_updated payload, so for those rules the API reported { from => X, to => 'ONLYMESSAGE' } and the check-in page had to know the syspref keyword to render it. This patch makes _attach_messages_to_checkin translate that case into its own not_for_loan_status_notice message, with a { status } payload. not_for_loan_status_updated is only used for actual changes, and its 'to' is always a not for loan value. The $messages hash returned by AddReturn (used by SIP) is unchanged. The check-in page renders the notice with the status description, as the legacy page did, and no longer checks for 'ONLYMESSAGE'. Test plan: 1. Apply this patch, rebuild the API bundle and the JS and restart: $ ktd --shell k$ yarn api:bundle k$ yarn build k$ restart_all 2. Set UpdateNotForLoanStatusOnCheckin to: _ALL_: 1: ONLYMESSAGE 3. Check out an item, set its not for loan status to 1 and check it in on the check-in page => SUCCESS: 'Not for loan' is displayed, and the item keeps its status 4. Set UpdateNotForLoanStatusOnCheckin to: _ALL_: 1: 0 5. Repeat step 3 => SUCCESS: 'Not for loan status updated. Old value: Not for loan. New value: Available for loan.' is displayed 6. Run: k$ prove t/db_dependent/Circulation.t \ t/db_dependent/Circulation/issue.t \ xt/api.t => SUCCESS: Tests pass! 7. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #50 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Initial QA feedback: 1) http-client.js line 44 you're early returning if return_response is set but we already default every DELETE and COUNT request in all Vue modules to return this so those endpoints will now skip the existing error handling. Either needs its own checkin specific flag or the error handling needs re-thinking 2) XSS in Main.vue - the checkin message is customisable in the staff interface and is then put directly into v-html with no parsing. In the old TT version html_line_break was used to filter the message but we have no fallback here 3) Fetching the page can checkin an item - checkout an item and then paste the following into the browser with the barcode of that item: localhost:8081/cgi-bin/koha/circ/checkin.pl?barcode=<barcode>. A checkin will be handled silently and will appear in the database table but not in the UI. This used to be a CSRF protected POST but now fires on a page GET 4) No barcode filtering - Legacy code called barcodedecode - itemBarcodeInputFilter and the item_barcode_transform plugin hook also no longer apply to check-in through the Checkins.pm endpoints 5) Holds queue not updated - BatchUpdateBiblioHoldsQueue isn't called anywhere in the process flow and skip_holds_queue is passed through the AddReturn flow so the holds queue never reflects the check-in 6) audio-alert classes are added but playSound is never called after the initial page load 7) printAndResolveRecall, ConfirmationModal.vue line 966 - pulls the reserve_id from recallInfo but recallInfo doesn't carry a reserve_id property payload => { recall_id => $recall->id, needs_transfer => $messages->{RecallNeedsTransfer} ? 1 : 0, }, 8) xt/tt_script_tidy.t fails 9) No cypress tests - some of the existing Selenium tests are no longer robust, for example basic_workflow.t matches a static page title which exists before the Vue page has even mounted so is no longer actually proving anything. The Selenium tests either need updating or replacing with Cypress tests. This is a huge re-write for which good test coverage is critical 10) ModReserveAffect no longer receives a desk_id - any waiting hold will lose their desk 11) If MarkIssueReturned anonymises a patron record, when _attach_messages_to_checkin access $checkin->checkout->patron it is accessing the anonymous patron and not the actual patron involved. Any fines or waiting holds are incorrectly notified 12) Checkin.pm line 265 you throw an error based on the unless condition but ModReserveAffect can either return 0 or silently return if the hold isn't found (e.g. was deleted). This seems like if could incorrectly throw the exception in some cases 13) The /checkins/*_confirmation and /checkins/*_cancellation endpoints seem like they should probably require the same JWT confirmation flow as some of the work in bug 24401? These could be used to alter holds or transfers multiple days after they are created 14) Koha/Manual.pm needs updating to the new page URLs 15) ResolveClaimModal.vue uses the fetch api instead of the APIClient pattern. It also emits 'resolved' regardless of the response status which is then marked as a success further up the chain, even if a 4/500 error is thrown by the API 16) CheckinOptions.vue specifies MM/DD/YYYY which conflicts with Koha's date formatting options. The date is also no validated, should this use a flatpickr window? 17) checkin.js - fetch used for libraries which already have an APIClient - APIClient is already imported in the file. There are also multiple error strings throughout the file which aren't wrapped for translations - please check and update them 18) Missing translation wrapper in ConfirmationModal.vue line 799 19) TODO in ConfirmationModal.vue line seems like a regression/missing feature - is there a follow-up for this? 20) ConfirmationModal.vue line 799 - should this use patron_to_html to format the name? Same in CheckedInItems.vue line 442 21) recall_confirmation API method has no unit test 22) Policy.pm - the global keys are added to the hash before the subclass which means they can be overwritten. Should this not be reversed? 23) JWT is used for Module::Policy classes but the signature is never read in the UI. Either we need to check the signature or move away from JWT as it isn't adding anything -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #52 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209603 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209603&action=edit Bug 42383: (QA follow-up) Map the check-in page to the manual This patch renames the 'circ/returns' key in Koha::Manual's mapping to 'circ/checkin', as circ/returns.pl has been replaced by circ/checkin.pl. Without it, the 'Help' link on the new check-in page points to the manual's main page instead of the check-in section. It addresses point 14 of the QA comment. Test plan: 1. Go to Circulation > Check in 2. Click the 'Help' link on the top right => FAIL: The manual's front page opens 3. Apply this patch and restart plack: $ ktd --shell k$ koha-plack --restart kohadev 4. Repeat 2 => SUCCESS: The manual opens on the 'Check in (returning)' section 5. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #54 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209605 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209605&action=edit Bug 42383: (QA follow-up) Apply barcode filters on check-in by barcode This patch makes POST /checkins run the passed external_id through C4::Circulation::barcodedecode before looking up the item. This applies the itemBarcodeInputFilter system preference and the item_barcode_transform plugin hook, as the legacy circ/returns.pl did. Before this patch, a barcode needing transformation (e.g. one scanned with spaces when itemBarcodeInputFilter is 'whitespace') could not be checked in, and returned a 404. It addresses point 4 of the QA comment. Test plan: 1. Set the itemBarcodeInputFilter system preference to 'Remove spaces from' 2. Check out an item to a patron 3. Go to Circulation > Check in 4. Check in the item, typing its barcode with a space in the middle => FAIL: 'No item with barcode' is displayed 5. Apply this patch and restart plack: $ ktd --shell k$ koha-plack --restart kohadev 6. Repeat 4 => SUCCESS: The item is checked in 7. Run: k$ prove t/db_dependent/api/v1/checkins.t => SUCCESS: Tests pass! 8. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #56 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209607 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209607&action=edit Bug 42383: (QA follow-up) Set confirmed holds waiting at the check-in desk This patch makes Koha::Checkin->confirm_hold pass the checkin's desk_id to ModReserveAffect, so a hold confirmed (or auto-filled through HoldsAutoFill) on the check-in page is set waiting at the desk the staff member is using, as the legacy circ/returns.pl did. The checkin row already records the desk from the user environment. It addresses point 10 of the QA comment. Test plan: 1. Enable UseCirculationDesks and create a desk for your library 2. Set the desk for your session (Set library and desk) 3. Place an item level hold for a patron, pickup at your library 4. Check the item in on the check-in page and confirm the hold 5. Look at the hold on the record's holds page => FAIL: The waiting hold has no desk 6. Apply this patch and restart plack: $ ktd --shell k$ koha-plack --restart kohadev 7. Repeat 3 to 5 => SUCCESS: The hold is waiting at your desk 8. Run: k$ prove t/db_dependent/Koha/Checkin.t => SUCCESS: Tests pass! 9. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #58 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209609 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209609&action=edit Bug 42383: (QA follow-up) Use the returning patron for check-in notices This patch makes AddReturn pass the patron the item was checked out to down to _attach_messages_to_checkin, which uses it for the patron_has_fines (FineNotifyAtCheckin) and patron_has_waiting_holds (WaitingNotifyAtCheckin) messages. Before, those messages were built from $checkin->checkout->patron. The checkin's checkout is the old_issues row, which MarkIssueReturned has already anonymised for patrons with privacy set to 'Never' (when AnonymousPatron is set). The messages were then computed for, and pointed to, the anonymous patron instead of the patron who returned the item. It addresses point 11 of the QA comment. Test plan: 1. Set AnonymousPatron to an existing patron, and enable FineNotifyAtCheckin and WaitingNotifyAtCheckin 2. Pick a patron with no fines, set their privacy to 'Never'. Give them a manual fee and a waiting hold at your library 3. Check out an item to them, then check it in on the check-in page => FAIL: No fines or waiting holds message is displayed 4. Apply this patch and restart plack: $ ktd --shell k$ koha-plack --restart kohadev 5. Repeat 3 => SUCCESS: The fines and waiting holds messages are displayed and link to the right patron 6. Run: k$ prove t/db_dependent/Circulation.t => SUCCESS: Tests pass! 7. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #60 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209611 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209611&action=edit Bug 42383: (QA follow-up) Reject follow-up actions on stale checkins This patch makes the checkin follow-up endpoints refuse to act on a checkin that is not the latest checkin of its item: - POST /checkins/{checkin_id}/hold_confirmation - POST /checkins/{checkin_id}/hold_cancellation - POST /checkins/{checkin_id}/transfer_confirmation - POST /checkins/{checkin_id}/transfer_cancellation - POST /checkins/{checkin_id}/recall_confirmation Before, they only checked the permission and that the checkin existed, so an old checkin_id could be used to alter holds, transfers or recalls long after the item had circulated again. A new Koha::Checkin->is_latest method tells whether a later checkin exists for the item. The endpoints now return a 409 with the 'stale_checkin' error_code when it is not the latest. The shared lookup is done in a new _get_actionable_checkin controller helper. It addresses point 13 (a) of the QA comment. Test plan: 1. Apply this patch 2. Run: $ ktd --shell k$ yarn api:bundle k$ prove t/db_dependent/Koha/Checkin.t \ t/db_dependent/api/v1/checkins.t \ xt/api.t => SUCCESS: Tests pass! 3. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #62 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209613 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209613&action=edit Bug 42383: (QA follow-up) Send module policies as plain JSON This patch drops the JWT serialization of module policies. The policy was signed with Koha::Token->encode_claims and sent in the X-Koha-Module-Policy header on some error responses, but nothing ever verified the signature: the page bootstraps from the plain JSON policy injected by circ/checkin.tt, and the client just base64-decoded the JWT payload. The signature added complexity and suggested a guarantee that did not exist. The policy is informational (the server validates every action), so it is now sent as a plain JSON object in the X-Koha-Module-Policy header, the same representation the page is bootstrapped with: - Koha::Module::Policy->as_jwt is removed - Koha::REST::Plugin::Policy sets the header to the JSON-encoded to_hashref - The check-in page parses the header as JSON - POD, OpenAPI header descriptions and tests are updated Koha::Token->encode_claims and decode_claims are kept, as the bug 24401 availability tokens use them. It addresses point 23 of the QA comment. Test plan: 1. Apply this patch, rebuild the API bundle and the Vue files, and restart plack: $ ktd --shell k$ yarn api:bundle k$ yarn js:build k$ koha-plack --restart kohadev 2. Log in as a staff member with the writeoff permission, open the check-in page and tick 'Forgive overdue charges' 3. In another tab, remove the updatecharges > writeoff permission from that staff member 4. Check in a checked out item on the first tab => SUCCESS: The check-in is refused and the 'Forgive overdue charges' option is no longer offered 5. Run: k$ prove t/db_dependent/Koha/Module/Policy.t \ t/db_dependent/Koha/Module/Policy/Checkin.t \ t/db_dependent/Koha/REST/Plugin/Policy.t \ t/db_dependent/api/v1/checkins.t \ xt/api.t => SUCCESS: Tests pass! 6. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #64 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209615 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209615&action=edit Bug 42383: (QA follow-up) Escape the item type check-in message The check-in page renders its alert boxes with v-html. Every value interpolated into them is escaped, except the item type check-in message (itemtypes.checkinmsg) used for the item_type_checkinmsg message, which was rendered as raw HTML. The legacy page displayed it with the html_line_break filter, which escapes the text and turns line breaks into <br>. This patch (QA comment point 2) escapes the message and converts its line breaks into <br>, matching the legacy rendering. The other alert boxes already escape the API provided data they interpolate. Test plan: 1. Go to Administration > Item types and edit the type of an item. Set 'Checkin message' to: <b>Bold</b><img src=x onerror=alert('XSS')> Second line 2. Check the item out, then check it in on the check-in page => FAIL: 'Bold' is rendered bold and a JS alert pops up 3. Apply this patch and rebuild: $ ktd --shell k$ yarn js:build k$ restart_all 4. Repeat step 2 => SUCCESS: The message is displayed literally, with the HTML tags visible, no alert pops up and 'Second line' is on its own line 5. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #66 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209617 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209617&action=edit Bug 42383: (QA follow-up) Play audio alerts after each check-in The check-in page marks the latest row with the audio-alert-warning or audio-alert-success classes, and the confirmation modal with audio-alert-action, so the default AudioAlerts selectors match them. But js_includes.inc only looks for the configured selectors and calls playSound() once, on document ready. The legacy page was reloaded after every check-in so it played a sound each time; the Vue page never did. This patch (QA comment point 6) adds a playAudioAlert() helper to the check-in page, which repeats the js_includes.inc logic: it walks window.AUDIO_ALERTS in precedence order and plays the sound of the first selector found in the page. It is called once the result of each check-in has been rendered, and when the next pending modal is opened after resolving one. The error box displayed for unknown barcodes and failed requests also gets the audio-alert-warning class, so these produce the warning sound like on the legacy page. Test plan: 1. Enable the AudioAlerts system preference 2. On the check-in page, check in: - an item that is checked out - a barcode that does not exist - an item with a hold on it => FAIL: No sound is played 3. Apply this patch and rebuild: $ ktd --shell k$ yarn js:build k$ restart_all 4. Repeat step 2 (check items out and place the hold again) => SUCCESS: The 'beep' sound plays for the normal check-in => SUCCESS: The 'critical' sound plays for the unknown barcode => SUCCESS: The 'opening' sound plays when the hold modal opens 5. Disable AudioAlerts and repeat step 2 => SUCCESS: No sound is played 6. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #68 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209619 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209619&action=edit Bug 42383: (QA follow-up) Use the API client to resolve return claims The 'Resolve return claim' modal of the check-in page called PUT /api/v1/return_claims/{claim_id}/resolve with a raw fetch() and emitted 'resolved' whatever the response was. Failures (e.g. a missing permission or a claim deleted in the meantime) were silently reported as success and the modal closed. This patch (QA comment point 15) adds a return_claims.resolve() method to ClaimAPIClient, built on HttpClient.put() like the other API clients, and uses it from the modal through APIClient. 'resolved' is only emitted when the request succeeds; otherwise the error is displayed in the modal, which stays open. Test plan: 1. Set ClaimReturnedLostValue to a lost value 2. Check an item out to a patron and, from the patron's checkouts table, mark it as 'Claim returned' 3. Check the item in on the check-in page 4. Click 'Resolve' on the row, pick a resolution and click 'Resolve claim' => SUCCESS: The claim is resolved 5. Repeat steps 2 to 3, open the resolve modal and, before submitting it, delete the claim from the patron's 'Claims' tab in another browser tab. Submit the modal => FAIL: The modal closes as if the claim had been resolved 6. Apply this patch and rebuild: $ ktd --shell k$ yarn js:build k$ restart_all 7. Repeat steps 2 to 4 => SUCCESS: The claim is resolved and the modal closes 8. Repeat step 5 => SUCCESS: The modal stays open and displays the error returned by the API 9. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #70 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209621 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209621&action=edit Bug 42383: (QA follow-up) Show the configured date format for the return date The check-in settings panel labelled the return date input 'Specify return date (MM/DD/YYYY):' whatever the dateformat system preference was. The input is a flatpickr date picker that uses the configured format, so the hint was wrong for every format but 'us'. The legacy page used date-format.inc to display the configured format. This patch (QA comment point 16) builds the hint from flatpickr_dateformat_string, set by calendar.inc from the dateformat system preference, so it reads e.g. DD/MM/YYYY, DD.MM.YYYY or YYYY-MM-DD as date-format.inc does. If it is not available, the label is displayed without a format. Test plan: 1. Enable SpecifyReturnDate and set dateformat to 'dd/mm/yyyy' 2. On the check-in page, open the check-in settings (sliders icon) => FAIL: The label reads 'Specify return date (MM/DD/YYYY):' 3. Apply this patch and rebuild: $ ktd --shell k$ yarn js:build k$ restart_all 4. Reload the page and open the settings => SUCCESS: The label reads 'Specify return date (DD/MM/YYYY):' 5. Try the other dateformat values => SUCCESS: The label matches the format used by the date picker 6. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #72 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209623 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209623&action=edit Bug 42383: (QA follow-up) Format patron names with $patron_to_html The check-in page built patron names by hand from surname and firstname in several places: the 'Returned by' column of the check-in table, the hold and recall patron in the confirmation modal, and the 'Hold filled for' and waiting holds alert boxes. This ignores the preferred name, middle name, other name and the 'A patron from X' fallback that the rest of the staff interface displays, and prints 'undefined' for patrons without a first name. The 'Patron #X' fallback was also not translatable. This patch (QA comment points 18 and 20) uses the global $patron_to_html helper, as other Vue components do, with invert_name (Surname, Firstname) and the card number where it was shown before. The 'Returned by' column uses its url option for the link to the patron's details page. 'Patron #%s' is wrapped with $__(). $patron_to_html escapes the patron data, so the names are rendered with v-html and no longer go through escapeHtml in the alert boxes. Test plan: 1. Create a patron with a first name, a middle name and a preferred name, and another one with only a surname 2. Check out items to both patrons, check them in on the check-in page => FAIL: The 'Returned by' column shows 'Surname, Firstname (card)', ignoring the preferred and middle names, and 'Surname, undefined' for the patron without a first name 3. Place a hold on an item for the first patron and check it in => FAIL: The hold modal shows the patron name built by hand too 4. Apply this patch and rebuild: $ ktd --shell k$ yarn js:build k$ restart_all 5. Repeat steps 2 and 3 => SUCCESS: Names are displayed as in the rest of the staff interface (e.g. 'Surname, Preferredname Middlename (cardnumber)'), linking to the same pages as before => SUCCESS: The patron without a first name shows only the surname 6. Repeat step 3 with HoldsAutoFill enabled => SUCCESS: The 'Hold filled for:' box shows the formatted name 7. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #74 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209625 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209625&action=edit Bug 42383: (QA follow-up) Do not return $__ from the check-in components The check-in components returned the $__ translation function from setup(). Vue reserves the '$' prefix for its internals and logs a warning on each mount in development builds: [Vue warn]: setup() return property "$__" should not start with "$" or "_" which are reserved prefixes for Vue internals. The function is already available to the templates as a global property, registered by the @koha-vue/i18n plugin the check-in app uses. This patch removes it from the setup() return values of the Circ/Checkin components. The Cypress support file fails the tests on any console warning, so this was making the check-in page Cypress tests fail. Test plan: 1. Run: $ ktd --shell k$ yarn js:build 2. Open the browser console and go to Circulation > Check in => FAIL: The Vue warning about "$__" is logged 3. Apply this patch, repeat 1 and 2 => SUCCESS: No warning 4. Check in some items, open the settings panel => SUCCESS: All the strings are displayed as before 5. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #76 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209627 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209627&action=edit Bug 42383: (QA follow-up) Make basic_workflow.t check in for real The check-in step of basic_workflow.t loaded circ/checkin.pl with the barcode URL parameter and checked the page title. The new page does not check in items from the URL anymore, and the title is rendered before the Vue app is even mounted, so this proved nothing. This patch addresses point 9 of the QA comment by making the test drive the Vue page like staff would: it waits for the app to be mounted, types each barcode, submits the form and waits for the item to be listed in the checked in items table, along with its title. It then checks in the database that the patron has no checkouts left. Test plan: 1. Apply this patch 2. Run: $ ktd --selenium up -d $ ktd --shell k$ prove -v t/db_dependent/selenium/basic_workflow.t => SUCCESS: Tests pass! The check-in tests now list the barcodes 3. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #78 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Re-uploaded the whole series. While working on point 23 we reworked the module policy design: instead of a header (signed JWT, then plain JSON), the policy is now returned as an optional, well-defined 'policy' attribute of the error object (module_policy envelope with typed 'global' and 'checkin' blocks, validated against the spec), the same structure the page is bootstrapped with. As it was a design change, it is folded into the original patches that introduced the policy code, so the 'Prevent module policies from overriding globals' and 'Send module policies as plain JSON' follow-ups are gone. All the other QA follow-ups are unchanged and still one per point. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #79 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209739 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209739&action=edit Bug 42383: Add Koha::Module::Policy framework This patch adds a module policy framework that exposes per-module capabilities to the UI. This replaces the pattern of dedicated GET /config endpoints (like GET /erm/config) with a reactive, session-scoped mechanism. A module policy is the intersection of: - System preferences (feature toggles) - User permissions (what this staff member can do) - Library context (branch-specific configuration) The policy is a plain JSON structure: an envelope with the `module` name, a `global` block with the keys common to all modules, and the module-specific keys under the module name: { module => 'checkin', global => { audio_alerts => true, ... }, checkin => { exempt_fine => false, ... }, } Flags are JSON booleans. Modules cannot override the global keys, as they live in their own namespace. The page is bootstrapped with the policy, and the API returns it in the error object when it refuses a request because of it, so the client can refresh the UI. The server remains authoritative - the policy is informational, not an access token. Components: - Koha::Module::Policy - Base class with new(), module(), to_hashref() and scope() - Koha::Module::Policy::Checkin - Checkin module policy covering fines, holds, recalls, transfers, display, and blockers Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Module/Policy.t \ t/db_dependent/Koha/Module/Policy/Checkin.t => SUCCESS: Tests pass 3. Tests cover: - Base class constructor validation - Abstract method enforcement - scope() derivation from class name - The policy envelope, and globals not being overridable by modules - Permission-gated capabilities (exempt_fine, forgive_hold_fees) - Superlibrarian vs limited user - Flags being JSON booleans 4. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #81 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209741 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209741&action=edit Bug 42383: Add Koha::Checkin->confirm_hold This patch adds a confirm_hold() domain method to Koha::Checkin that sets the trapped hold to waiting. If the pickup library differs from the checkin library, it also creates a transfer and sets it in transit. This moves the logic out of the controller into the domain object, making it testable independently and reusable (e.g. for HoldsAutoFill). Throws Koha::Exceptions::MissingParameter if the checkin has no hold_id. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Checkin.t => SUCCESS: Tests pass 3. Tests cover: - confirm_hold at same library (hold set to waiting, no transfer) - confirm_hold at different library (hold in transit, transfer created) - confirm_hold without hold_id (throws MissingParameter) 4. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #83 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209743 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209743&action=edit Bug 42383: Add Koha::Checkin->confirm_transfer This patch adds a confirm_transfer() domain method to Koha::Checkin that sets the associated transfer in transit (populates datesent). The method is idempotent — calling it on an already-sent transfer is a no-op. Throws Koha::Exceptions::MissingParameter if the checkin has no transfer_id. No fuzzy fallbacks to item->get_transfer. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Checkin.t => SUCCESS: Tests pass 3. Tests cover: - confirm_transfer sets transfer in transit - confirm_transfer is idempotent on already-sent transfer - confirm_transfer without transfer_id (throws MissingParameter) - Returns $self for chaining 4. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #85 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209745 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209745&action=edit Bug 42383: Add Koha::Checkin->confirm_recall This patch adds a confirm_recall() domain method to Koha::Checkin that resolves the recall associated with the checkin: - If the recall's pickup library matches the checkin library, sets the recall to waiting with a calculated expiration date. - If the pickup library differs, starts a transfer (sets status to in_transit). The method is idempotent — calling it on an already-resolved recall is a no-op. Throws Koha::Exceptions::MissingParameter if the checkin has no recall_id. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Checkin.t => SUCCESS: Tests pass 3. Tests cover: - confirm_recall at same library (set to waiting + expiration date) - confirm_recall at different library (set to in_transit) - Idempotent on already in-transit recall - confirm_recall without recall_id (throws MissingParameter) 4. Sign off :-D Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #87 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209747 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209747&action=edit Bug 42383: Add checkin messages and item-parts/bundle confirmations Extends the checkin flow with net-new behavior on top of the checkin audit record: * _attach_messages_to_checkin gains three independent messages: - item_type_checkinmsg: the item type's checkin message - patron_has_fines: when FineNotifyAtCheckin is on and the patron who had the item has an outstanding balance - patron_has_waiting_holds: when WaitingNotifyAtCheckin is on and the patron has holds waiting for pickup at the current branch * Koha::Item::Availability::Checkin gains two confirmations: - item_parts: multi-part item (CircConfirmItemParts) needs staff acknowledgement of the accompanying materials - items_bundle: bundle items need content verification The existing blocker/confirmation key names (NotIssued, BlockedWithdrawn, Wrongbranch, BlockedLost) are intentionally left unchanged; renaming them is out of scope here and would ripple into SIP, AddReturn and existing tests. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Circulation.t t/db_dependent/Koha/Item/Availability/Checkin.t t/db_dependent/Koha/Checkin.t => SUCCESS: Tests pass! 3. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #89 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209749 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209749&action=edit Bug 42383: Remove legacy returns.pl, wire checkin.pl Retires the legacy circ/returns.pl checkin page in favour of the new Vue circ/checkin.pl, and repoints every entry point to it: * circ-nav.inc, checkin-search-box.inc, circulation-home.tt, staff-global.js: 'Check in' links/search now target checkin.pl * ILL SupplyingResource.vue: checkin action targets checkin.pl * circ/returns.pl and circ/returns.tt are removed * Main.vue honours a ?barcode= deep-link query param so links carried over from returns.pl keep working (auto-submits the checkin) * basic_workflow.t selenium flow updated to the new page The transfers-to-receive page keeps its bug 42443 AJAX DataTable implementation (it already cancels transfers through the REST API and never linked to returns.pl), so its 42383-era changes are not needed. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ yarn build => SUCCESS: rspack compiles 3. From the circ home, nav sidebar, and header search 'Check in', confirm they all open circ/checkin.pl 4. Visit /cgi-bin/koha/circ/checkin.pl?barcode=<known barcode> => SUCCESS: the item is checked in automatically 5. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #91 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209751 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209751&action=edit Bug 42383: Use snake_case for checkin availability keys The checkin availability object (Koha::Item::Availability::Checkin) is surfaced directly to the API through GET /checkins/availability and the 412 confirmation response, so its blocker/confirmation/warning keys are API-facing values and should be standard, meaningful snake_case: NotIssued -> not_issued (confirmation) BlockedWithdrawn -> blocked_withdrawn (blocker) BlockedLost -> blocked_lost (blocker) Wrongbranch -> wrong_branch (blocker) { Wrongbranch, Rightbranch } -> { wrong_branch, right_branch } The keys are now documented in the checkin_availability definition. AddReturn keeps consuming the availability object internally, and still exposes its own legacy $messages hashref (Wrongbranch => { Wrongbranch, Rightbranch }, NotIssued, ...) unchanged, so C4::SIP and existing AddReturn callers/tests are unaffected: the snake_case values are mapped back to the legacy structure where AddReturn builds $messages. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Item/Availability/Checkin.t \ t/Koha/Result/Availability.t \ t/db_dependent/api/v1/checkins.t \ t/db_dependent/Circulation.t \ t/db_dependent/SIP/Transaction.t \ xt/api.t => SUCCESS: Tests pass! 3. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #93 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209753 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209753&action=edit Bug 42383: (follow-up) Show item type checkin message text The check in message area rendered the item_type_checkinmsg message as the raw "item type checkinmsg" key. Render its payload text instead (the item type's configured checkin message), matching what the legacy returns.pl dialog shows in its dedicated box. Test plan: 1. Apply patch, yarn build 2. Set a checkin message on an item type; check in such an item => SUCCESS: the modal shows the actual checkin message text 3. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #95 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209755 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209755&action=edit Bug 42383: (follow-up) Show patron notification info in hold and recall modals The legacy returns page shows how the patron will be notified in the hold and recall confirmation dialogs. Consume the notification_summary embed (Bug 43486) to render the same detail: - "Patron notification: Email, SMS." (or "Patron is not notified.") - "Main contact method: ..." The hold modal uses the Hold_Filled transports; the recall modal uses the Recall_Waiting transports (the notice that fires when a recall is set waiting on checkin). The boxes are styled like the legacy page. This wires hold.patron.notification_summary and recall.patron.notification_summary into the checkin embed (client and swagger enum) and renders the lines in ConfirmationModal. Test plan: 1. Have a patron with a Hold_Filled and/or Recall_Waiting messaging preference set, and place a hold/recall 2. Check the item in, yarn build => SUCCESS: the confirmation modal shows the patron notification line and the main contact method, matching the legacy returns page 3. Try a patron with no messaging preference => SUCCESS: the modal shows "Patron is not notified." in a grey box 4. Sign off :-D Assisted-by: Sonnet 4.6 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #97 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209757 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209757&action=edit Bug 42383: (QA follow-up) Restore legacy post-return processing in the checkin API QA testing (comments 9 and 10) found several things returns.pl did after AddReturn that POST /checkins did not. This patch adds them: - HoldsAutoFill: when a hold is found (and not already waiting) the hold is now filled on checkin, as returns.pl did, using Koha::Checkin->confirm_hold. A 'hold_auto_filled' message is added so the client can skip the hold modal. - Wrong transfers: a new Koha::Checkin->replace_wrong_transfer replaces the transfer with one from the checkin library (replace => 'WrongTransfer', ignore_limits => 1) and links it to the checkin, so transfer_confirmation/transfer_cancellation work on it. - Bundles: verified_bundle_barcodes is now processed. verify_bundle also sets datelastborrowed, records a return claim and charges the replacement fee (WhenLostChargeReplacementFee) for missing items, like returns.pl. 'bundle_missing_items' and 'bundle_unexpected_items' messages report the outcome. The patron is the one returned by AddReturn, as the stored checkout may already be anonymized. - Koha::Checkin->cancel_hold honours forgive_hold_fees (it was accepted by the endpoint but ignored), charging the cancellation fee otherwise. - The in_bundle message payload now carries the host biblio id and barcode, so the client can link to the host record. - checkout.patron.checkouts+count can be embedded. - The checkin module policy exposes dropbox_date and anonymous_patron_id, and recalls_enabled is now false when UseRecalls is 'off' (it was always true). Koha::Checkin->cancel_transfer had the same UseRecalls check. Test plan: 1. Apply this patch 2. Run: $ ktd --shell k$ yarn api:bundle k$ restart_all k$ prove t/db_dependent/Koha/Checkin.t \ t/db_dependent/api/v1/checkins.t \ t/db_dependent/Koha/Module/Policy/Checkin.t \ t/db_dependent/Circulation.t \ xt/api.t => SUCCESS: Tests pass! 3. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #99 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209759 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209759&action=edit Bug 42383: (QA follow-up) Fix check-in page findings from browser testing This patch fixes differences with the legacy returns.pl page found by testing both pages side by side in the browser. Changes: - Main.vue: a transfer modal disables the barcode input when TransfersBlockCirc is set, so a scanner cannot check in the next item behind it. The input is focused again once the transfer is resolved. - ConfirmationModal.vue: "Cancel hold" is only offered for holds that are already waiting, as on the legacy page. On an unconfirmed record-level hold it failed with a 500 (Koha::Hold->cancel needs an item to calculate the cancellation fee). - stores/checkin.js: confirming a hold that needs a transfer shows the transfer (destination and reason) on the row. - CheckedInItems.vue: due and acquisition dates are formatted with $date() (dateformat syspref) instead of the browser locale. - POST /checkins accepts the item+strings, hold.patron+strings and recall.patron+strings embeds. The table shows shelving location, item type and collection descriptions, and the hold/recall modals show the patron category description, instead of the codes. - The transfer modal no longer shows "Already returned": was_returned is set on every return and the legacy page never displayed it. - Patron card numbers are shown next to the patron names. - The item type check-in message gets a label in the status column. - The print button reads "Print slip, transfer, and confirm (P)" when the hold needs a transfer, as on the legacy page. Test plan: 1. Apply patch and rebuild: $ ktd --shell k$ yarn api:bundle && yarn js:build && restart_all 2. Set TransfersBlockCirc to "Block", AutomaticItemReturn to "Do", HoldsAutoFill to "Don't", and set your browser language to one that uses day/month dates (e.g. Spanish) 3. Check out to a patron: an item from another library (A), an item with a shelving location (B), an item with a record-level hold for pickup at another library (C) 4. Go to Circulation > Check in, check in A => SUCCESS: The transfer modal shows, the barcode input is disabled => SUCCESS: No "Already returned" message in the modal 5. Scan B with the modal open => SUCCESS: Nothing is checked in 6. Click "Yes, transfer" => SUCCESS: The barcode input is enabled and focused 7. Check in B => SUCCESS: Dates follow the dateformat syspref => SUCCESS: Shelving location and item type show their descriptions => SUCCESS: The patron card number is shown 8. Check in C => SUCCESS: The patron category description and card number show => SUCCESS: There is no "Cancel hold" button => SUCCESS: The print button reads "Print slip, transfer, and confirm" 9. Click "Confirm hold and transfer" => SUCCESS: The row shows the destination library and "Hold" reason 10. Check in an item with a waiting hold, click "Cancel hold", pick a reason and confirm => SUCCESS: The hold is cancelled with the chosen reason 11. Run: k$ prove t/db_dependent/api/v1/checkins.t xt/api.t => SUCCESS: Tests pass! 12. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #101 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209761 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209761&action=edit Bug 42383: (QA follow-up) Format restriction and return dates on the check-in page This patch fixes the remaining dates on the check-in page that did not follow the dateformat syspref: - The "Patron is now restricted" and "Patron was earlier restricted" messages now include the restriction end date, formatted with $date() as the legacy page did with $KohaDates. They used to omit the date. - The "Return date" badge shown while the options panel is collapsed displayed the raw ISO date (2026-10-10). It is now formatted. Test plan: 1. Apply patch and rebuild: $ ktd --shell k$ yarn js:build && restart_all 2. Set the 'Suspension in days' circulation rule to 1 and check out an overdue item to a patron (e.g. backdate the checkout) 3. Go to Circulation > Check in and check the item in => SUCCESS: "Patron is now restricted until <date>." uses the dateformat syspref 4. Check in another overdue item of the same patron => SUCCESS: "Reminder: Patron was earlier restricted until <date>." uses the dateformat syspref 5. Enable SpecifyReturnDate, open the check-in settings, pick a return date and collapse the settings => SUCCESS: The "Return date" badge uses the dateformat syspref 6. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #103 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209763 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209763&action=edit Bug 42383: (QA follow-up) Close non-blocking transfer modals on click outside or Escape With TransfersBlockCirc disabled, the legacy returns.pl transfer modals had no static backdrop: clicking outside of them or pressing Escape closed them, and with AutomaticConfirmTransfer enabled closing them confirmed the transfer. On the Vue check-in page the only way out of a non-blocking transfer modal was one of its buttons, or scanning the next item. This patch makes ConfirmationModal emit 'close' when a non-blocking transfer modal gets a click outside of the dialog or an Escape key press. Main.vue then confirms the transfer if the checkin has one and AutomaticConfirmTransfer is enabled, and otherwise moves on leaving the transfer as it is. Blocking transfer modals (TransfersBlockCirc enabled) and the other modals are unchanged. Test plan: 1. Apply this patch and rebuild the JS: $ ktd --shell k$ yarn build 2. Set TransfersBlockCirc to "Don't block" and AutomaticConfirmTransfer to "Do" 3. Put an item in transit to library A, and check it in at library B (wrong transfer) => SUCCESS: The "Please return item to A" modal shows 4. Click anywhere outside of the modal => SUCCESS: The modal closes => SUCCESS: The item shows as in transit to A on its details page 5. Repeat 3, press Escape instead of clicking outside => SUCCESS: Same as step 4 6. Set AutomaticConfirmTransfer to "Don't" and repeat 3 and 4 => SUCCESS: The modal closes and the transfer is not sent 7. Click inside the modal (e.g. on its text) => SUCCESS: The modal stays open 8. Set TransfersBlockCirc to "Block" and repeat 3 and 4 => SUCCESS: The modal stays open until one of its buttons is used 9. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #105 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209765 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209765&action=edit Bug 42383: (QA follow-up) Show the new overdue charge message on lost returns With the lost item refund policy 'Refund lost item charge and charge new overdue fine', AddReturn reports both lost_item_fee_refunded and lost_item_fee_charged. The check-in page shows a single lost fee outcome, picking the first one found in a list that started with lost_item_fee_refunded, so the new overdue charge was never mentioned. This patch makes lost_item_fee_charged and lost_item_fee_restored take precedence over lost_item_fee_refunded. Their texts already state that the lost item charge was refunded. Test plan: 1. Set a circulation rule with an overdue fine and finesMode to 'Calculate and charge' 2. Set the lost item refund policy (Default lost item fee refund on return policy) to 'Refund lost item charge and charge new overdue fine' 3. Check out an item with a replacement cost and a due date in the past, then mark it lost 4. Check the item in on the check-in page => FAIL: Only 'A refund for the lost item charge has been applied to the borrowing patron's account.' is displayed 5. Apply this patch and rebuild the JS: $ ktd --shell k$ yarn build 6. Repeat steps 3 and 4 => SUCCESS: The message also says 'and new overdue charge has been calculated and applied', and it is displayed only once 7. Check the patron's account => SUCCESS: The lost fee was refunded and a new overdue fine created 8. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #107 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209767 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209767&action=edit Bug 42383: (QA follow-up) Allow forgiving fees when cancelling a hold on check-in With 'Forgive fees for manually expired holds' checked, cancelling a waiting hold from the check-in page did nothing and the hold modal kept coming back. The page sends forgive_hold_fees to POST /checkins/{checkin_id}/hold_cancellation, but the request body schema did not include it and has additionalProperties: false, so the request was rejected with a 400. The error was swallowed by the store, the check-in stayed pending and its modal was shown again. This patch: - Adds forgive_hold_fees to the hold_cancellation body schema. The controller already handled it, including the updatecharges.writeoff permission check. - Makes the check-in store report a failed hold, transfer or recall action in the page error box, so a rejected action is no longer silently retried. - Documents the 403 no_permission_for_forgive_hold_fees error of hold_cancellation, which carries the current module policy in the `policy` attribute. The store applies it, so the option is no longer offered, and reports the refusal. Test plan: 1. Set ExpireReservesMaxPickUpDelayCharge to 5 2. Place a hold and check the item in to set it waiting 3. Set the hold's expiration date in the past: $ ktd --dbshell k> UPDATE reserves SET expirationdate = '2026-01-01' WHERE reserve_id = X; 4. On the check-in page, open the settings and check 'Forgive fees for manually expired holds' 5. Check the item in, click 'Cancel hold', then 'Cancel hold' in the cancellation pop-up => FAIL: The 'Hold found (item is already waiting)' modal comes back and the hold is not cancelled 6. Apply this patch, rebuild the API bundle and the JS and restart: $ ktd --shell k$ yarn api:bundle k$ yarn build k$ restart_all 7. Repeat steps 4 and 5 => SUCCESS: The hold is cancelled, the modal closes and the patron is not charged the expired hold fee 8. Run: k$ prove t/db_dependent/api/v1/checkins.t xt/api.t => SUCCESS: Tests pass! 9. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #109 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209769 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209769&action=edit Bug 42383: (QA follow-up) Clear the suggested transfer when closing 'Transfer now?' With AutomaticItemReturn disabled, the check-in only suggests a transfer (needs_transfer message) and the 'Transfer now?' modal asks whether to create it. Closing that modal with Escape or by clicking outside it created no transfer, as expected, but the row kept showing the suggested destination and reason in the 'Transfer to' and 'Transfer reason' columns. 'No, don't transfer' cleared them. This patch makes closing the modal without a transfer behave like 'No, don't transfer'. Closing a modal for an existing transfer is unchanged: it is confirmed when AutomaticConfirmTransfer is set, and left pending otherwise. Test plan: 1. Set TransfersBlockCirc and AutomaticItemReturn to "Don't" 2. Check in an item whose home library is not the current library => SUCCESS: The 'Transfer now?' modal is displayed 3. Press Escape => FAIL: No transfer is created, but the row still shows the home library and 'Return to home library' in the transfer columns 4. Apply this patch and rebuild the JS: $ ktd --shell k$ yarn build 5. Repeat steps 2 and 3, then once more clicking outside the modal instead of pressing Escape => SUCCESS: No transfer is created and the transfer columns are empty 6. Repeat step 5 with AutomaticConfirmTransfer set to "Do" => SUCCESS: Same result, no transfer is created 7. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #111 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209771 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209771&action=edit Bug 42383: (QA follow-up) Apply barcode filters on check-in by barcode This patch makes POST /checkins run the passed external_id through C4::Circulation::barcodedecode before looking up the item. This applies the itemBarcodeInputFilter system preference and the item_barcode_transform plugin hook, as the legacy circ/returns.pl did. Before this patch, a barcode needing transformation (e.g. one scanned with spaces when itemBarcodeInputFilter is 'whitespace') could not be checked in, and returned a 404. It addresses point 4 of the QA comment. Test plan: 1. Set the itemBarcodeInputFilter system preference to 'Remove spaces from' 2. Check out an item to a patron 3. Go to Circulation > Check in 4. Check in the item, typing its barcode with a space in the middle => FAIL: 'No item with barcode' is displayed 5. Apply this patch and restart plack: $ ktd --shell k$ koha-plack --restart kohadev 6. Repeat 4 => SUCCESS: The item is checked in 7. Run: k$ prove t/db_dependent/api/v1/checkins.t => SUCCESS: Tests pass! 8. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #113 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209773 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209773&action=edit Bug 42383: (QA follow-up) Set confirmed holds waiting at the check-in desk This patch makes Koha::Checkin->confirm_hold pass the checkin's desk_id to ModReserveAffect, so a hold confirmed (or auto-filled through HoldsAutoFill) on the check-in page is set waiting at the desk the staff member is using, as the legacy circ/returns.pl did. The checkin row already records the desk from the user environment. It addresses point 10 of the QA comment. Test plan: 1. Enable UseCirculationDesks and create a desk for your library 2. Set the desk for your session (Set library and desk) 3. Place an item level hold for a patron, pickup at your library 4. Check the item in on the check-in page and confirm the hold 5. Look at the hold on the record's holds page => FAIL: The waiting hold has no desk 6. Apply this patch and restart plack: $ ktd --shell k$ koha-plack --restart kohadev 7. Repeat 3 to 5 => SUCCESS: The hold is waiting at your desk 8. Run: k$ prove t/db_dependent/Koha/Checkin.t => SUCCESS: Tests pass! 9. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #115 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209775 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209775&action=edit Bug 42383: (QA follow-up) Use the returning patron for check-in notices This patch makes AddReturn pass the patron the item was checked out to down to _attach_messages_to_checkin, which uses it for the patron_has_fines (FineNotifyAtCheckin) and patron_has_waiting_holds (WaitingNotifyAtCheckin) messages. Before, those messages were built from $checkin->checkout->patron. The checkin's checkout is the old_issues row, which MarkIssueReturned has already anonymised for patrons with privacy set to 'Never' (when AnonymousPatron is set). The messages were then computed for, and pointed to, the anonymous patron instead of the patron who returned the item. It addresses point 11 of the QA comment. Test plan: 1. Set AnonymousPatron to an existing patron, and enable FineNotifyAtCheckin and WaitingNotifyAtCheckin 2. Pick a patron with no fines, set their privacy to 'Never'. Give them a manual fee and a waiting hold at your library 3. Check out an item to them, then check it in on the check-in page => FAIL: No fines or waiting holds message is displayed 4. Apply this patch and restart plack: $ ktd --shell k$ koha-plack --restart kohadev 5. Repeat 3 => SUCCESS: The fines and waiting holds messages are displayed and link to the right patron 6. Run: k$ prove t/db_dependent/Circulation.t => SUCCESS: Tests pass! 7. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #117 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209777 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209777&action=edit Bug 42383: (QA follow-up) Reject follow-up actions on stale checkins This patch makes the checkin follow-up endpoints refuse to act on a checkin that is not the latest checkin of its item: - POST /checkins/{checkin_id}/hold_confirmation - POST /checkins/{checkin_id}/hold_cancellation - POST /checkins/{checkin_id}/transfer_confirmation - POST /checkins/{checkin_id}/transfer_cancellation - POST /checkins/{checkin_id}/recall_confirmation Before, they only checked the permission and that the checkin existed, so an old checkin_id could be used to alter holds, transfers or recalls long after the item had circulated again. A new Koha::Checkin->is_latest method tells whether a later checkin exists for the item. The endpoints now return a 409 with the 'stale_checkin' error_code when it is not the latest. The shared lookup is done in a new _get_actionable_checkin controller helper. It addresses point 13 (a) of the QA comment. Test plan: 1. Apply this patch 2. Run: $ ktd --shell k$ yarn api:bundle k$ prove t/db_dependent/Koha/Checkin.t \ t/db_dependent/api/v1/checkins.t \ xt/api.t => SUCCESS: Tests pass! 3. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #119 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209779 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209779&action=edit Bug 42383: (QA follow-up) Do not check in items from the barcode URL param The check-in page kept the legacy ?barcode= deep link working by submitting a check-in for it on page load. That turns a GET request into a state change: any link or embedded resource pointing at circ/checkin.pl?barcode=XXX checks the item in for the logged in staff user. The legacy page required a CSRF protected POST (op=cud-checkin). This patch (QA comment point 3) makes the page only prefill the barcode input with the parameter value and focus it. Staff need to press 'Check in' (or Enter) to actually check the item in. A setBarcode() method is added to the BarcodeInput component for this. Test plan: 1. Check out an item to a patron 2. Visit /cgi-bin/koha/circ/checkin.pl?barcode=<the item barcode> => FAIL: The item is checked in on page load 3. Check the item out again 4. Apply this patch and rebuild: $ ktd --shell k$ yarn js:build k$ restart_all 5. Repeat step 2 => SUCCESS: The barcode input contains the barcode and has the focus => SUCCESS: Nothing is checked in, the item is still checked out 6. Press Enter => SUCCESS: The item is checked in 7. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #121 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209781 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209781&action=edit Bug 42383: (QA follow-up) Make returning error responses opt-in in HttpClient The check-in page needs the raw response of POST /checkins for error statuses too (412 confirmation, 403 blocked, 404 not found), so an earlier patch made HttpClient::_fetchJSON return the response object for non-2xx responses whenever return_response was set. But delete() and count() default return_response to true, so every DELETE and count request in the Vue apps stopped going through the error handling: errors were no longer thrown nor reported with setError(), and callers treated failures as successes. This patch (QA comment point 1) restores the previous behaviour for return_response and adds a separate return_error_response parameter, false by default. It is only passed by post(), and only the check-in client sets it (checkins.create, and checkins.cancelHold whose refusals carry the module policy). Test plan: 1. Apply this patch and rebuild: $ ktd --shell k$ yarn js:build k$ restart_all 2. Check in items on the check-in page, including: - an item that is not checked out - a barcode that does not exist - an item that requires a confirmation (e.g. a lost item with BlockReturnOfLostItems set) => SUCCESS: The page behaves as before: the right messages, modals and confirmation prompts are displayed 3. In the ERM module, try deleting an agreement that cannot be deleted (e.g. edit it in another tab and delete it there first, then delete it again from the first tab) => SUCCESS: The error is displayed at the top of the page, as it was before the check-in patches 4. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #123 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209783 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209783&action=edit Bug 42383: (QA follow-up) Print the recall pickup slip for recalls The 'Print slip and confirm (P)' button of the recall modal used recallInfo.reserve_id (falling back to the checkin hold_id) to load the hold transfer slip. The recall_found message payload only contains recall_id and needs_transfer, so no slip was ever printed, or the wrong one was. The legacy page opened recalls/recall_pickup_slip.pl?recall_id=X. This patch (QA comment point 7) adds a recallSlipUrl() helper to slip-printer.js and makes the button load the recall pickup slip for the payload recall_id, once the recall has been confirmed (same as the hold slip). Test plan: 1. Enable UseRecalls and set up circulation rules allowing recalls 2. Check an item out to patron A, then place a recall on it for patron B from the OPAC 3. Check the item in on the check-in page => SUCCESS: The recall modal is displayed 4. Click 'Print slip and confirm (P)' => FAIL: No slip, or an empty / wrong slip, is displayed 5. Apply this patch and rebuild: $ ktd --shell k$ yarn js:build k$ restart_all 6. Repeat steps 2 to 4 with a new item => SUCCESS: The recall is confirmed and the recall pickup slip for patron B is displayed in a popup 7. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #125 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209785 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209785&action=edit Bug 42383: (QA follow-up) Fix xt/tt_script_tidy.t for circ/checkin.tt xt/tt_script_tidy.t checks that inline <script> blocks of templates do not contain Template::Toolkit tags, unless they are explicitly allowed. circ/checkin.tt passes the module policy, the library and the table settings to the Vue app through a <script> block with TT tags, which made the test fail. This patch (QA comment point 8) marks that block with data-tt-allowed="true", like other templates (e.g. recalls/request.tt) do for script blocks that only expose server side values to JS. Test plan: 1. Run: $ ktd --shell k$ prove xt/tt_script_tidy.t => FAIL: The test fails for koha-tmpl/intranet-tmpl/prog/en/modules/circ/checkin.tt 2. Apply this patch and repeat step 1 => SUCCESS: Tests pass 3. Restart and load the check-in page: $ ktd --shell k$ restart_all => SUCCESS: The page loads and items can be checked in 4. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #127 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209787 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209787&action=edit Bug 42383: (QA follow-up) Use APIClient for libraries and translate store strings The check-in Pinia store fetched the libraries list with a raw fetch('/api/v1/libraries?_per_page=-1') while APIClient, which has a library client, is already imported and used for authorised values. It also set some user visible error messages as plain, untranslatable strings. This patch (QA comment point 17) loads the libraries through APIClient.library.libraries.getAll() and wraps the remaining English strings of the store with $__(). Test plan: 1. Apply this patch and rebuild: $ ktd --shell k$ yarn js:build k$ restart_all 2. Check an item whose home library is not the logged in library in on the check-in page, with AutomaticItemReturn enabled => SUCCESS: The 'Transfer to' column and the transfer modal show the home library name (not its code) 3. Check the store strings are picked up for translation: $ ktd --shell k$ cd misc/translator k$ perl translate update fr-FR k$ grep -A1 'Network error. Please try again.' po/fr-FR-messages-js.po => SUCCESS: The string is listed 4. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #129 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209789 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209789&action=edit Bug 42383: (QA follow-up) Restore hold group handling in the hold modal When DisplayAddHoldGroups is enabled and the hold found on check-in is part of a hold group, the legacy check-in page: - displayed 'Part of a hold group:' with a 'View patron's holds' link to the patron's holds, in the hold found modals - offered, for waiting holds, a 'Cancel all holds in hold group (X)' button calling POST /api/v1/patrons/{patron_id}/hold_groups/ {hold_group_id}/cancel with the selected cancellation reason, and labelled the single hold cancellation button 'Cancel this hold (X)' instead of 'Cancel hold (X)' The Vue page only had a TODO for it. This patch (QA comment point 19) ports that behaviour: - The hold group id comes from the embedded hold (hold.hold_group_id), already requested by the check-in client, and the display_hold_groups module policy flag (DisplayAddHoldGroups). No back-end change is needed. - A hold_groups.cancel() method is added to PatronAPIClient. - 'Cancel all holds in hold group' opens the existing cancellation pop-up, which asks for confirmation and the cancellation reason, and calls the new cancelHoldGroup() store action. On success the row is marked as checked in; on failure the error is displayed and the hold modal stays pending. Test plan: 1. Enable DisplayAddHoldGroups 2. Place a hold group for a patron on two records (staff interface, Place hold, search the patron, select 2 records and check 'Treat as hold group') 3. Check in an item of the first record => SUCCESS: The 'Hold found' modal is displayed => FAIL: There is no mention of the hold group 4. Confirm the hold, then check the item in again => FAIL: The 'Hold found (item is already waiting)' modal only offers 'Cancel hold (X)' 5. Apply this patch and rebuild: $ ktd --shell k$ yarn js:build k$ restart_all 6. Repeat step 4 => SUCCESS: The modal shows 'Part of a hold group:' and a 'View patron's holds' link opening the patron's holds in a new tab => SUCCESS: The buttons read 'Cancel all holds in hold group (X)' and 'Cancel this hold (X)' 7. Click 'Cancel all holds in hold group (X)', pick a reason and confirm => SUCCESS: The row is marked as checked in, and the patron's holds page shows both holds of the group as cancelled 8. Repeat steps 2 to 4 with DisplayAddHoldGroups disabled => SUCCESS: No hold group information is shown, and the button reads 'Cancel hold (X)' 9. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #131 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209791 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209791&action=edit Bug 42383: (QA follow-up) Add Cypress tests for the check-in page This patch adds end-to-end Cypress tests for the Vue check-in page (circ/checkin.pl), addressing point 9 of the QA comment. The data is created in the database using the Cypress tasks, the page is driven like staff would do it, and the outcome is verified in the database. The tests cover: - Checking in a checked out item: the row is displayed with the barcode and title, and the checkout is moved to old_issues - An unknown barcode displays an error - The barcode URL parameter prefills the input but does not check the item in - The item type check-in message is displayed as text, not HTML - A withdrawn item is not checked in with BlockReturnOfWithdrawnItems - Holds: confirming a hold for the logged in library sets it waiting, confirming a hold for another library puts it in transit with a transfer, and a waiting hold can be cancelled from the hold modal - Transfers: confirming the return to the home library sends the transfer, and TransfersBlockCirc blocks the barcode input meanwhile - Recalls: confirming a recall sets it waiting - Module policy: when the server refuses a return date because SpecifyReturnDate was disabled after the page was loaded, the page applies the policy returned in the error, tells why and no longer offers the return date The system preferences used are set by each test, and restored to their original values afterwards. All the created data is removed. Test plan: 1. Apply this patch 2. Run: $ ktd --shell k$ yarn cypress run --spec t/cypress/integration/Circulation/Checkin_spec.ts => SUCCESS: Tests pass! 3. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off --- Comment #133 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Thanks Matt! One follow-up per point: 1) Handle HoldsAutoFill errors on check-in - the item is already returned, so a HoldNotFound/ItemCheckedOut from confirm_hold now becomes a 'hold_auto_fill_failed' warning message on the checkin (with the error_code) instead of a 500. The holds queue and bundle logic still run. 2) Make checkin follow-up actions atomic - confirm_hold, cancel_hold, confirm_transfer, cancel_transfer, confirm_recall and replace_wrong_transfer run inside a transaction. Tests make the transfer step die and check the hold is untouched. 3) Mark the row of the resolved return claim - the modal emits the claim_id and the matching row is updated (Cypress test with two claimed items). 4) Restore the time and past-only return date - the picker takes a time and blocks future dates as the legacy page did. The API now takes a date-time and rejects future dates with 400 'return_date_in_future'. 5) The X-Koha-Module-Policy header is gone: the series was re-uploaded with the policy returned as an optional, well-defined 'policy' attribute of the error object instead (see the previous comment). 6) Fix the Location header of checkin actions - it is now the action path (e.g. /api/v1/checkins/1/hold_confirmation), without the action token query string. Back to Signed Off for your review. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #135 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209813 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209813&action=edit Bug 42383: (QA follow-up) Make checkin follow-up actions atomic Koha::Checkin->confirm_hold makes several writes: ModReserveAffect sets the hold aside, then a transfer is requested and sent, and the checkin is linked to it. They are not done in a transaction, so an error while creating or sending the transfer leaves the hold in transit with no transfer. This patch addresses point 2 of comment #77 by wrapping the writes of confirm_hold in a transaction. The other checkin actions writing more than once are wrapped the same way: - cancel_hold: cancels the hold and unlinks it from the checkin - confirm_transfer: requests the transfer, links it and sends it - cancel_transfer: cancels the transfer, reverts an in transit recall and unlinks the transfer from the checkin - confirm_recall: Koha::Recall->start_transfer updates the recall and creates the transfer, set_waiting updates the recall and the item - replace_wrong_transfer: replaces the transfer and links the new one confirm_transfer now checks its parameters before starting the transaction. The behaviour is unchanged. Test plan: 1. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Checkin.t => FAIL: When sending the transfer fails, the hold is left in transit and a transfer is created 2. Apply this patch 3. Run: k$ prove t/db_dependent/Koha/Checkin.t \ t/db_dependent/api/v1/checkins.t => SUCCESS: Tests pass! 4. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42383 --- Comment #137 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 209815 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=209815&action=edit Bug 42383: (QA follow-up) Restore the time and past-only return date On the legacy returns page, the return date override took a time (data-flatpickr-enable-time) and future dates could not be picked (data-flatpickr-pastinclusive). The check-in page only offers a date, which is sent as midnight, and allows dates in the future. The API accepts any string as return_date, a future one included. This patch addresses point 4 of comment #77: - FlatPickrWrapper takes a 'config' prop, merged into the Koha flatpickr defaults - The return date picker takes a time, displayed according to TimeFormat, and its maximum date is now (refreshed when it is opened). The data attributes read by the flatpickr defaults are set too, so the date and time input mask is used, 'Today' sets the current time, and 'Tomorrow' is disabled like on the legacy page - The value sent to the API is a date-time without offset (e.g. 2026-10-29T10:30:00), so it is in the Koha time zone, like the legacy page posted it. The 'Return date' badge displays the time - POST /checkins declares return_date as a date-time, parses it as ISO (honouring the offset if any) and rejects a date in the future with a 400 and the 'return_date_in_future' error code, documented in the spec. The page displays a translatable message for it Test plan: 1. Apply this patch 2. Run: $ ktd --shell k$ yarn api:bundle k$ yarn js:build k$ restart_all 3. Enable SpecifyReturnDate 4. Check out an item, go to Circulation > Check in and open the check-in settings 5. Open the return date picker => SUCCESS: Dates after today are disabled, and so is 'Tomorrow' => SUCCESS: A time can be picked, in the TimeFormat format 6. Pick yesterday at 10:30 and check the item in => SUCCESS: The 'Return date' badge shows the date and time => SUCCESS: The item is checked in, and the return date on the item's checkout history shows yesterday at 10:30 7. Run: k$ prove t/db_dependent/api/v1/checkins.t xt/api.t => SUCCESS: Tests pass! k$ yarn cypress run --spec t/cypress/integration/Circulation/Checkin_spec.ts => SUCCESS: Tests pass! 8. Sign off :-D Assisted-by: Opus 5.5 (Anthropic) -- You are receiving this mail because: You are watching all bug changes.